Skip to content

Commit

Permalink
kvserver: fix TestStoreRangeSplitAndMergeWithGlobalReads
Browse files Browse the repository at this point in the history
Directly set the span config for the range under test rather than
setting the ZoneConfig and waiting for it to propagate. In addition to
simplifying the test it also makes it run faster.

Fixes: cockroachdb#119230

Epic: none

Release note: None
  • Loading branch information
andrewbaptist committed Apr 22, 2024
1 parent aa241ba commit 4305c28
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 22 deletions.
1 change: 1 addition & 0 deletions pkg/kv/kvserver/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ go_test(
"//pkg/spanconfig",
"//pkg/spanconfig/spanconfigptsreader",
"//pkg/spanconfig/spanconfigstore",
"//pkg/spanconfig/spanconfigtestutils",
"//pkg/sql",
"//pkg/sql/catalog/bootstrap",
"//pkg/sql/catalog/catalogkeys",
Expand Down
43 changes: 21 additions & 22 deletions pkg/kv/kvserver/client_split_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/spanconfig"
"github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigtestutils"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema"
"github.com/cockroachdb/cockroach/pkg/sql/sqlstats"
Expand Down Expand Up @@ -4007,8 +4008,6 @@ func TestStoreRangeSplitAndMergeWithGlobalReads(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

skip.WithIssue(t, 119230)

// Detect splits and merges over the global read ranges. Assert that the split
// and merge transactions commit with pushed write timestamps, and that the
// commit-wait sleep for these transactions is performed before running their
Expand Down Expand Up @@ -4043,13 +4042,29 @@ func TestStoreRangeSplitAndMergeWithGlobalReads(t *testing.T) {
return nil
}

descID := bootstrap.TestingUserDescID(0)
descKey := keys.SystemSQLCodec.TablePrefix(descID)
splitKey := append(descKey, []byte("split")...)

// Set global reads for the test ranges.
spanConfig := zonepb.DefaultZoneConfig().AsSpanConfig()
spanConfig.GlobalReads = true

ctx := context.Background()
s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{
Knobs: base.TestingKnobs{
Store: &kvserver.StoreTestingKnobs{
DisableMergeQueue: true,
TestingResponseFilter: respFilter,
},
SpanConfig: &spanconfig.TestingKnobs{
StoreKVSubscriberOverride: func(original spanconfig.KVSubscriber) spanconfig.KVSubscriber {
wrapped := spanconfigtestutils.NewWrappedKVSubscriber(original)
wrapped.AddOverride(descKey, spanConfig)
wrapped.AddOverride(splitKey, spanConfig)
return wrapped
},
},
},
})

Expand All @@ -4062,11 +4077,8 @@ func TestStoreRangeSplitAndMergeWithGlobalReads(t *testing.T) {
tdb.Exec(t, `SET CLUSTER SETTING kv.rangefeed.closed_timestamp_refresh_interval = '20ms'`)
store, err := s.GetStores().(*kvserver.Stores).GetStore(s.GetFirstStoreID())
require.NoError(t, err)
config.TestingSetupZoneConfigHook(s.Stopper())

// Split off the range for the test.
descID := bootstrap.TestingUserDescID(0)
descKey := keys.SystemSQLCodec.TablePrefix(descID)
splitArgs := adminSplitArgs(descKey)
_, pErr := kv.SendWrapped(ctx, store.TestSender(), splitArgs)
require.Nil(t, pErr)
Expand All @@ -4075,36 +4087,23 @@ func TestStoreRangeSplitAndMergeWithGlobalReads(t *testing.T) {
// response filter.
clockPtr.Store(s.Clock())

// Set global reads.
zoneConfig := zonepb.DefaultZoneConfig()
zoneConfig.GlobalReads = proto.Bool(true)
config.TestingSetZoneConfig(config.ObjectID(descID), zoneConfig)

// Perform a write to the system config span being watched by
// the SystemConfigProvider.
tdb.Exec(t, "CREATE TABLE foo ()")
testutils.SucceedsSoon(t, func() error {
repl := store.LookupReplica(roachpb.RKey(descKey))
if repl.ClosedTimestampPolicy() != roachpb.LEAD_FOR_GLOBAL_READS {
return errors.Errorf("expected LEAD_FOR_GLOBAL_READS policy")
}
return nil
})
// Verify that the closed timestamp policy is set up.
repl := store.LookupReplica(roachpb.RKey(descKey))
require.Equal(t, repl.ClosedTimestampPolicy(), roachpb.LEAD_FOR_GLOBAL_READS)

// Write to the range, which has the effect of bumping the closed timestamp.
pArgs := putArgs(descKey, []byte("foo"))
_, pErr = kv.SendWrapped(ctx, store.TestSender(), pArgs)
require.Nil(t, pErr)

// Split the range. Should succeed.
splitKey := append(descKey, []byte("split")...)
splitArgs = adminSplitArgs(splitKey)
_, pErr = kv.SendWrapped(ctx, store.TestSender(), splitArgs)
require.Nil(t, pErr)
require.Equal(t, int64(1), store.Metrics().CommitWaitsBeforeCommitTrigger.Count())
require.Equal(t, int64(1), atomic.LoadInt64(&splits))

repl := store.LookupReplica(roachpb.RKey(splitKey))
repl = store.LookupReplica(roachpb.RKey(splitKey))
require.Equal(t, splitKey, repl.Desc().StartKey.AsRawKey())

// Merge the range. Should succeed.
Expand Down

0 comments on commit 4305c28

Please sign in to comment.