Skip to content

ZOOKEEPER-3996: Fix flaky ReadOnlyModeTest.testConnectionEvents - #1896

Merged
anmolnar merged 1 commit into
apache:masterfrom
kezhuw:ZOOKEEPER-3996-flaky-ReadOnlyModeTest-testConnectionEvents
Jun 19, 2023
Merged

ZOOKEEPER-3996: Fix flaky ReadOnlyModeTest.testConnectionEvents#1896
anmolnar merged 1 commit into
apache:masterfrom
kezhuw:ZOOKEEPER-3996-flaky-ReadOnlyModeTest-testConnectionEvents

Conversation

@kezhuw

@kezhuw kezhuw commented Jun 22, 2022

Copy link
Copy Markdown
Member

The same watcher was resued across different clients. It is hard to
know which event will last during verification, disconnected from old
client or connected from new client.

A brand new watcher solves this.

@kezhuw

kezhuw commented Jun 22, 2022

Copy link
Copy Markdown
Member Author

@kezhuw

kezhuw commented Jun 22, 2022

Copy link
Copy Markdown
Member Author

#1667(ZOOKEEPER-4271) proposed another approach. But it have not updated for almost one year.

@kezhuw
kezhuw force-pushed the ZOOKEEPER-3996-flaky-ReadOnlyModeTest-testConnectionEvents branch from c7f8638 to ffb31f7 Compare June 22, 2022 13:21
@kezhuw
kezhuw force-pushed the ZOOKEEPER-3996-flaky-ReadOnlyModeTest-testConnectionEvents branch from ffb31f7 to ff05cc3 Compare June 17, 2023 06:03
The same watcher was resued across different clients. It is hard to
know which event will last during verification, disconnected from old
client or connected from new client.

A brand new watcher solves this.
@kezhuw
kezhuw force-pushed the ZOOKEEPER-3996-flaky-ReadOnlyModeTest-testConnectionEvents branch from ff05cc3 to b1073c3 Compare June 17, 2023 06:34
@kezhuw

kezhuw commented Jun 17, 2023

Copy link
Copy Markdown
Member Author

The old test could not last 10 runs after adding Thread.sleep(700) before watcher.waitForReadOnlyConnected(5000). The assertion message proves that "disconnected from old client" comes after "connected from new client".

After patch with watcher = new CountdownWatcher(), the test last 100 runs with no failure.

@anmolnar anmolnar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm.

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

@anmolnar
anmolnar merged commit 618b676 into apache:master Jun 19, 2023
@anmolnar

Copy link
Copy Markdown
Contributor

Submitted and Jira resolved. Thanks @kezhuw !

anurag-harness pushed a commit to anurag-harness/zookeeper that referenced this pull request Aug 31, 2023
…he#1896)

The same watcher was resued across different clients. It is hard to
know which event will last during verification, disconnected from old
client or connected from new client.

A brand new watcher solves this.
anurag-harness added a commit to anurag-harness/zookeeper that referenced this pull request Aug 31, 2023
…he#1896) (#61)

The same watcher was resued across different clients. It is hard to
know which event will last during verification, disconnected from old
client or connected from new client.

A brand new watcher solves this.

Co-authored-by: Kezhu Wang <kezhuw@gmail.com>
rahulrane50 pushed a commit to rahulrane50/zookeeper that referenced this pull request Sep 15, 2023
…he#1896)

The same watcher was resued across different clients. It is hard to
know which event will last during verification, disconnected from old
client or connected from new client.

A brand new watcher solves this.
@kezhuw
kezhuw deleted the ZOOKEEPER-3996-flaky-ReadOnlyModeTest-testConnectionEvents branch September 24, 2023 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants