Skip to content

Commit

Permalink
server,kvaccessor: record span configs during tenant creation/gc
Browse files Browse the repository at this point in the history
For newly created tenants, we want to ensure hard splits on tenant
boundaries. The source of truth for split points in the span configs
subsystem is the contents of system.span_configurations. To ensure
hard splits, we insert a single key record at the tenant prefix. In a
future commit we'll introduce the spanconfig.Reconciler process, which
runs per tenant and governs all config entries under each tenant's
purview. This has implications for this initial record we're talking
about (this record might get cleaned up for e.g.); we'll explore it in
tests for the Reconciler itself.

Creating a single key record is easy enough -- we could've written
directly to system.span_configurations. When a tenant is GC-ed
however, we need to clear out all tenant state transactionally. To that
end we plumb in a txn-scoped KVAccessor into the planner where
crdb_internal.destroy_tenant is executed. This lets us easily delete
all abandoned tenant span config records.

Note: We get rid of spanconfig.experimental_kvaccessor.enabled. Access
to spanconfigs infrastructure is already sufficiently gated through
the env var. Now that crdb_internal.create_tenant attempts to write
through the KVAccessor, it's cumbersome to have to enable the setting
manually in every multi-tenant test (increasingly the default) enabling
some part of the span configs infrastructure.

---

This commit also needs a migration -- for existing clusters with
secondary tenants, when upgrading we need to install this initial record
at the tenant prefix for all extant tenants (and make sure to continue
doing so for subsequent newly created tenants). This is to preserve the
hard-split-on-tenant-boundary invariant we wish to provide. It's
possible for an upgraded multi-tenant cluster to have dormant sql pods
that have never reconciled. If KV switches over to the span configs
subsystem, splitting only on the contents of system.span_configurations,
we'll fail to split on all tenant boundaries. To this end we introduce
clusterversion.SeedTenantSpanConfigs, which allows us to seed span
config data for secondary tenants. The associated migration seeds
entries for existing tenants.

Release note: None
  • Loading branch information
irfansharif committed Dec 10, 2021
1 parent 884088d commit 29fadd6
Show file tree
Hide file tree
Showing 28 changed files with 589 additions and 129 deletions.
2 changes: 1 addition & 1 deletion docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,4 @@ trace.debug.enable boolean false if set, traces for recent requests can be seen
trace.jaeger.agent string the address of a Jaeger agent to receive traces using the Jaeger UDP Thrift protocol, as <host>:<port>. If no port is specified, 6381 will be used.
trace.opentelemetry.collector string address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.
trace.zipkin.collector string the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.
version version 21.2-26 set the active cluster version in the format '<major>.<minor>'
version version 21.2-30 set the active cluster version in the format '<major>.<minor>'
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,6 @@
<tr><td><code>trace.jaeger.agent</code></td><td>string</td><td><code></code></td><td>the address of a Jaeger agent to receive traces using the Jaeger UDP Thrift protocol, as <host>:<port>. If no port is specified, 6381 will be used.</td></tr>
<tr><td><code>trace.opentelemetry.collector</code></td><td>string</td><td><code></code></td><td>address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.</td></tr>
<tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.</td></tr>
<tr><td><code>version</code></td><td>version</td><td><code>21.2-26</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
<tr><td><code>version</code></td><td>version</td><td><code>21.2-30</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
</tbody>
</table>
1 change: 1 addition & 0 deletions pkg/ccl/kvccl/kvtenantccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ go_library(
"//pkg/config",
"//pkg/config/zonepb",
"//pkg/gossip",
"//pkg/kv",
"//pkg/kv/kvclient/kvcoord:with-mocks",
"//pkg/kv/kvclient/kvtenant",
"//pkg/kv/kvclient/rangecache:with-mocks",
Expand Down
6 changes: 6 additions & 0 deletions pkg/ccl/kvccl/kvtenantccl/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/config"
"github.com/cockroachdb/cockroach/pkg/config/zonepb"
"github.com/cockroachdb/cockroach/pkg/gossip"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord"
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvtenant"
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangecache"
Expand Down Expand Up @@ -453,6 +454,11 @@ func (c *Connector) UpdateSpanConfigEntries(
})
}

// WithTxn implements the spanconfig.KVAccessor interface.
func (c *Connector) WithTxn(*kv.Txn) spanconfig.KVAccessor {
panic("not applicable")
}

