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

Fixed ZKSessionTest.testReacquireLocksAfterSessionLost #11886

Merged
merged 1 commit into from
Sep 1, 2021

Conversation

merlimat
Copy link
Contributor

@merlimat merlimat commented Sep 1, 2021

Motivation

Fixes the flakiness in ZKSessionTest.testReacquireLocksAfterSessionLost test:

Error:  testReacquireLocksAfterSessionLost(org.apache.pulsar.metadata.ZKSessionTest)  Time elapsed: 22.547 s  <<< FAILURE!
java.lang.AssertionError: expected [Reconnected] but found [null]
	at org.testng.Assert.fail(Assert.java:99)
	at org.testng.Assert.failNotEquals(Assert.java:1037)
	at org.testng.Assert.assertEqualsImpl(Assert.java:140)
	at org.testng.Assert.assertEquals(Assert.java:122)
	at org.testng.Assert.assertEquals(Assert.java:617)
	at org.apache.pulsar.metadata.ZKSessionTest.testReacquireLocksAfterSessionLost(ZKSessionTest.java:126)

The root cause for the test failing is that, when the session is forcefully expired, we're also getting a watch triggered for the deletion of the ephemeral node. That means that, depending on the timing of the watch, we can end up trying to revalidate the lock while we're not connected to ZK.

When that happens, we should expire the lock, instead we only expire it if we get a BadVersion on the put operation, which means that someone else took the lock over.

This way, we can retry the validation when we're finally reconnected to ZK.

@merlimat merlimat added type/bug The PR fixed a bug or issue reported a bug doc-not-needed Your PR changes do not impact docs release/2.8.2 labels Sep 1, 2021
@merlimat merlimat added this to the 2.9.0 milestone Sep 1, 2021
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

Good stuff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.8 Archived: 2.8 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.2 release/2.9.0 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants