sql: initialize tenant span config with rangefeeds enabled

Fixes #76331. Not doing this causes problems when we construct new tenants as they won't be able to establish a rangefeed until their first full reconciliation completes and then propagates. This isn't "buggy", but it is slow. Even this is not great. If the preceding range did not have rangefeeds enabled, it would take a closed timestamp interval for this enablement to propagate. Perhaps this is evidence that we should always carve out a span at the end of the keyspace and set it to have rangefeeds enabled. I'll leave that fix and testing of this to somebody else. Hope this is helpful enough on its own. Release note: None Co-authored-by: irfan sharif <[email protected]>
cockroachdb · Feb 11, 2022 · eee2148 · eee2148
1 parent ce55af9
commit eee2148
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 18 deletions.
diff --git a/pkg/ccl/spanconfigccl/spanconfigcomparedccl/testdata/multitenant b/pkg/ccl/spanconfigccl/spanconfigcomparedccl/testdata/multitenant
@@ -19,16 +19,14 @@ configs version=current offset=43
 ----
 ...
 /Table/50                                  database system (host)
-/Tenant/10                                 range default
-/Tenant/11                                 range default
+/Tenant/10                                 database system (tenant)
+/Tenant/11                                 database system (tenant)
 
 diff offset=50
 ----
 --- gossiped system config span (legacy)
 +++ span config infrastructure (current)
 ...
-+/Tenant/10                                 range default
-+/Tenant/11                                 range default
 
 reconcile tenant=11
 ----
@@ -40,7 +38,7 @@ configs version=current offset=43 limit=5
 ----
 ...
 /Table/50                                  database system (host)
-/Tenant/10                                 range default
+/Tenant/10                                 database system (tenant)
 /Tenant/11                                 database system (tenant)
 /Tenant/11/Table/4                         database system (tenant)
 /Tenant/11/Table/5                         database system (tenant)
@@ -58,8 +56,7 @@ diff offset=48 limit=10
 --- gossiped system config span (legacy)
 +++ span config infrastructure (current)
 ...
-+/Table/50                                  database system (host)
-+/Tenant/10                                 range default
+ /Tenant/10                                 database system (tenant)
  /Tenant/11                                 database system (tenant)
 +/Tenant/11/Table/4                         database system (tenant)
 +/Tenant/11/Table/5                         database system (tenant)
@@ -68,6 +65,7 @@ diff offset=48 limit=10
 +/Tenant/11/Table/11                        database system (tenant)
 +/Tenant/11/Table/12                        database system (tenant)
 +/Tenant/11/Table/13                        database system (tenant)
++/Tenant/11/Table/14                        database system (tenant)
  ...
 
 # Sanity check that new tenant tables show up correctly.
@@ -84,8 +82,7 @@ diff offset=48
 --- gossiped system config span (legacy)
 +++ span config infrastructure (current)
 ...
-+/Table/50                                  database system (host)
-+/Tenant/10                                 range default
+ /Tenant/10                                 database system (tenant)
  /Tenant/11                                 database system (tenant)
 +/Tenant/11/Table/4                         database system (tenant)
 +/Tenant/11/Table/5                         database system (tenant)

diff --git a/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/basic b/pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/basic
@@ -23,8 +23,8 @@ state offset=47
 ----
 ...
 /Table/5{0-1}                              database system (host)
-/Tenant/10{-"\x00"}                        range default
-/Tenant/11{-"\x00"}                        range default
+/Tenant/10{-"\x00"}                        database system (tenant)
+/Tenant/11{-"\x00"}                        database system (tenant)
 
 # Start the reconciliation loop for the secondary tenant.
 reconcile tenant=10
@@ -106,7 +106,7 @@ state offset=47
 /Tenant/10/Table/4{3-4}                    database system (tenant)
 /Tenant/10/Table/4{4-5}                    database system (tenant)
 /Tenant/10/Table/4{6-7}                    database system (tenant)
-/Tenant/11{-"\x00"}                        range default
+/Tenant/11{-"\x00"}                        database system (tenant)
 
 exec-sql tenant=10
 CREATE DATABASE db;
@@ -125,4 +125,4 @@ state offset=81
 /Tenant/10/Table/4{6-7}                    database system (tenant)
 /Tenant/10/Table/10{6-7}                   range default
 /Tenant/10/Table/10{7-8}                   range default
-/Tenant/11{-"\x00"}                        range default
+/Tenant/11{-"\x00"}                        database system (tenant)
diff --git a/pkg/config/system.go b/pkg/config/system.go
@@ -309,12 +309,13 @@ func (s *SystemConfig) GetSpanConfigForKey(
 	}
 	spanConfig := zone.AsSpanConfig()
 	if id <= keys.MaxReservedDescID {
-		// We enable rangefeeds for system tables; various internal
-		// subsystems (leveraging system tables) rely on rangefeeds to
-		// function.
+		// We enable rangefeeds for system tables; various internal subsystems
+		// (leveraging system tables) rely on rangefeeds to function. We also do the
+		// same for the tenant pseudo range ID for forwards compatibility with the
+		// span configs infrastructure.
 		spanConfig.RangefeedEnabled = true
-		// We exclude system tables from strict GC enforcement, it's
-		// only really applicable to user tables.
+		// We exclude system tables from strict GC enforcement, it's only really
+		// applicable to user tables.
 		spanConfig.GCPolicy.IgnoreStrictEnforcement = true
 	}
 	return spanConfig, nil

diff --git a/pkg/sql/tenant.go b/pkg/sql/tenant.go
@@ -149,6 +149,17 @@ func CreateTenantRecord(
 	// Does it even matter given it'll disappear as soon as tenant starts
 	// reconciling?
 	tenantSpanConfig := execCfg.DefaultZoneConfig.AsSpanConfig()
+	// Make sure to enable rangefeeds; the tenant will need them on its system
+	// tables as soon as it starts up. It's not unsafe/buggy if we didn't do this,
+	// -- the tenant's span config reconciliation process would eventually install
+	// appropriate (rangefeed.enabled = true) configs for its system tables, at
+	// which point subsystems that rely on rangefeeds are able to proceed. All of
+	// this can noticeably slow down pod startup, so we just enable things to
+	// start with.
+	tenantSpanConfig.RangefeedEnabled = true
+	// Make it behave like usual system database ranges, for good measure.
+	tenantSpanConfig.GCPolicy.IgnoreStrictEnforcement = true
+
 	tenantPrefix := keys.MakeTenantPrefix(roachpb.MakeTenantID(tenID))
 	toUpsert := []spanconfig.Record{
 		{