We currently have multiple Raft replica migrations that are performed during store initialization. These are no longer needed after nodes have started on 23.1 at least once.
We should remove these, as they have a non-negligible cost and have occasionally been seen to take over a minute. We should also add corresponding assertions for the invariants that the migrations are meant to ensure.
|
// INVARIANT: all replicas have a persisted full ReplicaID (i.e. a "ReplicaID from disk"). |
|
// |
|
// This invariant is true for replicas created in 22.1. Without further action, it |
|
// would be violated for clusters that originated before 22.1. In this method, we |
|
// backfill the ReplicaID (for initialized replicas) and we remove uninitialized |
|
// replicas lacking a ReplicaID (see below for rationale). |
|
// |
|
// The migration can be removed when the KV host cluster MinSupportedVersion |
|
// matches or exceeds 23.1 (i.e. once we know that a store has definitely |
|
// started up on >=23.1 at least once). |
|
|
|
// Collect all the RangeIDs that either have a RaftReplicaID or HardState. For |
|
// unmigrated replicas we see only the HardState - that is how we detect |
|
// replicas that still need to be migrated. |
|
// |
|
// TODO(tbg): tighten up the case where we see a RaftReplicaID but no HardState. |
|
// This leads to the general desire to validate the internal consistency of the |
|
// entire raft state (i.e. HardState, TruncatedState, Log). |
|
{ |
|
var msg kvserverpb.RaftReplicaID |
|
if err := IterateIDPrefixKeys(ctx, eng, func(rangeID roachpb.RangeID) roachpb.Key { |
|
return keys.RaftReplicaIDKey(rangeID) |
|
}, &msg, func(rangeID roachpb.RangeID) error { |
|
s.setReplicaID(rangeID, msg.ReplicaID) |
|
return nil |
|
}); err != nil { |
|
return nil, err |
|
} |
|
|
|
var hs raftpb.HardState |
|
if err := IterateIDPrefixKeys(ctx, eng, func(rangeID roachpb.RangeID) roachpb.Key { |
|
return keys.RaftHardStateKey(rangeID) |
|
}, &hs, func(rangeID roachpb.RangeID) error { |
|
s.setHardState(rangeID, hs) |
|
return nil |
|
}); err != nil { |
|
return nil, err |
|
} |
|
} |
|
// Migrate into RaftReplicaID. This migration can be removed once the |
|
// MinSupportedVersion is >= 23.1, and we can assert that |
|
// repl.ReplicaID != 0 always holds. |
|
|
|
if descReplicaID != 0 { |
|
// Backfill RaftReplicaID for an initialized Replica. |
|
if err := logstore.NewStateLoader(repl.RangeID).SetRaftReplicaID(ctx, eng, descReplicaID); err != nil { |
|
return nil, errors.Wrapf(err, "backfilling ReplicaID for r%d", repl.RangeID) |
|
} |
|
repl.ReplicaID = descReplicaID |
|
sl[newIdx] = repl |
|
newIdx++ |
|
log.Eventf(ctx, "backfilled replicaID for initialized replica %s", repl.ID()) |
|
} else { |
|
// We found an uninitialized replica that did not have a persisted |
|
// ReplicaID. We can't determine the ReplicaID now, so we migrate by |
|
// removing this uninitialized replica. This technically violates raft |
|
// invariants if this replica has cast a vote, but the conditions under |
|
// which this matters are extremely unlikely. |
|
// |
|
// TODO(tbg): if clearRangeData were in this package we could destroy more |
|
// effectively even if for some reason we had in the past written state |
|
// other than the HardState here (not supposed to happen, but still). |
|
if err := eng.ClearUnversioned(logstore.NewStateLoader(repl.RangeID).RaftHardStateKey(), storage.ClearOptions{}); err != nil { |
|
return nil, errors.Wrapf(err, "removing HardState for r%d", repl.RangeID) |
|
} |
|
log.Eventf(ctx, "removed legacy uninitialized replica for r%s", repl.RangeID) |
|
// NB: removed from `sl` since we're not incrementing `newIdx`. |
|
} |
Jira issue: CRDB-34262
We currently have multiple Raft replica migrations that are performed during store initialization. These are no longer needed after nodes have started on 23.1 at least once.
We should remove these, as they have a non-negligible cost and have occasionally been seen to take over a minute. We should also add corresponding assertions for the invariants that the migrations are meant to ensure.
cockroach/pkg/kv/kvserver/kvstorage/init.go
Lines 417 to 455 in bf76e36
cockroach/pkg/kv/kvserver/kvstorage/init.go
Lines 508 to 536 in bf76e36
Jira issue: CRDB-34262