On 9 Jan 2012, at 11:43, Mircea Markus wrote:

On 9 Jan 2012, at 13:36, Manik Surtani wrote:

On 9 Jan 2012, at 10:50, Mircea Markus wrote:

On 7 Jan 2012, at 18:57, Manik Surtani wrote:

After spending some hours on ISPN-1284, I think we have a few conceptual problems with using Synchronizations.

Here is the topic branch containing my work:

https://github.com/maniksurtani/infinispan/tree/t_1284

So far what I have done is:

* Changed defaults

* Changed config validations to emit a warning and disable synchronisations if recovery is enabled

* Update some tests that rely on XA to specifically set synchronisations to false

But there are still some issues, and what specifically worry me is behaviour demonstrated by these two test failures:

  testModsCommit(org.infinispan.lock.StaleEagerLocksOnPrepareFailureTest)

  testNoModsCommit(org.infinispan.lock.StaleEagerLocksOnPrepareFailureTest)

They both attempt to abort a transaction by throwing an exception during prepare.  Now since we use Synchronizations by default, these failures do *not* abort the transaction since it is only seen by the SynchronizationAdapter in afterCommit().  Where does this stand, conceptually?  With optimistic transactions, locks may not be able to be acquired at prepare-time.  These transactions should fail.

With pessimistic transactions, the first phase of 2PC is skipped: that is because locks are acquired and modifications propagated at this stage. So beforeCompletion doesn't do anything. 

However in the case of optimistic transactions the prepare happens during beforeCompletion and it would fail causing the tx to roll back.

So Synchronizations and pessimistic locking are incompatible?  :)

Well this is very similar to what happens in the case of optimistic locking when the  CommitCommand cannot be broadcasted: the transaction silently fails.

Then the tests need to be re-written in this case, if we make synchronisations a default? :)