diff --git a/pkg/keys/sql.go b/pkg/keys/sql.go index 8b21314b86de..750b764ad07a 100644 --- a/pkg/keys/sql.go +++ b/pkg/keys/sql.go @@ -121,7 +121,7 @@ func (e sqlEncoder) TenantPrefix() roachpb.Key { return *e.buf } -// TenantPrefix returns the key prefix used for the tenants's data. +// TenantSpan returns a span representing the tenant's keyspace. func (e sqlEncoder) TenantSpan() roachpb.Span { key := *e.buf return roachpb.Span{Key: key, EndKey: key.PrefixEnd()} diff --git a/pkg/multitenant/mtinfopb/info.go b/pkg/multitenant/mtinfopb/info.go index f9feec9c5ca7..aed7a445c1d0 100644 --- a/pkg/multitenant/mtinfopb/info.go +++ b/pkg/multitenant/mtinfopb/info.go @@ -96,7 +96,7 @@ var TenantDataStateValues = map[string]TenantDataState{ // TenantInfo captures both a ProtoInfo and the SQLInfo columns that // go alongside it, sufficient to represent an entire row in -// system.tenans. +// system.tenants. type TenantInfo struct { ProtoInfo SQLInfo diff --git a/pkg/server/BUILD.bazel b/pkg/server/BUILD.bazel index e22e270f7724..1a8fba4da9f1 100644 --- a/pkg/server/BUILD.bazel +++ b/pkg/server/BUILD.bazel @@ -260,7 +260,6 @@ go_library( "//pkg/storage", "//pkg/storage/enginepb", "//pkg/storage/fs", - "//pkg/testutils", "//pkg/testutils/serverutils", "//pkg/testutils/skip", "//pkg/ts", diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index 4d23548feeaa..ec5198450b9f 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -50,7 +50,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" "github.com/cockroachdb/cockroach/pkg/storage" - "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/ts" @@ -62,7 +61,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/log/severity" "github.com/cockroachdb/cockroach/pkg/util/metric" addrutil "github.com/cockroachdb/cockroach/pkg/util/netutil/addr" - "github.com/cockroachdb/cockroach/pkg/util/rangedesc" "github.com/cockroachdb/cockroach/pkg/util/retry" "github.com/cockroachdb/cockroach/pkg/util/stop" "github.com/cockroachdb/cockroach/pkg/util/tracing" @@ -820,31 +818,6 @@ func (t *TestTenant) SettingsWatcher() interface{} { return t.SQLServer.settingsWatcher } -// WaitForTenantEndKeySplit is part of the TestTenantInterface. -func (t *TestTenant) WaitForTenantEndKeySplit(ctx context.Context) error { - // Wait until the tenant end key split happens. - return testutils.SucceedsWithinError(func() error { - factory := t.RangeDescIteratorFactory().(rangedesc.IteratorFactory) - - iterator, err := factory.NewIterator(ctx, t.Codec().TenantSpan()) - if err != nil { - return err - } - if !iterator.Valid() { - return errors.New("range iterator has no ranges") - } - - for iterator.Valid() { - rangeDesc := iterator.CurRangeDescriptor() - if rangeDesc.EndKey.Compare(roachpb.RKeyMax) == 0 { - return errors.Newf("range ID %d end key not split", rangeDesc.RangeID) - } - iterator.Next() - } - return nil - }, 10*time.Second) -} - // StartSharedProcessTenant is part of TestServerInterface. func (ts *TestServer) StartSharedProcessTenant( ctx context.Context, args base.TestSharedProcessTenantArgs, @@ -1668,12 +1641,6 @@ func (ts *TestServer) Tracer() *tracing.Tracer { return ts.node.storeCfg.AmbientCtx.Tracer } -// WaitForTenantEndKeySplit is part of the TestTenantInterface. -func (ts *TestServer) WaitForTenantEndKeySplit(context.Context) error { - // Does not apply to system tenant. - return nil -} - // ForceTableGC is part of TestServerInterface. func (ts *TestServer) ForceTableGC( ctx context.Context, database, table string, timestamp hlc.Timestamp, diff --git a/pkg/sql/catalog/bootstrap/metadata.go b/pkg/sql/catalog/bootstrap/metadata.go index c84dcf7bee55..6ebcb022e7e9 100644 --- a/pkg/sql/catalog/bootstrap/metadata.go +++ b/pkg/sql/catalog/bootstrap/metadata.go @@ -232,14 +232,16 @@ func (ms MetadataSchema) GetInitialValues() ([]roachpb.KeyValue, []roachpb.RKey) // boundaries. In fact, if we tried to split at table boundaries, those // splits would quickly be merged away. The only enforced split points are // between secondary tenants (e.g. between /tenant/ and /tenant/). - // So we drop all descriptor split points and replace it with a single split - // point at the beginning of this tenant's keyspace. + // So we drop all descriptor split points and replace them with split points + // at the beginning and end of this tenant's keyspace. if ms.codec.ForSystemTenant() { for _, id := range ms.otherSplitIDs { splits = append(splits, roachpb.RKey(ms.codec.TablePrefix(id))) } } else { - splits = []roachpb.RKey{roachpb.RKey(ms.codec.TenantPrefix())} + tenantStartKey := roachpb.RKey(ms.codec.TenantPrefix()) + tenantEndKey := tenantStartKey.PrefixEnd() + splits = []roachpb.RKey{tenantStartKey, tenantEndKey} } // Other key/value generation that doesn't fit into databases and @@ -273,11 +275,16 @@ func InitialValuesToString(ms MetadataSchema) string { for _, kv := range kvs { records = append(records, record{k: kv.Key, v: kv.Value.TagAndDataBytes()}) } + p := ms.codec.TenantPrefix() + pNext := p.PrefixEnd() for _, s := range splits { + // Filter out the tenant end key because it does not have the same prefix. + if bytes.HasPrefix(s, pNext) { + continue + } records = append(records, record{k: s}) } // Strip the tenant prefix if there is one. - p := []byte(ms.codec.TenantPrefix()) for i, r := range records { if !bytes.Equal(p, r.k[:len(p)]) { panic("unexpected prefix") @@ -340,6 +347,10 @@ func InitialValuesFromString( kvs = append(kvs, kv) } } + // Add back the filtered out tenant end key. + if !codec.ForSystemTenant() { + splits = append(splits, roachpb.RKey(p.PrefixEnd())) + } return kvs, splits, nil } diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index 934549f91f23..f9d1dc3efa07 100644 --- a/pkg/sql/logictest/logic.go +++ b/pkg/sql/logictest/logic.go @@ -1466,9 +1466,6 @@ func (t *logicTest) newCluster( if err != nil { t.rootT.Fatalf("%+v", err) } - if err := tenant.WaitForTenantEndKeySplit(context.Background()); err != nil { - t.rootT.Fatalf("%+v", err) - } t.tenantAddrs[i] = tenant.SQLAddr() } diff --git a/pkg/sql/multitenant_admin_function_test.go b/pkg/sql/multitenant_admin_function_test.go index c5b8474a166c..3d0ef11a2dd9 100644 --- a/pkg/sql/multitenant_admin_function_test.go +++ b/pkg/sql/multitenant_admin_function_test.go @@ -258,7 +258,6 @@ func (tc testCase) runTest( testServer.Stopper(), ) - var secondaryTenants []serverutils.TestTenantInterface createSecondaryDB := func( tenantID roachpb.TenantID, clusterSettings ...*settings.BoolSetting, @@ -270,13 +269,12 @@ func (tc testCase) runTest( clusterSetting.Override(ctx, &testingClusterSettings.SV, true) } } - tenant, db := serverutils.StartTenant( + _, db := serverutils.StartTenant( t, testServer, base.TestTenantArgs{ Settings: testingClusterSettings, TenantID: tenantID, }, ) - secondaryTenants = append(secondaryTenants, tenant) return db } @@ -339,12 +337,6 @@ func (tc testCase) runTest( fn() } - // Wait for splits after starting all tenants to make test start up faster. - for _, tenant := range secondaryTenants { - err := tenant.WaitForTenantEndKeySplit(ctx) - require.NoError(t, err) - } - execQueries(testCluster, systemDB, "system", tc.system) if tc.secondary.isSet() { execQueries(testCluster, secondaryDB, "secondary", tc.secondary) diff --git a/pkg/sql/sem/builtins/builtins.go b/pkg/sql/sem/builtins/builtins.go index 34b2a1094b6b..f7308fec7ef4 100644 --- a/pkg/sql/sem/builtins/builtins.go +++ b/pkg/sql/sem/builtins/builtins.go @@ -7703,7 +7703,7 @@ expires until the statement bundle is collected`, }, types.String, "Removes constants from a SQL statement. String provided must contain at most "+ - "1 statement.", + "1 statement. (Hint: one way to easily quote arbitrary SQL is to use dollar-quotes.)", volatility.Immutable, ), tree.Overload{ @@ -7740,7 +7740,8 @@ expires until the statement bundle is collected`, return result, nil }, Info: "Hide constants for each element in an array of SQL statements. " + - "Note that maximum 1 statement is permitted per string element.", + "Note that maximum 1 statement is permitted per string element. (Hint: one way to easily " + + "quote arbitrary SQL is to use dollar-quotes.)", Volatility: volatility.Immutable, }, ), @@ -7779,7 +7780,8 @@ expires until the statement bundle is collected`, }, types.String, "Surrounds constants in SQL statement with redaction markers. String provided must "+ - "contain at most 1 statement.", + "contain at most 1 statement. (Hint: one way to easily quote arbitrary SQL is to use "+ + "dollar-quotes.)", volatility.Immutable, ), tree.Overload{ @@ -7816,7 +7818,8 @@ expires until the statement bundle is collected`, return result, nil }, Info: "Surrounds constants with redaction markers for each element in an array of SQL " + - "statements. Note that maximum 1 statement is permitted per string element.", + "statements. Note that maximum 1 statement is permitted per string element. (Hint: one " + + "way to easily quote arbitrary SQL is to use dollar-quotes.)", Volatility: volatility.Immutable, }, ), diff --git a/pkg/sql/tests/testdata/initial_keys b/pkg/sql/tests/testdata/initial_keys index c85b8fb8fb2b..9aefffd6be48 100644 --- a/pkg/sql/tests/testdata/initial_keys +++ b/pkg/sql/tests/testdata/initial_keys @@ -275,8 +275,9 @@ initial-keys tenant=5 /Tenant/5/NamespaceTable/30/1/1/29/"web_sessions"/4/1 /Tenant/5/NamespaceTable/30/1/1/29/"zones"/4/1 /Tenant/5/Table/48/1/0/0 -1 splits: +2 splits: /Tenant/5 + /Tenant/6 initial-keys tenant=999 ---- @@ -377,5 +378,6 @@ initial-keys tenant=999 /Tenant/999/NamespaceTable/30/1/1/29/"web_sessions"/4/1 /Tenant/999/NamespaceTable/30/1/1/29/"zones"/4/1 /Tenant/999/Table/48/1/0/0 -1 splits: +2 splits: /Tenant/999 + /Tenant/1000 diff --git a/pkg/testutils/serverutils/test_tenant_shim.go b/pkg/testutils/serverutils/test_tenant_shim.go index 4fa0e21e0051..7d367f7ff2de 100644 --- a/pkg/testutils/serverutils/test_tenant_shim.go +++ b/pkg/testutils/serverutils/test_tenant_shim.go @@ -185,14 +185,6 @@ type TestTenantInterface interface { // Tracer returns a reference to the tenant's Tracer. Tracer() *tracing.Tracer - // WaitForTenantEndKeySplit blocks until the tenant's initial range is split - // at the end key. For example, this will wait until tenant 10 has a split at - // /Tenant/11. - // - // Tests that use crdb_internal.ranges, crdb_internal.ranges_no_leases, or - // SHOW RANGES from a secondary tenant should call this to avoid races. - WaitForTenantEndKeySplit(ctx context.Context) error - // MigrationServer returns the tenant's migration server, which is used in // upgrade testing. MigrationServer() interface{}