kvserver: simplify testContext #72392
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
This is a deprecated testing tool that complicates `testContext`. This commit phases it out in two out of three tests that use it. Release note: None
This artificial test wasn't adding anything. Besides, we are not very concerned about whether the raft groups are initialized lazily or not. One could argue that at this point the laziness is a problem since it can make things look good when the system would become unstable once all raft groups are initialized. In practice, the queues go through all replicas every ~10 minutes anyway and after that has happened once, all raft groups will have been instantiated. Since this test uses the deprecated mode `bootstrapRangeOnly`, remove it. Release note: None
This is the reward for the preceding two commits. Release note: None
Release note: None
It supports pre-populating fields where this is never used, and if it were used who knows how well it would work. Better remove it now. Release note: None
This test originally verified that SSTs that would be sent as part of the raft log in a raft snapshot would be correctly inlined. But it has been fooling itself for a long time, since snapshots haven't had a log (in this test) for 1-2 years at this point. The test ended up using a for loop that never went anywhere and a resulting index of zero and just happened to never return a failing result. Replace it with a more useful test verifying that we can propose and read back a sideloaded proposal, the main bit of logic I wanted to preserve is a test for an idiosyncracy of `entries()` when no sideloaded storage is provided, in which case it can't add to the raft entry cache for fear of putting a non-inlined entry there. As a little additional upshot, there was no reason for this test to use an on-disk engine (anymore), since the sideloaded storage is backed by the pebble instance as well. Release note: None
This functionality was no longer used and is now removed. Release note: None
|
TFTR! bors r=erikgrinaker |
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commits for details. I just spent some time cleaning up
createTestStore(#72383) and I noticed thattestContextis much moreheavily used and has accumulated quite a bit of detritus.
I hope to ultimately make it use
createTestStorebut that will be foranother day.
testContext.engRelease note: None