[infinispan-issues] [JBoss JIRA] (ISPN-2161) InitiatorCrashPessimisticReplTest.testInitiatorNodeCrashesBeforeCommit fails randomly

[Galder Zamarreño (JIRA)]

    [ https://issues.jboss.org/browse/ISPN-2161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707495#comment-12707495 ] 

Galder Zamarreño commented on ISPN-2161:
----------------------------------------

These are pretty intricate tests that need some rethinking to make them more reliable. To be more precise, these are the things I've spotted:

* In testInitiatorNodeCrashesBeforeCommit failure, a race condition could make the forked thread's transaction to commit in between these lines: http://goo.gl/lUPth - so, the key would be unlocked before reaching the next line (if the transaction's commit is executed by then). To make these tests clearer, maybe what we need is a transaction manager (or transaction object) that's plugged with latches to control the different lifecycles within it (prepare vs commit), as oppose to fiddling with the cache internals. So, we could control when a transaction runs the prepare, and when the transaction runs the commit phases. The problem with this approach is that in spite of making tests clearer, it only works for situations when that particular TM is plugged, and so the test might not work when it's run with other TMs.

* In testInitiatorCrashesBeforeReleasingLock, it's hard to say which of the parameters failed (we need to improve on debugging failures on this), but could be for a similar situation as explained previously.

* These tests leak transactions are shown by the WARN messages, so TestingUtil.withTx() needs to be used where possible to guarantee that there's always an outcome for the transaction, one way or the other.

* Finally, it would be advisable that all forked threads are converted to Callable so that Future instances can be kept and at the end of the test future.get() can be invoked. This is necessary to catch any extraordinary exceptions that might have happen and which have an impact in the test. Otherwise you're relying on log reading...

@Mircea, I'll assign it to you since you're more familiar with this codebase and you might have other ideas.
                
> InitiatorCrashPessimisticReplTest.testInitiatorNodeCrashesBeforeCommit fails randomly
> -------------------------------------------------------------------------------------
>
>                 Key: ISPN-2161
>                 URL: https://issues.jboss.org/browse/ISPN-2161
>             Project: Infinispan
>          Issue Type: Bug
>          Components: Test Suite
>            Reporter: Galder Zamarreño
>            Assignee: Galder Zamarreño
>              Labels: testsuite_stability
>             Fix For: 5.2.0.ALPHA3
>
>         Attachments: testInitiatorNodeCrashesBeforeCommit-0.log
>
>
> Failed tests:   testInitiatorNodeCrashesBeforeCommit(org.infinispan.lock.singlelock.replicated.pessimistic.InitiatorCrashPessimisticReplTest): expected key 'k' to be locked on cache Cache '___defaultcache'@InitiatorCrashPessimisticReplTest-NodeD-44283, but it is not

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira