kvnemesis,kvserver: add coverage for illegal lease applied index and reproposal errors#105272
Merged
craig[bot] merged 8 commits intocockroachdb:masterfrom Jun 27, 2023
Merged
Conversation
Member
|
This change is |
Member
Author
|
No good deed goes unpunished, the new testing is bringing out some weird results: DetailsI am not sure how Edit: ah, better logging shows: Edit: I don't understand the push error though, this is a edit: filed #105330. |
bd731ea to
c8e86ba
Compare
tbg
added a commit
to tbg/cockroach
that referenced
this pull request
Jun 21, 2023
These are in cockroachdb#105272.
c8e86ba to
b2531ef
Compare
erikgrinaker
approved these changes
Jun 26, 2023
Contributor
erikgrinaker
left a comment
There was a problem hiding this comment.
Great to have coverage of this!
| }) | ||
| } | ||
|
|
||
| func TestKVNemesisSingleNode_ReproposalChaos(t *testing.T) { |
Contributor
There was a problem hiding this comment.
I take it we don't really need any long-running nightlies for this, it's sufficient with a few runs as part of regular stress runs?
Epic: none Release note: none
This allows setting a LeaseFilter from outside the `kvserver` package.
Epic: none Release note: none
This test can shake out bugs in the reproposal pipeline by actively introducing reproposals. Epic: none Release note: none
It hasn't bit us yet, but with upcoming code changes we will see more situations in which a batch will hard-fail. There were a few places in `ClosureTxnOperation` where this would leave the remaining operations in the txn with uninitialized results. `kvnemesis` requires all results to be explicit (i.e. success or error). Now, extending the previous but incomplete approach, they are all set to `errOmitted`.
b2531ef to
f2da8d3
Compare
This introduces a flavor of single-node kvnemesis where KV mutations randomly bounce around in the replication layer a few times before actually applying.
We're frequently getting a TransactionPushError back from `db.{Get,Scan,...}`.
Accept this as "ok" while we investigate the failure, to avoid delaying merging
the kvnemesis updates.
cockroachdb#105330
f2da8d3 to
e3bafec
Compare
Member
Author
|
Here goes, TFTR! bors r=erikgrinaker |
Contributor
|
Build succeeded: |
tbg
added a commit
to tbg/cockroach
that referenced
this pull request
Jun 28, 2023
This test can't be salvaged and besides, hasn't been run in >1.5 years. We just merged decent enough coverage in cockroachdb#105272, though. Closes cockroachdb#71148. Epic: none Release note: None
craig bot
pushed a commit
that referenced
this pull request
Jun 28, 2023
105622: kvserver: rm TestProposalNotAcknowledgedOrReproposedAfterApplication r=erikgrinaker a=tbg This test can't be salvaged and besides, hasn't been run in >1.5 years. We just merged decent enough coverage in #105272, though. Closes #71148. Epic: none Release note: None Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This adds testing knobs that allow probabilistically invalidating the
MaxLeaseIndexof proposals in the replication layer, and also allowsinjecting errors into the reproposals that result from doing so.o
It then uses these knobs in kvnemesis and in a newly added "more targeted"
kvserverunit test. This unit test was co-developed with the kvnemesisflavor, and various amounts of cross-pollination have ensued. The idea is that
kvnemesis produces "more randomness" and thus has a higher chance of finding
interesting behavior, while the kvserver test is more suitable to
reproducing/regression testing that behavior. For example, tracing initially
caused problems in kvnemesis, so I added tracking to the kvserver test, and
sorted out the problems this encountered (which in turn addressed the problem
in kvnemesis as well).
I am adding this testing as part of my work on
#97779. Because the test is
agnostic to how exactly reproposals are implemented, I am landing it
separately which will make it very clear that it can pass without these
changes (and once did). With #97779, the plan is to metamorphically swap
the implementations to retain coverage for both, until we are willing to
commit to the new method in #97779.
Release note: None
Epic: CRDB-25287