kv: improve unit testing around txnSpanRefresher#40454
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Sep 4, 2019
Merged
kv: improve unit testing around txnSpanRefresher#40454craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Member
|
This change is |
tbg
approved these changes
Sep 4, 2019
Member
tbg
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten)
pkg/kv/txn_interceptor_pipeliner_test.go, line 56 at r1 (raw file):
} func (m *mockLockedSender) ChainMockSend(
Add a comment on the order in which things are chained.
421d047 to
0ce4af9
Compare
nvb
commented
Sep 4, 2019
Contributor
Author
nvb
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/kv/txn_interceptor_pipeliner_test.go, line 56 at r1 (raw file):
Previously, tbg (Tobias Grieger) wrote…
Add a comment on the order in which things are chained.
Done.
This was decently tested through its inclusion in the TxnCoordSender, but since I'm planning on changing some behavior to address cockroachdb#36431, I figured we should first build up a test suite in the same style that we have for other transaction interceptors. Release note: None
0ce4af9 to
6ffbc65
Compare
Contributor
Author
|
bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Sep 4, 2019
40435: storage: be more resilient to learner snap conflicts r=tbg a=danhhz The replica addition code first adds it as a raft learner, then hands it a snapshot, then promotes it to a voter. For various unfortunate reasons described in the code, we have to allow the raft snapshot queue to _also_ send snapshots to learners. A recent etcd change exposed that this code has always been brittle to the raft snapshot queue winning the race and starting the snapshot first by making the race dramatically more likely. After this commit, learner replica addition grabs a (best effort) lock before the conf change txn to add the learner is started. This prevents the race when the raft leader (and thus the raft snapshot queue for that range) is on the same node. Closes #40207 Release note: None 40454: kv: improve unit testing around txnSpanRefresher r=nvanbenschoten a=nvanbenschoten This was decently tested through its inclusion in the TxnCoordSender, but since I'm planning on changing some behavior to address #36431, I figured we should first build up a test suite in the same style that we have for other transaction interceptors. Release note: None Co-authored-by: Daniel Harrison <daniel.harrison@gmail.com> Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Contributor
Build succeeded |
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 was decently tested through its inclusion in the TxnCoordSender,
but since I'm planning on changing some behavior to address #36431, I
figured we should first build up a test suite in the same style that
we have for other transaction interceptors.
Release note: None