-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
release-25.1: kvserver: alias range_id in flow control integration tests #139567
base: release-25.1
Are you sure you want to change the base?
Conversation
These were duplicated around many tests in `flow_control_integration_test.go`. De-duplicate. Part of: #137727 Release note: None
The SQL query strings which are reused across several `TestFlowControl.*` tests were inconsistently formatted. Format them consistently, although not following any style guide. Part of: #137727 Release note: None
Previously, the `range_id` of scratch ranges used in various `TestFlowControl.*` tests would be printed into the testdata file. This made it tedious to introduce new system tables, which in turn may create new ranges and necessitate regenerating the `test_flow_control_integration` testdata files to pick up the higher default scratch range id. Alias the range id to letters in `a,b,...,z` so that regardless of the underlying scratch range id changing, the aliased scratch range id remains unchanged. For example: An old example: ```sql SELECT range_id, count(*) AS streams FROM crdb_internal.kv_flow_control_handles GROUP BY (range_id) ORDER BY streams DESC; range_id | stream_count -----------+--------------- 75 | 3 76 | 3 ``` Now looks like: ```sql SELECT chr(96 + dense_rank() OVER (ORDER BY range_id)) as range_id, count(*) AS streams FROM crdb_internal.kv_flow_control_handles GROUP BY range_id ORDER BY range_id; range_id | stream_count -----------+--------------- a | 3 b | 3 ``` Resolves: #137727 Release note: None
These tests are now redundant, as users will run only the RACv2 protocol post-upgrade to 24.3. Part of: #136529 Release note: None
Epic: none Release note: none
Previously, some `TestFlowControl.*` put requests were not printed to the datadriven test file, only printed to the test log. This commit updates these tests: ``` TestFlowControlRangeSplitMergeV2 TestFlowControlAdmissionPostSplitMergeV2 ``` To also print to the datadriven test file on puts. The test log is also still written to. Epic: None Release note: None
The following tests are updated to issue 1 byte sized puts, which are overridden via a testing knob to be 1 MiB. Some of these tests have previously flaked due to disconnected streams, hypothesized to be a result of CPU starvation in the test. ``` TestFlowControlBasicV2 TestFlowControlAdmissionPostSplitMergeV2 TestFlowControlRangeSplitMergeV2 TestFlowControlBlockedAdmissionV2 TestFlowControlCrashedNodeV2 TestFlowControlRaftMembershipV2 TestFlowControlRaftMembershipRemoveSelfV2 TestFlowControlUnquiescedRangeV2 TestFlowControlClassPrioritizationV2 TestFlowControlTransferLeaseV2 TestFlowControlLeaderNotLeaseholderV2 TestFlowControlGranterAdmitOneByOneV2 ``` Note, some of these tests did not have any recent failures associated with them and the load reduction is preventative. Informs: #137510 Informs: #138103 Release note: None
2115309
to
77f175c
Compare
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Failure on |
It does, in around 5 minutes of
|
Backport 7/7 commits from #138085 on behalf of @kvoli.
/cc @cockroachdb/release
Earlier commits are mechanical, reformatting and deduplicating SQL queries
used by the flow control integration tests.
Previously, the
range_id
of scratch ranges used in variousTestFlowControl.*
tests would be printed into the testdata file. Thismade it tedious to introduce new system tables, which in turn may create
new ranges and necessitate regenerating the
test_flow_control_integration
testdata files to pick up the higherdefault scratch range id.
Alias the range id to letters in
a,b,...,z
so that regardless of theunderlying scratch range id changing, the aliased scratch range id
remains unchanged. For example:
An old example:
Now looks like:
Resolves: #137727
Release note: None
Release justification: Test only.