Skip to content

Conversation

@ztzg
Copy link
Contributor

@ztzg ztzg commented Feb 4, 2021

WatcherCleanerTest performs latency checks which fail when outside of a 20+Xms window. Before this patch, X was 5ms—whereas 30+ms is frequently seen on an i5 Mac Mini running macOS Catalina.

This "dumb" patch just widens the window to 20ms, which makes it "work on my machine," but could obviously still fail in a loaded environment or VM.

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

@symat symat left a comment

Choose a reason for hiding this comment

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

looks OK, for now.

although I really don't like these type of tests, operating with real timing... a better solution would be to use some logical / mocked time. But I didn't dig deep enough to be able to decide how much work that test refactoring would be.

But honestly, if we see this test to continue to be flaky, then we should @ignore it until it can be rewritten in a more robust way.

@ztzg
Copy link
Contributor Author

ztzg commented Feb 4, 2021

although I really don't like these type of tests, operating with real timing... a better solution would be to use some logical / mocked time.

Definitely. This is a minimal workaround to give the test a chance to succeed as I am still trying to release 3.7—but we do indeed have a steep slope to climb before we are back to test/CI stability…

`WatcherCleanerTest` performs latency checks which fail when outside
of a 20+Xms window.  Before this patch, X was 5ms--whereas 30+ms is
frequently seen on an i5 Mac Mini running macOS Catalina.

This "dumb" patch just widens the window to 20ms, which makes it "work
on my machine," but could obviously still fail in a loaded environment
or VM.
@ztzg ztzg force-pushed the ZOOKEEPER-4200-widen-latency-window branch from b7f3488 to ede957e Compare February 17, 2021 17:50
@ztzg
Copy link
Contributor Author

ztzg commented Feb 17, 2021

@eolivelli, @symat: I have rebased this on master, and CI went through! Would one of you mind merging it?

@eolivelli
Copy link
Contributor

doing it now, thanks @ztzg
should I cherry pick to branch-3.7 ?

@eolivelli eolivelli closed this in d8ff555 Feb 17, 2021
eolivelli pushed a commit that referenced this pull request Feb 17, 2021
`WatcherCleanerTest` performs latency checks which fail when outside of a 20+Xms window.  Before this patch, X was 5ms—whereas 30+ms is frequently seen on an i5 Mac Mini running macOS Catalina.

This "dumb" patch just widens the window to 20ms, which makes it "work on my machine," but could obviously still fail in a loaded environment or VM.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes #1592 from ztzg/ZOOKEEPER-4200-widen-latency-window

(cherry picked from commit d8ff555)
Signed-off-by: Enrico Olivelli <[email protected]>
@eolivelli
Copy link
Contributor

Picked to 3.7.0 branch
thanks

@ztzg
Copy link
Contributor Author

ztzg commented Feb 17, 2021

Yes. Perfect—thanks!

asfgit pushed a commit that referenced this pull request Mar 6, 2021
`WatcherCleanerTest` performs latency checks which fail when outside of a 20+Xms window.  Before this patch, X was 5ms—whereas 30+ms is frequently seen on an i5 Mac Mini running macOS Catalina.

This "dumb" patch just widens the window to 20ms, which makes it "work on my machine," but could obviously still fail in a loaded environment or VM.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes #1592 from ztzg/ZOOKEEPER-4200-widen-latency-window
@ztzg
Copy link
Contributor Author

ztzg commented Mar 6, 2021

Now also in branch-3.7!

RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
`WatcherCleanerTest` performs latency checks which fail when outside of a 20+Xms window.  Before this patch, X was 5ms—whereas 30+ms is frequently seen on an i5 Mac Mini running macOS Catalina.

This "dumb" patch just widens the window to 20ms, which makes it "work on my machine," but could obviously still fail in a loaded environment or VM.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1592 from ztzg/ZOOKEEPER-4200-widen-latency-window
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
`WatcherCleanerTest` performs latency checks which fail when outside of a 20+Xms window.  Before this patch, X was 5ms—whereas 30+ms is frequently seen on an i5 Mac Mini running macOS Catalina.

This "dumb" patch just widens the window to 20ms, which makes it "work on my machine," but could obviously still fail in a loaded environment or VM.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1592 from ztzg/ZOOKEEPER-4200-widen-latency-window
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
`WatcherCleanerTest` performs latency checks which fail when outside of a 20+Xms window.  Before this patch, X was 5ms—whereas 30+ms is frequently seen on an i5 Mac Mini running macOS Catalina.

This "dumb" patch just widens the window to 20ms, which makes it "work on my machine," but could obviously still fail in a loaded environment or VM.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1592 from ztzg/ZOOKEEPER-4200-widen-latency-window
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
`WatcherCleanerTest` performs latency checks which fail when outside of a 20+Xms window.  Before this patch, X was 5ms—whereas 30+ms is frequently seen on an i5 Mac Mini running macOS Catalina.

This "dumb" patch just widens the window to 20ms, which makes it "work on my machine," but could obviously still fail in a loaded environment or VM.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1592 from ztzg/ZOOKEEPER-4200-widen-latency-window
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 29, 2022
`WatcherCleanerTest` performs latency checks which fail when outside of a 20+Xms window.  Before this patch, X was 5ms—whereas 30+ms is frequently seen on an i5 Mac Mini running macOS Catalina.

This "dumb" patch just widens the window to 20ms, which makes it "work on my machine," but could obviously still fail in a loaded environment or VM.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1592 from ztzg/ZOOKEEPER-4200-widen-latency-window
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