Skip to content

Commit

Permalink
sql: initialize tenant span config with rangefeeds enabled
Browse files Browse the repository at this point in the history 
Fixes cockroachdb#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]>
  • Loading branch information
@ajwerner @irfansharif @RajivTS
2 people authored and RajivTS committed Mar 6, 2022
1 parent 5351e78 commit 2ae0937
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 18 deletions.
15 changes: 6 additions & 9 deletions pkg/ccl/spanconfigccl/spanconfigcomparedccl/testdata/multitenant
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
----
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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.
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions pkg/ccl/spanconfigccl/spanconfigreconcilerccl/testdata/multitenant/basic
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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)
11 changes: 6 additions & 5 deletions pkg/config/system.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions pkg/sql/tenant.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down

0 comments on commit 2ae0937

Please sign in to comment.