Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISPN-2161 InitiatorCrashPessimisticReplTest.testInitiatorNodeCrashesBefo... #1958

Closed
wants to merge 1 commit into from

Conversation

pruivo
Copy link
Member

@pruivo pruivo commented Jul 8, 2013

...reCommit fails randomly

  • Cleanup by replacing all the assert keywords by AssertJUnit.assertEquals()
  • Problematic test implementation is no-op. See code commentary.

https://issues.jboss.org/browse/ISPN-2161

…eforeCommit fails randomly

* Cleanup by replacing all the assert keywords by AssertJUnit.assertEquals()

* Problematic test implementation is no-op. See code commentary.
@mmarkus
Copy link
Contributor

mmarkus commented Jul 9, 2013

thanks!

@mmarkus mmarkus closed this Jul 9, 2013
In pessimist caches, we have only one phase (PrepareCommand with onePhaseCommit to true).
So we only have two scenarios:
1. initiator dies before prepare (covered in testInitiatorNodeCrashesBeforePrepare)
2. initiator dies after prepare (covered in testInitiatorCrashesBeforeReleasingLock)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pruivo I thought the PrepareCommand is sent during the commit phase?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danberindei you are correct. However, this test is invoking TM.commit() that will perform the prepare phase + commit phase

Also the test tries to block the commit in the begin of the interceptor chain (that never happens because the commit is not sent). The tests fails when the prepare is faster than "main" thread and it commits and releases the locks before the assertions.

If the comment is not clear I can try to improve it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I didn't read the test properly and I thought it was using TransactionXaAdapter.prepare/commit to check the locks before sending the prepare/commit (confusion with InitiatorCrashPessimisticTest, perhaps).

Maybe the comment could be clearer, e.g "before the prepare is received on the coordinator", but I wouldn't ask for a new PR just for that :)

@pruivo pruivo deleted the ISPN-2161 branch July 5, 2022 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants