Skip to content

Commit 352bdf4

Browse files
andreimateiandreimatei
committed
storage: start with all the system ranges at bootstrap
Before this patch, a store would be bootstrapped with a single range, and then we'd rely on the split queue to create splits at a statically defined list of keys and between system tables. This patch changes the bootstrapping so that the store starts up with all these ranges and we don't rely on the split queue any more. This simplifies thigs: less moving pieces for new clusters. Besides that, there was a problem before since all the ranges deriving from the original mother one were starting with expiration-based leases. Once those expired, most of them would transition to epoch-based leases. That transition is disruptive - clears the timestamp cache and such - and so transactions running at those times (4.5s after cluster startup) would incur restarts. This was a problem for tests. Also, the fact that ranges were starting up with expiration-based leases was also a problem for rangefeed tests since rangefeeds don't work on those ranges (as I understand it). As a result of this patch, a bunch of different tests now run with more realistic stores: there are a million ways to create stores in tests and most of them before were just getting a single range; now most get many. Fixes cockroachdb#32495 Fixes cockroachdb#31182 Fixes cockroachdb#31065 Release note: None
1 parent 046cca2 commit 352bdf4

58 files changed

Lines changed: 1671 additions & 941 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎pkg/base/test_server_args.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,5 +172,6 @@ const (
172172
// ReplicationManual means that the split and replication queues of all
173173
// servers are stopped, and the test must manually control splitting and
174174
// replication through the TestServer.
175+
// Note that the server starts with a number of system ranges.
175176
ReplicationManual
176177
)

‎pkg/ccl/changefeedccl/changefeed_test.go‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,6 @@ func TestChangefeedTimestamps(t *testing.T) {
207207
defer leaktest.AfterTest(t)()
208208

209209
testFn := func(t *testing.T, db *gosql.DB, f testfeedFactory) {
210-
// HACK: remove this once #32495 is fixed.
211-
maybeWaitForEpochLeases(t, f.Server())
212-
213210
ctx := context.Background()
214211
sqlDB := sqlutils.MakeSQLRunner(db)
215212
sqlDB.Exec(t, `CREATE TABLE foo (a INT PRIMARY KEY)`)
@@ -299,9 +296,6 @@ func TestChangefeedResolvedFrequency(t *testing.T) {
299296
defer leaktest.AfterTest(t)()
300297

301298
testFn := func(t *testing.T, db *gosql.DB, f testfeedFactory) {
302-
// HACK: remove this once #32495 is fixed.
303-
maybeWaitForEpochLeases(t, f.Server())
304-
305299
sqlDB := sqlutils.MakeSQLRunner(db)
306300
sqlDB.Exec(t, `CREATE TABLE foo (a INT PRIMARY KEY)`)
307301

@@ -577,9 +571,6 @@ func TestChangefeedSchemaChangeAllowBackfill(t *testing.T) {
577571
defer leaktest.AfterTest(t)()
578572

579573
testFn := func(t *testing.T, db *gosql.DB, f testfeedFactory) {
580-
// HACK: remove this once #32495 is fixed.
581-
maybeWaitForEpochLeases(t, f.Server())
582-
583574
sqlDB := sqlutils.MakeSQLRunner(db)
584575

585576
t.Run(`add column with default`, func(t *testing.T) {
@@ -905,9 +896,6 @@ func TestChangefeedMonitoring(t *testing.T) {
905896
defer leaktest.AfterTest(t)()
906897

907898
testFn := func(t *testing.T, db *gosql.DB, f testfeedFactory) {
908-
// HACK: remove this once #32495 is fixed.
909-
maybeWaitForEpochLeases(t, f.Server())
910-
911899
beforeEmitRowCh := make(chan struct{}, 2)
912900
knobs := f.Server().(*server.TestServer).Cfg.TestingKnobs.
913901
DistSQL.(*distsqlrun.TestingKnobs).
@@ -1131,9 +1119,6 @@ func TestChangefeedDataTTL(t *testing.T) {
11311119
defer leaktest.AfterTest(t)()
11321120

11331121
testFn := func(t *testing.T, db *gosql.DB, f testfeedFactory) {
1134-
// HACK: remove this once #32495 is fixed.
1135-
maybeWaitForEpochLeases(t, f.Server())
1136-
11371122
// Set a very simple channel-based, wait-and-resume function as the
11381123
// BeforeEmitRow hook.
11391124
var shouldWait int32

‎pkg/ccl/changefeedccl/helpers_test.go‎

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import (
3535
"github.com/cockroachdb/cockroach/pkg/sql/parser"
3636
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
3737
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
38-
"github.com/cockroachdb/cockroach/pkg/storage"
3938
"github.com/cockroachdb/cockroach/pkg/testutils"
4039
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
4140
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
@@ -670,62 +669,6 @@ func expectResolvedTimestamp(t testing.TB, f testfeed) hlc.Timestamp {
670669
return parseTimeToHLC(t, valueRaw.CRDB.Resolved)
671670
}
672671

673-
// maybeWaitForEpochLeases waits until all ranges serving user table data have
674-
// epoch leases.
675-
//
676-
// Changefeed resolved timestamps rely on RangeFeed checkpoints which rely on
677-
// closed timestamps which only work with epoch leases. This means that any
678-
// changefeed test that _uses RangeFeed_ and _needs resolved timestamps_ should
679-
// first wait until all the relevant ranges have epoch-leases.
680-
//
681-
// We added this to unblock RangeFeed work, but it takes ~10s, so we should fix
682-
// it for real at some point. The permanent fix is being tracked in #32495.
683-
func maybeWaitForEpochLeases(t *testing.T, s serverutils.TestServerInterface) {
684-
// If it's not a rangefeed test, don't bother waiting.
685-
if !strings.Contains(t.Name(), `rangefeed`) {
686-
return
687-
}
688-
689-
userTablesSpan := roachpb.RSpan{
690-
Key: roachpb.RKey(keys.MakeTablePrefix(keys.MinUserDescID)),
691-
EndKey: roachpb.RKeyMax,
692-
}
693-
testutils.SucceedsSoon(t, func() error {
694-
ctx := context.Background()
695-
var rangeDescs []roachpb.RangeDescriptor
696-
if err := s.DB().Txn(ctx, func(ctx context.Context, txn *client.Txn) error {
697-
var err error
698-
rangeDescs, err = allRangeDescriptors(ctx, txn)
699-
return err
700-
}); err != nil {
701-
return err
702-
}
703-
704-
// Force a lease acquisition so we don't get stuck waiting forever.
705-
if _, err := s.DB().Scan(ctx, userTablesSpan.Key, userTablesSpan.EndKey, 0); err != nil {
706-
return err
707-
}
708-
709-
stores := s.GetStores().(*storage.Stores)
710-
for _, rangeDesc := range rangeDescs {
711-
if !rangeDesc.ContainsKeyRange(userTablesSpan.Key, userTablesSpan.EndKey) {
712-
continue
713-
}
714-
replica, err := stores.GetReplicaForRangeID(rangeDesc.RangeID)
715-
if err != nil {
716-
return err
717-
}
718-
lease, _ := replica.GetLease()
719-
if lease.Epoch == 0 {
720-
err := errors.Errorf("%s does not have an epoch lease: should resolve in %s",
721-
rangeDesc, lease.Expiration.GoTime().Sub(timeutil.Now()))
722-
return err
723-
}
724-
}
725-
return nil
726-
})
727-
}
728-
729672
func sinklessTest(testFn func(*testing.T, *gosql.DB, testfeedFactory)) func(*testing.T) {
730673
return func(t *testing.T) {
731674
ctx := context.Background()

‎pkg/ccl/changefeedccl/validations_test.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@ func TestValidations(t *testing.T) {
3232
if strings.Contains(t.Name(), `rangefeed`) {
3333
t.Skip(`#32946`)
3434
}
35-
// HACK: remove this once #32495 is fixed.
36-
maybeWaitForEpochLeases(t, f.Server())
37-
3835
sqlDB := sqlutils.MakeSQLRunner(db)
3936

4037
t.Run("bank", func(t *testing.T) {

‎pkg/cli/cli_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2069,16 +2069,16 @@ func checkNodeStatus(t *testing.T, c cliTest, output string, start time.Time) {
20692069
// address. They don't need closer checks.
20702070
baseIdx := len(baseNodeColumnHeaders)
20712071

2072-
// Adding fields that need verification for --range flag.
2072+
// Adding fields that need verification for --ranges flag.
20732073
// We have to allow up to 1 unavailable/underreplicated range because
20742074
// sometimes we run the `node status` command before the server has fully
20752075
// initialized itself and it doesn't consider itself live yet. In such cases,
20762076
// there will only be one range covering the entire keyspace because it won't
20772077
// have been able to do any splits yet.
20782078
if nodeCtx.statusShowRanges || nodeCtx.statusShowAll {
20792079
testcases = append(testcases,
2080-
testCase{"leader_ranges", baseIdx, 3},
2081-
testCase{"leaseholder_ranges", baseIdx + 1, 3},
2080+
testCase{"leader_ranges", baseIdx, 22},
2081+
testCase{"leaseholder_ranges", baseIdx + 1, 22},
20822082
testCase{"ranges", baseIdx + 2, 22},
20832083
testCase{"unavailable_ranges", baseIdx + 3, 1},
20842084
testCase{"underreplicated_ranges", baseIdx + 4, 1},

‎pkg/config/system.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,13 +319,17 @@ var staticSplits = []roachpb.RKey{
319319
}
320320

321321
// StaticSplits are predefined split points in the system keyspace.
322+
// Corresponding ranges are created at cluster bootstrap time.
322323
//
323324
// There are two reasons for a static split. First, spans that are critical to
324325
// cluster stability, like the node liveness span, are split into their own
325326
// ranges to ease debugging (see #17297). Second, spans in the system keyspace
326327
// that can be targeted by zone configs, like the meta span and the timeseries
327328
// span, are split off into their own ranges because zone configs cannot apply
328329
// to fractions of a range.
330+
//
331+
// Note that these are not the only splits created at cluster bootstrap; splits
332+
// between various system tables are also created.
329333
func StaticSplits() []roachpb.RKey {
330334
return staticSplits
331335
}
@@ -338,10 +342,14 @@ func StaticSplits() []roachpb.RKey {
338342
// system ranges that come before the system tables. The system-config range is
339343
// somewhat special in that it can contain multiple SQL tables
340344
// (/table/0-/table/<max-system-config-desc>) within a single range.
341-
func (s *SystemConfig) ComputeSplitKey(startKey, endKey roachpb.RKey) roachpb.RKey {
345+
func (s *SystemConfig) ComputeSplitKey(startKey, endKey roachpb.RKey) (rr roachpb.RKey) {
342346
// Before dealing with splits necessitated by SQL tables, handle all of the
343347
// static splits earlier in the keyspace. Note that this list must be kept in
344348
// the proper order (ascending in the keyspace) for the logic below to work.
349+
//
350+
// For new clusters, the static splits correspond to ranges created at
351+
// bootstrap time. Older stores might be used with a version with more
352+
// staticSplits though, in which case this code is useful.
345353
for _, split := range staticSplits {
346354
if startKey.Less(split) {
347355
if split.Less(endKey) {

‎pkg/config/system_test.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ func TestGetLargestID(t *testing.T) {
167167
ms := sqlbase.MakeMetadataSchema()
168168
descIDs := ms.DescriptorIDs()
169169
maxDescID := descIDs[len(descIDs)-1]
170-
return testCase{ms.GetInitialValues(), uint32(maxDescID), 0, ""}
170+
kvs, _ /* splits */ := ms.GetInitialValues()
171+
return testCase{kvs, uint32(maxDescID), 0, ""}
171172
}(),
172173

173174
// Test non-zero max.
@@ -259,8 +260,9 @@ func TestComputeSplitKeySystemRanges(t *testing.T) {
259260
}
260261

261262
cfg := config.NewSystemConfig()
263+
kvs, _ /* splits */ := sqlbase.MakeMetadataSchema().GetInitialValues()
262264
cfg.SystemConfigEntries = config.SystemConfigEntries{
263-
Values: sqlbase.MakeMetadataSchema().GetInitialValues(),
265+
Values: kvs,
264266
}
265267
for tcNum, tc := range testCases {
266268
splitKey := cfg.ComputeSplitKey(tc.start, tc.end)
@@ -290,10 +292,10 @@ func TestComputeSplitKeyTableIDs(t *testing.T) {
290292

291293
schema := sqlbase.MakeMetadataSchema()
292294
// Real system tables only.
293-
baseSql := schema.GetInitialValues()
295+
baseSql, _ /* splits */ := schema.GetInitialValues()
294296
// Real system tables plus some user stuff.
295-
userSQL := append(schema.GetInitialValues(),
296-
descriptor(start), descriptor(start+1), descriptor(start+5))
297+
kvs, _ /* splits */ := schema.GetInitialValues()
298+
userSQL := append(kvs, descriptor(start), descriptor(start+1), descriptor(start+5))
297299
// Real system tables and partitioned user tables.
298300
subzoneSQL := append(userSQL,
299301
zoneConfig(start+1, subzone("a", ""), subzone("c", "e")),
@@ -419,8 +421,10 @@ func TestGetZoneConfigForKey(t *testing.T) {
419421
config.ZoneConfigHook = originalZoneConfigHook
420422
}()
421423
cfg := config.NewSystemConfig()
424+
425+
kvs, _ /* splits */ := sqlbase.MakeMetadataSchema().GetInitialValues()
422426
cfg.SystemConfigEntries = config.SystemConfigEntries{
423-
Values: sqlbase.MakeMetadataSchema().GetInitialValues(),
427+
Values: kvs,
424428
}
425429
for tcNum, tc := range testCases {
426430
var objectID uint32

‎pkg/keys/constants.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,3 +335,9 @@ const (
335335
TableCommentType = 1
336336
ColumnCommentType = 2
337337
)
338+
339+
// PseudoTableIDs is the list of ids from above that are not real tables (i.e.
340+
// there's no table descriptor). They're grouped here because the cluster
341+
// bootstrap process needs to create splits for them; splits for the tables
342+
// happen separately.
343+
var PseudoTableIDs = []uint32{MetaRangesID, SystemRangesID, TimeseriesRangesID, LivenessRangesID}

‎pkg/kv/txn_coord_sender_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1082,7 +1082,8 @@ func setupMetricsTest(t *testing.T) (*localtestcluster.LocalTestCluster, TxnMetr
10821082
s := &localtestcluster.LocalTestCluster{
10831083
DBContext: &dbCtx,
10841084
// Liveness heartbeat txns mess up the metrics.
1085-
DontStartLivenessHeartbeat: true,
1085+
DisableLivenessHeartbeat: true,
1086+
DontCreateSystemRanges: true,
10861087
}
10871088
s.Start(t, testutils.NewNodeTestBaseContext(), InitFactoryForLocalTestCluster)
10881089

‎pkg/server/admin_test.go‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import (
4444
"github.com/cockroachdb/cockroach/pkg/sql"
4545
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
4646
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
47+
"github.com/cockroachdb/cockroach/pkg/storage"
4748
"github.com/cockroachdb/cockroach/pkg/storage/storagepb"
4849
"github.com/cockroachdb/cockroach/pkg/testutils"
4950
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
@@ -1406,14 +1407,48 @@ func TestAdminAPIRangeLogByRangeID(t *testing.T) {
14061407
// TestAdminAPIRangeLogByRangeID).
14071408
func TestAdminAPIFullRangeLog(t *testing.T) {
14081409
defer leaktest.AfterTest(t)()
1409-
s, _, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
1410+
s, db, _ := serverutils.StartServer(t,
1411+
base.TestServerArgs{
1412+
Knobs: base.TestingKnobs{
1413+
Store: &storage.StoreTestingKnobs{
1414+
DisableSplitQueue: true,
1415+
},
1416+
},
1417+
})
14101418
defer s.Stopper().Stop(context.Background())
14111419

1412-
expectedRanges, err := ExpectedInitialRangeCount(kvDB)
1420+
// Insert something in the rangelog table, otherwise it's empty for new
1421+
// clusters.
1422+
rows, err := db.Query(`SELECT count(1) FROM system.rangelog`)
14131423
if err != nil {
14141424
t.Fatal(err)
14151425
}
1416-
expectedEvents := expectedRanges - 1 // one for each split
1426+
if !rows.Next() {
1427+
t.Fatal("missing row")
1428+
}
1429+
var cnt int
1430+
if err := rows.Scan(&cnt); err != nil {
1431+
t.Fatal(err)
1432+
}
1433+
if err := rows.Close(); err != nil {
1434+
t.Fatal(err)
1435+
}
1436+
if cnt != 0 {
1437+
t.Fatalf("expected 0 rows in system.rangelog, found: %d", cnt)
1438+
}
1439+
const rangeID = 100
1440+
for i := 0; i < 10; i++ {
1441+
if _, err := db.Exec(
1442+
`INSERT INTO system.rangelog (
1443+
timestamp, "rangeID", "storeID", "eventType"
1444+
) VALUES (now(), $1, 1, $2)`,
1445+
rangeID,
1446+
storagepb.RangeLogEventType_add.String(),
1447+
); err != nil {
1448+
t.Fatal(err)
1449+
}
1450+
}
1451+
expectedEvents := 10
14171452

14181453
testCases := []struct {
14191454
hasLimit bool
@@ -1436,8 +1471,13 @@ func TestAdminAPIFullRangeLog(t *testing.T) {
14361471
if err := getAdminJSONProto(s, url, &resp); err != nil {
14371472
t.Fatal(err)
14381473
}
1439-
if e, a := tc.expected, len(resp.Events); e != a {
1440-
t.Fatalf("expected %d events, got %d", e, a)
1474+
events := resp.Events
1475+
if e, a := tc.expected, len(events); e != a {
1476+
var sb strings.Builder
1477+
for _, ev := range events {
1478+
sb.WriteString(ev.String() + "\n")
1479+
}
1480+
t.Fatalf("expected %d events, got %d:\n%s", e, a, sb.String())
14411481
}
14421482
})
14431483
}

0 commit comments

Comments
 (0)