Skip to content

Commit

Permalink
Merge #39658
Browse files Browse the repository at this point in the history
39658: storage: avoid crash in splitPostApply r=danhhz a=tbg

See #39571.

The above PR was incomplete because we also need to wind back the
replicaID. I was able to confirm that this *actually* works by running
tests with this diff, which simulates an incoming raft message
reflecting a high replicaID just before the split gets applied.

```diff
diff --git a/pkg/storage/replica_raft.go b/pkg/storage/replica_raft.go
index 6d5a044657..981f3e1714 100644
--- a/pkg/storage/replica_raft.go
+++ b/pkg/storage/replica_raft.go
@@ -1525,7 +1525,7 @@ func (r *Replica) maybeAcquireSplitMergeLock(
 func (r *Replica) acquireSplitLock(
 	ctx context.Context, split *roachpb.SplitTrigger,
 ) (func(), error) {
-	rightRng, _, err := r.store.getOrCreateReplica(ctx, split.RightDesc.RangeID, 0, nil)
+	rightRng, _, err := r.store.getOrCreateReplica(ctx, split.RightDesc.RangeID, 100, nil)
 	if err != nil {
 		return nil, err
 	}
```

@danhhz is working on a real test for this, so we'll soon have
end-to-end coverage.

Fixes #39651.

Release note: None

Co-authored-by: Tobias Schottdorf <tobias.schottdorf@gmail.com>
  • Loading branch information
craig[bot] and tbg committed Aug 14, 2019
2 parents f81fedb + d822868 commit e73d88d
Showing 1 changed file with 11 additions and 0 deletions.
11 changes: 11 additions & 0 deletions pkg/storage/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2156,6 +2156,17 @@ func splitPostApply(
// about restoring the existing minReplicaID if it's nonzero. No
// promises have been made, so none need to be kept.
rightRng.mu.minReplicaID = 0
rightDesc, ok := split.RightDesc.GetReplicaDescriptor(r.StoreID())
if !ok {
log.Fatalf(ctx, "replica descriptor of local store not found in right hand side of split")
}
// We also have to potentially wind back the replicaID to avoid an error
// below. We can't overwrite unconditionally since usually the replicaID is
// zero (and thus special) and in that case, moving it forward out-of-band
// results in an broken state.
if rightRng.mu.replicaID > rightDesc.ReplicaID {
rightRng.mu.replicaID = rightDesc.ReplicaID
}
err := rightRng.initRaftMuLockedReplicaMuLocked(&split.RightDesc, r.store.Clock(), 0)
rightRng.mu.Unlock()
if err != nil {
Expand Down

0 comments on commit e73d88d

Please sign in to comment.