Skip to content

Commit 84f9ed3

Browse files
committed
kvserver: move the split RHS clearing to kvstorage
Epic: none Relese note: none
1 parent 9d8cfdd commit 84f9ed3

File tree

2 files changed

+40
-22
lines changed

2 files changed

+40
-22
lines changed

pkg/kv/kvserver/kvstorage/destroy.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,40 @@ func SubsumeReplica(
216216
UnreplicatedByRangeID: opts.ClearUnreplicatedByRangeID,
217217
}, destroyReplicaImpl(ctx, reader, writer, info, MergedTombstoneReplicaID, opts)
218218
}
219+
220+
// RemoveStaleRHSFromSplit removes all data for the RHS replica of a split. This
221+
// is used in a situation when the RHS replica is already known to have been
222+
// removed from our store, so any pending writes that were supposed to
223+
// initialize the RHS replica should be dropped from the write batch.
224+
//
225+
// TODO(#152199): do not remove the unreplicated state which can belong to a
226+
// newer (uninitialized) replica.
227+
func RemoveStaleRHSFromSplit(
228+
ctx context.Context,
229+
reader storage.Reader,
230+
writer storage.Writer,
231+
rangeID roachpb.RangeID,
232+
keys roachpb.RSpan,
233+
) error {
234+
return ClearRangeData(ctx, rangeID, reader, writer, ClearRangeDataOptions{
235+
// Since the RHS replica is uninitalized, we know there isn't anything in
236+
// the two replicated spans below, before the current batch. Setting these
237+
// options will in effect only clear the writes to the RHS replicated state
238+
// staged in the batch.
239+
ClearReplicatedBySpan: keys,
240+
ClearReplicatedByRangeID: true,
241+
// TODO(tbg): we don't actually want to touch the raft state of the RHS
242+
// replica since it's absent or a more recent one than in the split. Now
243+
// that we have a bool targeting unreplicated RangeID-local keys, we can set
244+
// it to false and remove the HardState+ReplicaID write-back in the caller.
245+
// However, there can be historical split proposals with the
246+
// RaftTruncatedState key set in splitTriggerHelper[^1]. We must first make
247+
// sure that such proposals no longer exist, e.g. with a below-raft
248+
// migration.
249+
//
250+
// [^1]: https://github.com/cockroachdb/cockroach/blob/f263a765d750e41f2701da0a923a6e92d09159fa/pkg/kv/kvserver/batcheval/cmd_end_transaction.go#L1109-L1149
251+
//
252+
// See also: https://github.com/cockroachdb/cockroach/issues/94933
253+
ClearUnreplicatedByRangeID: true,
254+
})
255+
}

pkg/kv/kvserver/store_split.go

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -98,28 +98,9 @@ func splitPreApply(
9898
// racing replica creation for a higher ReplicaID, and it can subsequently
9999
// update its HardState. Here, we can accidentally clear the HardState of
100100
// that new replica.
101-
if err := kvstorage.ClearRangeData(ctx, split.RightDesc.RangeID, readWriter, readWriter, kvstorage.ClearRangeDataOptions{
102-
// We know there isn't anything in these two replicated spans below in the
103-
// right-hand side (before the current batch), so setting these options
104-
// will in effect only clear the writes to the RHS replicated state we have
105-
// staged in this batch, which is what we're after.
106-
ClearReplicatedBySpan: split.RightDesc.RSpan(),
107-
ClearReplicatedByRangeID: true,
108-
// See the HardState write-back dance above and below.
109-
//
110-
// TODO(tbg): we don't actually want to touch the raft state of the RHS
111-
// replica since it's absent or a more recent one than in the split. Now
112-
// that we have a bool targeting unreplicated RangeID-local keys, we can
113-
// set it to false and remove the HardState+ReplicaID write-back. However,
114-
// there can be historical split proposals with the RaftTruncatedState key
115-
// set in splitTriggerHelper[^1]. We must first make sure that such
116-
// proposals no longer exist, e.g. with a below-raft migration.
117-
//
118-
// [^1]: https://github.com/cockroachdb/cockroach/blob/f263a765d750e41f2701da0a923a6e92d09159fa/pkg/kv/kvserver/batcheval/cmd_end_transaction.go#L1109-L1149
119-
//
120-
// See also: https://github.com/cockroachdb/cockroach/issues/94933
121-
ClearUnreplicatedByRangeID: true,
122-
}); err != nil {
101+
if err := kvstorage.RemoveStaleRHSFromSplit(
102+
ctx, readWriter, readWriter, split.RightDesc.RangeID, split.RightDesc.RSpan(),
103+
); err != nil {
123104
log.KvExec.Fatalf(ctx, "failed to clear range data for removed rhs: %v", err)
124105
}
125106
if rightRepl != nil {

0 commit comments

Comments
 (0)