On Oct 11, 2013, at 11:30 AM, Pedro Ruivo <pedro(a)infinispan.org&gt; wrote: > Hi guys. > > Re: https://issues.jboss.org/browse/ISPN-2090 > > Summary: in tx caches, when @listener(sync = true), the callback is done > by the same thread as the transaction. However, most of the events may > not cause any problem, some of them are trigger when the transaction is > in the state that cannot attach more commands. > > So, I want to discuss here what would be the correct behaviour/logic for > the callback (note this only affects tx caches) assuming that the > callback invokes some operation (get/put/etc.) over the cache. > > IMO, all the synchronous events should suspend the transaction. This > way, we ensure that the operation is not attached to the transaction, > the callback has the same behaviour if sync=true and sync=false and also > with the events triggered in remote nodes (you will never read any > non-committed data from a remote transaction neither attach the operation). ^ I think it makes sense, but this would break code that's based around the current assumptions, and given how late we are in the 6.0 release, I would not add this right now, and when added it needs to be properly documented to indicate the differences. > Mircea suggestion is the following: <- missing something here? > I think the correct logic should be: > > * if isPre() then the value should not be seen when reading the cache > * if !isPre() then value should be seen. Not totally sure, but the > transaction as return by TransactionManager.getTransaction should not be > visible (i.e. suspend it before notifying the listener), as this would > result in different behavior on local/remote nodes that get notified. ^ I'm not sure how you can achieve for data to be seen when !is.Pre(), assuming you mean data that's accessible via the cache interface and not data that comes as part of the event. I'd say that both isPre and !isPre should not allow on-going transaction data when accessing via the cache. Of course, event parameters should carry the data that the user might need.

For example a strong use case for these events is data validation: I'm looking forward for an integration with Bean Validation, and when that happens I'm pretty sure we'll want to rollback the transaction in case of validation failures.

BTW I still don't like the need to invoke "isPre()". IMO you should provide a way to register the Listener to happen only after or before; the isPre method can stay where it is for convenience but it bothers me that you have to call it on each invocation, it should be optional, for example for those cases in which I need two different operations to happen before/after and I'm too lazy to code two different listener.

It looks like that this discussion is based on what's more convenient for you all to code, but the decision should be made based on what users need, and frankly I think there are lots of good reasons to expect the event to happen in the scope of the same transaction, and even had the capability to fail the transaction. For example a strong use case for these events is data validation: I'm looking forward for an integration with Bean Validation, and when that happens I'm pretty sure we'll want to rollback the transaction in case of validation failures.

BTW I still don't like the need to invoke "isPre()". IMO you should provide a way to register the Listener to happen only after or before; the isPre method can stay where it is for convenience but it bothers me that you have to call it on each invocation, it should be optional, for example for those cases in which I need two different operations to happen before/after and I'm too lazy to code two different listener. Sanne On 17 October 2013 11:10, Radim Vansa <rvansa(a)redhat.com&gt; wrote: > On 10/17/2013 09:36 AM, Galder Zamarreño wrote: >> On Oct 11, 2013, at 11:30 AM, Pedro Ruivo <pedro(a)infinispan.org&gt; wrote: >> >>> Hi guys. >>> >>> Re: https://issues.jboss.org/browse/ISPN-2090 >>> >>> Summary: in tx caches, when @listener(sync = true), the callback is done >>> by the same thread as the transaction. However, most of the events may >>> not cause any problem, some of them are trigger when the transaction is >>> in the state that cannot attach more commands. >>> >>> So, I want to discuss here what would be the correct behaviour/logic for >>> the callback (note this only affects tx caches) assuming that the >>> callback invokes some operation (get/put/etc.) over the cache. >>> >>> IMO, all the synchronous events should suspend the transaction. This >>> way, we ensure that the operation is not attached to the transaction, >>> the callback has the same behaviour if sync=true and sync=false and also >>> with the events triggered in remote nodes (you will never read any >>> non-committed data from a remote transaction neither attach the operation). >> ^ I think it makes sense, but this would break code that's based around the current assumptions, and given how late we are in the 6.0 release, I would not add this right now, and when added it needs to be properly documented to indicate the differences. >> >>> Mircea suggestion is the following: >> <- missing something here? >> >>> I think the correct logic should be: >>> >>> * if isPre() then the value should not be seen when reading the cache >>> * if !isPre() then value should be seen. Not totally sure, but the >>> transaction as return by TransactionManager.getTransaction should not be >>> visible (i.e. suspend it before notifying the listener), as this would >>> result in different behavior on local/remote nodes that get notified. >> ^ I'm not sure how you can achieve for data to be seen when !is.Pre(), assuming you mean data that's accessible via the cache interface and not data that comes as part of the event. I'd say that both isPre and !isPre should not allow on-going transaction data when accessing via the cache. Of course, event parameters should carry the data that the user might need. > Carrying the data in parameter might not be enough if you allow the > event listener to do writes, IMO. Let's have A=1, B=1 in cache and > application constraint that A >= B. Let's set A to 2 - that would fire > an event in which you would like to set B to 2, and B's event listener > would like to know value of A - getting A = 1 will break the constraint. > And there's no way to get A's value besides reading it directly from cache. > > > Radim > > -- > Radim Vansa <rvansa(a)redhat.com&gt; > JBoss DataGrid QA > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev