Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kv: deflake TestFollowerReadsWithStaleDescriptor #108366

Conversation

nvanbenschoten
Copy link
Member

Fixes #108349.

This commit deflakes TestFollowerReadsWithStaleDescriptor by disabling kvprober. As of 769ba1c, kvprober is now metaphorically enabled in all tests. When enabled, it can cause unexpected updates to the range cache, which trip up the test's assertion that a given query is not served on a newly added follower replica.

The failure was easy to reproduce if I added a sleep after the non-voter addition. This change deflakes the test (even with this sleep) by disabling kvprober.

Release note: None

Fixes cockroachdb#108349.

This commit deflakes TestFollowerReadsWithStaleDescriptor by disabling
kvprober. As of 769ba1c, kvprober is now metaphorically enabled in
all tests. When enabled, it can cause unexpected updates to the range
cache, which trip up the test's assertion that a given query is not
served on a newly added follower replica.

The failure was easy to reproduce if I added a sleep after the non-voter
addition. This change deflakes the test (even with this sleep) by disabling
kvprober.

Release note: None
@nvanbenschoten nvanbenschoten requested review from a team as code owners August 8, 2023 16:37
@nvanbenschoten nvanbenschoten requested review from rachitgsrivastava and DarrylWong and removed request for a team August 8, 2023 16:37
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@andrewbaptist andrewbaptist left a comment

Choose a reason for hiding this comment

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

This should be closed once the kvProber revert happens.

craig bot pushed a commit that referenced this pull request Aug 9, 2023
107355: cli: enhance diagnosing contention with redacted debug zips r=fqazi a=fqazi

Previously a redacted zip, we would exclude retries and key information for contended keys since they could contain PII data. This patch does the following:

- Adds a new builtin is_system_table_key which allows us to know if a key belongs to a system table
- Modified redacted debug zips to include data for system keys in contention tables conditionally (if they belong to system tables)
- Include retries and last_retry_reason information for queries in cluster insights to help diagnose contention

Fixes: #104593

108454: Revert "kvprober: metamorphically enable / configure kvprober" r=andrewbaptist,joshimhoff a=nvanbenschoten

This reverts (most of) commit 769ba1c.

That commit metamorphically enabled kvprober. This has been observed to be destabliziing to unit tests. When the metamorphic constant is enabled (50% of the time) and when kvprober is fast enough, random ranges will see extra requests that they aren’t expecting. This adds nondeterminism which can trip up tests in any number of different ways.

All of the following flakes have been tracked back to kvprober:

Fixes #107864.
Fixes #108242.
Fixes #108441.
Fixes #108349.
Fixes #108124.
Closes #108366.

Release note: None

Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
@craig craig bot closed this in #108454 Aug 9, 2023
@craig craig bot closed this in 1374908 Aug 9, 2023
@nvanbenschoten nvanbenschoten deleted the nvanbenschoten/deflakeTestFollowerReadsWithStaleDescriptor branch August 9, 2023 20:22
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.

ccl/kvccl/kvfollowerreadsccl: TestFollowerReadsWithStaleDescriptor failed
3 participants