Skip to content

ZOOKEEPER-4200: Widen latency window in WatcherCleanerTest - #1592

Closed
ztzg wants to merge 1 commit into
apache:masterfrom
ztzg:ZOOKEEPER-4200-widen-latency-window
Closed

ZOOKEEPER-4200: Widen latency window in WatcherCleanerTest#1592
ztzg wants to merge 1 commit into
apache:masterfrom
ztzg:ZOOKEEPER-4200-widen-latency-window

Conversation

@ztzg

@ztzg ztzg commented Feb 4, 2021

Copy link
Copy Markdown
Contributor

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.

@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

@symat symat 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.

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

ztzg commented Feb 4, 2021

Copy link
Copy Markdown
Contributor Author

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

ztzg commented Feb 17, 2021

Copy link
Copy Markdown
Contributor Author

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

@eolivelli

Copy link
Copy Markdown
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 <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

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

(cherry picked from commit d8ff555)
Signed-off-by: Enrico Olivelli <eolivelli@datastax.com>
@eolivelli

Copy link
Copy Markdown
Contributor

Picked to 3.7.0 branch
thanks

@ztzg

ztzg commented Feb 17, 2021

Copy link
Copy Markdown
Contributor Author

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 <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

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

ztzg commented Mar 6, 2021

Copy link
Copy Markdown
Contributor Author

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 <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

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 <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

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 <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

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 <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

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 <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>

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