Skip to content

Conversation

@joshimhoff
Copy link
Collaborator

Fixes #107873.

kvprober: remove unnecessary checks to avoid flakes

This commit removes two unnecessary checks, to avoid flakes. The removed checks can cause flakes, since they require all probes (of a certain type) to fail, even tho the test server is not configured to fail probes until after server startup. That is, there is a race condition. The checks are also unnecessary. What we really care about is the checks that follow the deleted ones: That the ten probes run within test all fail, since those happen after the test server is configured to fail probes.

Release note: None.
Epic: None.

This commit removes two unnecessary checks, to avoid flakes. The removed checks
can cause flakes, since they require all probes (of a certain type) to fail,
even tho the test server is not configured to fail probes until after server
startup. That is, there is a race condition. The checks are also unnecessary.
What we really care about is the checks that follow the deleted ones: That
the ten probes run within test all fail, since those happen after the test
server is configured to fail probes.

Release note: None.
Epic: None.
@joshimhoff joshimhoff requested a review from pav-kv August 3, 2023 14:05
@joshimhoff joshimhoff requested a review from a team as a code owner August 3, 2023 14:05
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@pav-kv pav-kv left a comment

Choose a reason for hiding this comment

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

Does this need backporting, in case this flakes in other branches?

@joshimhoff
Copy link
Collaborator Author

I don't believe so. The test refactor made in #104365 is only in master, and I believe it lead to these flakes. If you want details, let me know, and I will elaborate. The TLDR is we are now using the server's prober rather than a separate one created within the test. I think it's a good change, but it requires adjustments to the checks, as made in this PR.

@joshimhoff
Copy link
Collaborator Author

TFTR!

@joshimhoff
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 3, 2023

Build succeeded:

@craig craig bot merged commit 03b1d4c into cockroachdb:master Aug 3, 2023
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.

kv/kvprober: TestProberDoesReadsAndWrites failed

3 participants