// withClient is a convenience wrapper that executes the given closure while
// papering over InternalClient retrieval errors.
func (c *Connector) withClient(
Expand Down
6 changes: 6 additions & 0 deletions pkg/ccl/migrationccl/migrationsccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,24 @@ go_test(
srcs = [
"main_test.go",
"records_based_registry_external_test.go",
"seed_tenant_span_configs_external_test.go",
],
deps = [
"//pkg/base",
"//pkg/ccl/baseccl",
"//pkg/ccl/kvccl/kvtenantccl",
"//pkg/clusterversion",
"//pkg/keys",
"//pkg/roachpb:with-mocks",
"//pkg/security",
"//pkg/security/securitytest",
"//pkg/server",
"//pkg/spanconfig",
"//pkg/testutils/serverutils",
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"//pkg/util/leaktest",
"//pkg/util/log",
"//pkg/util/protoutil",
"@com_github_stretchr_testify//require",
],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
// Copyright 2021 The Cockroach Authors.
//
// Licensed as a CockroachDB Enterprise file under the Cockroach Community
// License (the "License"); you may not use this file except in compliance with
// the License. You may obtain a copy of the License at
//
// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt

package migrationsccl_test

import (
"context"
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
_ "github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvtenantccl"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/spanconfig"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/stretchr/testify/require"
)

// TestPreSeedSpanConfigsWrittenWhenActive tests that seed span configs are
// written to for fresh tenants if the cluster version that introduced it is
// active.
func TestPreSeedSpanConfigsWrittenWhenActive(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

ctx := context.Background()
tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
EnableSpanConfigs: true, // we use spanconfig.KVAccessor to check if its contents are as we'd expect
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
DisableAutomaticVersionUpgrade: 1,
BinaryVersionOverride: clusterversion.ByKey(
clusterversion.PreSeedTenantSpanConfigs,
),
},
},
},
})

defer tc.Stopper().Stop(ctx)
ts := tc.Server(0)

tenantID := roachpb.MakeTenantID(10)
_, err := ts.StartTenant(ctx, base.TestTenantArgs{TenantID: tenantID})
require.NoError(t, err)

scKVAccessor := ts.SpanConfigKVAccessor().(spanconfig.KVAccessor)
tenantPrefix := keys.MakeTenantPrefix(tenantID)
tenantSpan := roachpb.Span{Key: tenantPrefix, EndKey: tenantPrefix.PrefixEnd()}
tenantSeedSpan := roachpb.Span{Key: tenantPrefix, EndKey: tenantPrefix.Next()}

{
entries, err := scKVAccessor.GetSpanConfigEntriesFor(ctx, []roachpb.Span{
tenantSpan,
})
require.NoError(t, err)
require.Len(t, entries, 1)
require.Equal(t, entries[0].Span, tenantSeedSpan)
}
}

// TestSeedTenantSpanConfigs tests that the migration installs relevant seed
// span configs for existing secondary tenants.
func TestSeedTenantSpanConfigs(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

ctx := context.Background()
tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
EnableSpanConfigs: true, // we use spanconfig.KVAccessor to check if its contents are as we'd expect
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
DisableAutomaticVersionUpgrade: 1,
BinaryVersionOverride: clusterversion.ByKey(
clusterversion.PreSeedTenantSpanConfigs - 1,
),
},
},
},
})

defer tc.Stopper().Stop(ctx)
ts := tc.Server(0)
tdb := sqlutils.MakeSQLRunner(tc.ServerConn(0))
scKVAccessor := ts.SpanConfigKVAccessor().(spanconfig.KVAccessor)

tenantID := roachpb.MakeTenantID(10)
tenantPrefix := keys.MakeTenantPrefix(tenantID)
tenantSpan := roachpb.Span{Key: tenantPrefix, EndKey: tenantPrefix.PrefixEnd()}
tenantSeedSpan := roachpb.Span{Key: tenantPrefix, EndKey: tenantPrefix.Next()}
{
_, err := ts.StartTenant(ctx, base.TestTenantArgs{TenantID: tenantID})
require.NoError(t, err)
}

{ // Ensure that no span config entries are to be found
entries, err := scKVAccessor.GetSpanConfigEntriesFor(ctx, []roachpb.Span{
tenantSpan,
})
require.NoError(t, err)
require.Empty(t, entries)
}

tdb.Exec(t,
"SET CLUSTER SETTING version = $1",
clusterversion.ByKey(clusterversion.SeedTenantSpanConfigs).String(),
)

{ // Ensure that the tenant now has a span config entry.
entries, err := scKVAccessor.GetSpanConfigEntriesFor(ctx, []roachpb.Span{
tenantSpan,
})
require.NoError(t, err)
require.Len(t, entries, 1)
require.Equal(t, entries[0].Span, tenantSeedSpan)
}
}

// TestSeedTenantSpanConfigsWithExistingEntry tests that the migration ignores
// tenants with existing span config entries.
func TestSeedTenantSpanConfigsWithExistingEntry(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

ctx := context.Background()
tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
EnableSpanConfigs: true, // we use spanconfig.KVAccessor to check if its contents are as we'd expect
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
DisableAutomaticVersionUpgrade: 1,
BinaryVersionOverride: clusterversion.ByKey(
clusterversion.PreSeedTenantSpanConfigs,
),
},
},
},
})

defer tc.Stopper().Stop(ctx)
ts := tc.Server(0)
tdb := sqlutils.MakeSQLRunner(tc.ServerConn(0))
scKVAccessor := ts.SpanConfigKVAccessor().(spanconfig.KVAccessor)

