ZOOKEEPER-4327: Fix flaky RequestThrottlerTest.testLargeRequestThrottling #31
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ling This test failed following assertions in ci: 1. `RequestThrottlerTest.testRequestThrottler:206 expected: <5> but was: <4>` This is caused by no happens-before relationship between `connectionLossCount` and `disconnected.await`. Places `disconnected.countDown()` after `connectionLossCount++` to solve this. 2. `RequestThrottlerTest.testLargeRequestThrottling:297 expected: <2> but was: <0>` Large request throttling is handled in io thread, while `prep_processor_request_queued` metric is updated in processor thread. Places metric assertion after `finished.await` to solve this. Additionally, I find one more potential flaky case. After connection closed due to throttling third request, reconnecting could fail this test in slow sending environment. It is easy to reproduce by adding `Thread.sleep(i * 100)` in sending loop. Author: Kezhu Wang <kezhuw@gmail.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1821 from kezhuw/ZOOKEEPER-4327-flaky-RequestThrottlerTest.testLargeRequestThrottling and squashes the following commits: e21c2f8 [Kezhu Wang] ZOOKEEPER-4327: Fix flaky RequestThrottlerTest.testDropStaleRequests 3df34b8 [Kezhu Wang] ZOOKEEPER-4327: Fix flaky RequestThrottlerTest.testLargeRequestThrottling
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This test failed following assertions in ci:
RequestThrottlerTest.testRequestThrottler:206 expected: <5> but was: <4>This is caused by no happens-before relationship between
connectionLossCountanddisconnected.await. Placesdisconnected.countDown()afterconnectionLossCount++to solvethis.
RequestThrottlerTest.testLargeRequestThrottling:297 expected: <2> but was: <0>Large request throttling is handled in io thread, while
prep_processor_request_queuedmetric is updated in processorthread. Places metric assertion after
finished.awaitto solve this.Additionally, I find one more potential flaky case. After connection
closed due to throttling third request, reconnecting could fail this
test in slow sending environment. It is easy to reproduce by adding
Thread.sleep(i * 100)in sending loop.Author: Kezhu Wang kezhuw@gmail.com
Reviewers: Enrico Olivelli eolivelli@apache.org, Mate Szalay-Beko symat@apache.org
Closes apache#1821 from kezhuw/ZOOKEEPER-4327-flaky-RequestThrottlerTest.testLargeRequestThrottling and squashes the following commits:
e21c2f8 [Kezhu Wang] ZOOKEEPER-4327: Fix flaky RequestThrottlerTest.testDropStaleRequests
3df34b8 [Kezhu Wang] ZOOKEEPER-4327: Fix flaky RequestThrottlerTest.testLargeRequestThrottling