kv: add TestStoreRangeSplitAndMergeWithGlobalReads #60753
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @nvanbenschoten)
pkg/kv/kvserver/replica.go, line 727 at r1 (raw file):
// ClosedTimestampPolicy returns the closed timestamp policy of the range, which // is updated asynchronously through gossip of zone configurations. func (r *Replica) ClosedTimestampPolicy() roachpb.RangeClosedTimestampPolicy {
Put this in a testing file until it's used in production code?
pkg/kv/kvserver/batcheval/cmd_subsume.go, line 96 at r1 (raw file):
// a deletion intent on the local range descriptor. descKey := keys.RangeDescriptorKey(desc.StartKey) _, intent, err := storage.MVCCGet(ctx, readWriter, descKey, hlc.MaxTimestamp,
Does this choice of timestamp deserve commentary?
Made possible by cockroachdb#60567. This commit adds a new test called TestStoreRangeSplitAndMergeWithGlobalReads that tests that a range configured to serve global reads can be split and merged. In essence, this tests whether the split and merge transactions can handle having their timestamp bumped by the closed timestamp on the ranges they're operating on. The test revealed that range merges did have issues in these cases, because SubsumeRequests assumed that the intent on the RHS's local descriptor was below the node's HLC clock. This is no longer always true, so we now perform the inconsistent scan at hlc.MaxTimestamp, just like QueryIntent requests do.
nvanbenschoten
force-pushed
the
nvanbenschoten/splitMergeGlobal
branch
from
7959165
to
1183dc8
Compare
There was a problem hiding this comment.
TFTR!
bors r+
Reviewable status:
pkg/kv/kvserver/replica.go, line 727 at r1 (raw file):
Previously, ajwerner wrote…
// ClosedTimestampPolicy returns the closed timestamp policy of the range, which // is updated asynchronously through gossip of zone configurations. func (r *Replica) ClosedTimestampPolicy() roachpb.RangeClosedTimestampPolicy {Put this in a testing file until it's used in production code?
Done.
pkg/kv/kvserver/batcheval/cmd_subsume.go, line 96 at r1 (raw file):
Previously, ajwerner wrote…
Does this choice of timestamp deserve commentary?
Done.
|
Build succeeded: |
Made possible by #60567.
This commit adds a new test called TestStoreRangeSplitAndMergeWithGlobalReads
that tests that a range configured to serve global reads can be split and
merged. In essence, this tests whether the split and merge transactions can
handle having their timestamp bumped by the closed timestamp on the ranges
they're operating on.
The test revealed that range merges did have issues in these cases, because
SubsumeRequests assumed that the intent on the RHS's local descriptor was below
the node's HLC clock. This is no longer always true, so we now perform the
inconsistent scan at hlc.MaxTimestamp, just like QueryIntent requests do.