tenantID := roachpb.MakeTenantID(10)
tenantPrefix := keys.MakeTenantPrefix(tenantID)
tenantSpan := roachpb.Span{Key: tenantPrefix, EndKey: tenantPrefix.PrefixEnd()}
tenantSeedSpan := roachpb.Span{Key: tenantPrefix, EndKey: tenantPrefix.Next()}
{
_, err := ts.StartTenant(ctx, base.TestTenantArgs{TenantID: tenantID})
require.NoError(t, err)
}

{ // Ensure that the tenant already has a span config entry.
entries, err := scKVAccessor.GetSpanConfigEntriesFor(ctx, []roachpb.Span{
tenantSpan,
})
require.NoError(t, err)
require.Len(t, entries, 1)
require.Equal(t, entries[0].Span, tenantSeedSpan)
}

// Ensure the cluster version bump goes through successfully.
tdb.Exec(t,
"SET CLUSTER SETTING version = $1",
clusterversion.ByKey(clusterversion.SeedTenantSpanConfigs).String(),
)

{ // Ensure that the tenant's span config entry stay as it was.
entries, err := scKVAccessor.GetSpanConfigEntriesFor(ctx, []roachpb.Span{
tenantSpan,
})
require.NoError(t, err)
require.Len(t, entries, 1)
require.Equal(t, entries[0].Span, tenantSeedSpan)
}
}
17 changes: 17 additions & 0 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,15 @@ const (
// This version must be active before any ProbeRequest is issued on the
// cluster.
ProbeRequest
// PreSeedTenantSpanConfigs precedes SeedTenantSpanConfigs, and enables the
// creation of initial span config records for newly created tenants.
PreSeedTenantSpanConfigs
// SeedTenantSpanConfigs populates system.span_configurations with seed
// data for secondary tenants. This state is what ensures that we always
// split on tenant boundaries when using the span configs infrastructure.
// This version comes with a migration to populate the same seed data
// for existing tenants.
SeedTenantSpanConfigs

// *************************************************
// Step (1): Add new versions here.
Expand Down Expand Up @@ -541,6 +550,14 @@ var versionsSingleton = keyedVersions{
Key: ProbeRequest,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 26},
},
{
Key: PreSeedTenantSpanConfigs,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 28},
},
{
Key: SeedTenantSpanConfigs,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 30},
},

// *************************************************
// Step (2): Add new versions here.
Expand Down
6 changes: 4 additions & 2 deletions pkg/clusterversion/key_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/migration/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ go_library(
"//pkg/roachpb:with-mocks",
"//pkg/server/serverpb",
"//pkg/settings/cluster",
"//pkg/spanconfig",
"//pkg/sql/catalog/descs",
"//pkg/sql/catalog/lease",
"//pkg/sql/sqlutil",
Expand Down
8 changes: 6 additions & 2 deletions pkg/migration/migrationjob/migration_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,19 @@ func (r resumer) Resume(ctx context.Context, execCtxI interface{}) error {
case *migration.SystemMigration:
err = m.Run(ctx, cv, mc.SystemDeps(), r.j)
case *migration.TenantMigration:
err = m.Run(ctx, cv, migration.TenantDeps{
tenantDeps := migration.TenantDeps{
DB: execCtx.ExecCfg().DB,
Codec: execCtx.ExecCfg().Codec,
Settings: execCtx.ExecCfg().Settings,
CollectionFactory: execCtx.ExecCfg().CollectionFactory,
LeaseManager: execCtx.ExecCfg().LeaseManager,
InternalExecutor: execCtx.ExecCfg().InternalExecutor,
TestingKnobs: execCtx.ExecCfg().MigrationTestingKnobs,
}, r.j)
}
tenantDeps.SpanConfig.KVAccessor = execCtx.ExecCfg().SpanConfigKVAccessor
tenantDeps.SpanConfig.Default = execCtx.ExecCfg().DefaultZoneConfig.AsSpanConfig()

err = m.Run(ctx, cv, tenantDeps, r.j)
default:
return errors.AssertionFailedf("unknown migration type %T", m)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/migration/migrations/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ go_library(
"records_based_registry.go",
"retry_jobs_with_exponential_backoff.go",
"schema_changes.go",
"seed_tenant_span_configs.go",
"separated_intents.go",
"span_configurations.go",
"sql_instances.go",
Expand Down
6 changes: 6 additions & 0 deletions pkg/migration/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,12 @@ var migrations = []migration.Migration{
NoPrecondition,
alterSystemStmtDiagReqs,
),
migration.NewTenantMigration(
"seed system.span_configurations with configs for existing for existing tenants",
toCV(clusterversion.SeedTenantSpanConfigs),
NoPrecondition,
seedTenantSpanConfigsMigration,
),
}

func init() {
Expand Down
Loading

0 comments on commit 29fadd6

Please sign in to comment.