Skip to content

Commit

Permalink
Merge #49784
Browse files Browse the repository at this point in the history 
49784: config: introduce pseudo "tenants" zone r=nvanbenschoten a=nvanbenschoten

Fixes #49318.
Fixes #49445.
Progress towards #48123.
Informs #48774.

This commit introduces a new pseudo object ID in the system tenant's namespace called "tenants". Like "liveness" and "timeseries" before it, the pseudo object allows zone configurations to be applied to pseudo-objects that do not live directly in the system tenant's SQL keyspace. In this case, the "tenants" zone allows zone configurations to be set by the system tenant and applied to all other tenants in the system. There may come a time when we want secondary tenants to have more control over their zone configurations, but that will require a much larger change to the zone config structure and UX as a whole.

While making this change, we rationalize the rest of zone configuration handling and how it relates to multi-tenancy. Now that secondary tenant ranges have a zone config to call their own, we can make sense of calls from SQL into the zone configuration infrastructure. We gate off calls that don't make sense for secondary tenants and clean up hooks in SQL that handle zone config manipulation.

All of this works towards a good cause - we eliminate the remaining uses of `keys.TODOSQLCodec` from `pkg/sql/...` and `pkg/config/...`, bringing us a big step closer towards being able to remove the placeholder and close #48123.

This work also reveals that in order to address #48774, we need to be able to determine split points from the SystemConfig. This makes it very difficult to split on secondary tenant object (e.g. table) boundaries. However, it makes it straightforward to split on secondary tenant keysapce boundaries. This is already what we were thinking (see #47907), so out both convenience and desire, I expect that we'll follow this up with a PR that splits Ranges only at secondary tenant boundaries - placing the overhead of an otherwise empty tenant at only a single Range and a few KBs of data.

Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
  • Loading branch information
@nvanbenschoten
craig[bot] and nvanbenschoten committed Jun 4, 2020
2 parents c817d37 + 6953182 commit 9b50756
Show file tree
Hide file tree
Showing 52 changed files with 657 additions and 385 deletions.
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/backup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,7 @@ func TestBackupRestoreControlJob(t *testing.T) {
if err != nil {
t.Fatal(err)
}
last := uint32(v.ValueInt())
last := config.SystemTenantObjectID(v.ValueInt())
zoneConfig := zonepb.DefaultZoneConfig()
zoneConfig.RangeMaxBytes = proto.Int64(5000)
config.TestingSetZoneConfig(last+1, zoneConfig)
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/importccl/exportcsv_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func setupExportableBank(t *testing.T, nodes, rows int) (*sqlutils.SQLRunner, st
if err != nil {
t.Fatal(err)
}
last := uint32(v.ValueInt())
last := config.SystemTenantObjectID(v.ValueInt())
zoneConfig := zonepb.DefaultZoneConfig()
zoneConfig.RangeMaxBytes = proto.Int64(5000)
config.TestingSetZoneConfig(last+1, zoneConfig)
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/partitionccl/partition_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1406,7 +1406,7 @@ ALTER TABLE t ALTER PRIMARY KEY USING COLUMNS (y)

// Get the zone config corresponding to the table.
table := sqlbase.GetTableDescriptor(kvDB, keys.SystemSQLCodec, "t", "t")
kv, err := kvDB.Get(ctx, config.MakeZoneKey(uint32(table.ID)))
kv, err := kvDB.Get(ctx, config.MakeZoneKey(config.SystemTenantObjectID(table.ID)))
if err != nil {
t.Fatal(err)
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/cli/testdata/zip/partial1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ requesting heap files for node 1... writing: debug/nodes/1/heapprof.err.txt
requesting goroutine files for node 1... writing: debug/nodes/1/goroutines.err.txt
^- resulted in ...
requesting log file ...
requesting ranges... 33 found
requesting ranges... 34 found
writing: debug/nodes/1/ranges/1.json
writing: debug/nodes/1/ranges/2.json
writing: debug/nodes/1/ranges/3.json
Expand Down Expand Up @@ -88,6 +88,7 @@ writing: debug/nodes/1/ranges/30.json
writing: debug/nodes/1/ranges/31.json
writing: debug/nodes/1/ranges/32.json
writing: debug/nodes/1/ranges/33.json
writing: debug/nodes/1/ranges/34.json
writing: debug/nodes/2/status.json
using SQL connection URL for node 2: postgresql://...
retrieving SQL data for crdb_internal.feature_usage... writing: debug/nodes/2/crdb_internal.feature_usage.txt
Expand Down Expand Up @@ -179,7 +180,7 @@ requesting heap files for node 3... writing: debug/nodes/3/heapprof.err.txt
requesting goroutine files for node 3... writing: debug/nodes/3/goroutines.err.txt
^- resulted in ...
requesting log file ...
requesting ranges... 33 found
requesting ranges... 34 found
writing: debug/nodes/3/ranges/1.json
writing: debug/nodes/3/ranges/2.json
writing: debug/nodes/3/ranges/3.json
Expand Down Expand Up @@ -213,6 +214,7 @@ writing: debug/nodes/3/ranges/30.json
writing: debug/nodes/3/ranges/31.json
writing: debug/nodes/3/ranges/32.json
writing: debug/nodes/3/ranges/33.json
writing: debug/nodes/3/ranges/34.json
requesting list of SQL databases... 3 found
requesting database details for defaultdb... writing: debug/schema/defaultdb@details.json
0 tables found
Expand Down
6 changes: 4 additions & 2 deletions pkg/cli/testdata/zip/partial1_excluded
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ requesting heap files for node 1... writing: debug/nodes/1/heapprof.err.txt
requesting goroutine files for node 1... writing: debug/nodes/1/goroutines.err.txt
^- resulted in ...
requesting log file ...
requesting ranges... 33 found
requesting ranges... 34 found
writing: debug/nodes/1/ranges/1.json
writing: debug/nodes/1/ranges/2.json
writing: debug/nodes/1/ranges/3.json
Expand Down Expand Up @@ -88,6 +88,7 @@ writing: debug/nodes/1/ranges/30.json
writing: debug/nodes/1/ranges/31.json
writing: debug/nodes/1/ranges/32.json
writing: debug/nodes/1/ranges/33.json
writing: debug/nodes/1/ranges/34.json
writing: debug/nodes/2.skipped
writing: debug/nodes/3/status.json
using SQL connection URL for node 3: postgresql://...
Expand Down Expand Up @@ -116,7 +117,7 @@ requesting heap files for node 3... writing: debug/nodes/3/heapprof.err.txt
requesting goroutine files for node 3... writing: debug/nodes/3/goroutines.err.txt
^- resulted in ...
requesting log file ...
requesting ranges... 33 found
requesting ranges... 34 found
writing: debug/nodes/3/ranges/1.json
writing: debug/nodes/3/ranges/2.json
writing: debug/nodes/3/ranges/3.json
Expand Down Expand Up @@ -150,6 +151,7 @@ writing: debug/nodes/3/ranges/30.json
writing: debug/nodes/3/ranges/31.json
writing: debug/nodes/3/ranges/32.json
writing: debug/nodes/3/ranges/33.json
writing: debug/nodes/3/ranges/34.json
requesting list of SQL databases... 3 found
requesting database details for defaultdb... writing: debug/schema/defaultdb@details.json
0 tables found
Expand Down
6 changes: 4 additions & 2 deletions pkg/cli/testdata/zip/partial2
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ requesting heap files for node 1... writing: debug/nodes/1/heapprof.err.txt
requesting goroutine files for node 1... writing: debug/nodes/1/goroutines.err.txt
^- resulted in ...
requesting log file ...
requesting ranges... 33 found
requesting ranges... 34 found
writing: debug/nodes/1/ranges/1.json
writing: debug/nodes/1/ranges/2.json
writing: debug/nodes/1/ranges/3.json
Expand Down Expand Up @@ -88,6 +88,7 @@ writing: debug/nodes/1/ranges/30.json
writing: debug/nodes/1/ranges/31.json
writing: debug/nodes/1/ranges/32.json
writing: debug/nodes/1/ranges/33.json
writing: debug/nodes/1/ranges/34.json
writing: debug/nodes/3/status.json
using SQL connection URL for node 3: postgresql://...
retrieving SQL data for crdb_internal.feature_usage... writing: debug/nodes/3/crdb_internal.feature_usage.txt
Expand Down Expand Up @@ -115,7 +116,7 @@ requesting heap files for node 3... writing: debug/nodes/3/heapprof.err.txt
requesting goroutine files for node 3... writing: debug/nodes/3/goroutines.err.txt
^- resulted in ...
requesting log file ...
requesting ranges... 33 found
requesting ranges... 34 found
writing: debug/nodes/3/ranges/1.json
writing: debug/nodes/3/ranges/2.json
writing: debug/nodes/3/ranges/3.json
Expand Down Expand Up @@ -149,6 +150,7 @@ writing: debug/nodes/3/ranges/30.json
writing: debug/nodes/3/ranges/31.json
writing: debug/nodes/3/ranges/32.json
writing: debug/nodes/3/ranges/33.json
writing: debug/nodes/3/ranges/34.json
requesting list of SQL databases... 3 found
requesting database details for defaultdb... writing: debug/schema/defaultdb@details.json
0 tables found
Expand Down
3 changes: 2 additions & 1 deletion pkg/cli/testdata/zip/testzip
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ requesting heap profile for node 1... writing: debug/nodes/1/heap.pprof
requesting heap files for node 1... ? found
requesting goroutine files for node 1... 0 found
requesting log file ...
requesting ranges... 33 found
requesting ranges... 34 found
writing: debug/nodes/1/ranges/1.json
writing: debug/nodes/1/ranges/2.json
writing: debug/nodes/1/ranges/3.json
Expand Down Expand Up @@ -86,6 +86,7 @@ writing: debug/nodes/1/ranges/30.json
writing: debug/nodes/1/ranges/31.json
writing: debug/nodes/1/ranges/32.json
writing: debug/nodes/1/ranges/33.json
writing: debug/nodes/1/ranges/34.json
requesting list of SQL databases... 3 found
requesting database details for defaultdb... writing: debug/schema/defaultdb@details.json
0 tables found
Expand Down
21 changes: 11 additions & 10 deletions pkg/config/keys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,20 @@ import (

// MakeZoneKeyPrefix returns the key prefix for id's row in the system.zones
// table.
func MakeZoneKeyPrefix(id uint32) roachpb.Key {
return keys.SystemSQLCodec.ZoneKeyPrefix(id)
func MakeZoneKeyPrefix(id SystemTenantObjectID) roachpb.Key {
return keys.SystemSQLCodec.ZoneKeyPrefix(uint32(id))
}

// MakeZoneKey returns the key for id's entry in the system.zones table.
func MakeZoneKey(id uint32) roachpb.Key {
return keys.SystemSQLCodec.ZoneKey(id)
func MakeZoneKey(id SystemTenantObjectID) roachpb.Key {
return keys.SystemSQLCodec.ZoneKey(uint32(id))
}

// DecodeObjectID decodes the object ID from the front of key. It returns the
// decoded object ID, the remainder of the key, and whether the result is valid
// (i.e., whether the key was within the structured key space).
func DecodeObjectID(key roachpb.RKey) (uint32, []byte, bool) {
rem, id, err := keys.TODOSQLCodec.DecodeTablePrefix(key.AsRawKey())
return id, rem, err == nil
// DecodeSystemTenantObjectID decodes the object ID for the system-tenant from
// the front of key. It returns the decoded object ID, the remainder of the key,
// and whether the result is valid (i.e., whether the key was within the system
// tenant's structured key space).
func DecodeSystemTenantObjectID(key roachpb.RKey) (SystemTenantObjectID, []byte, bool) {
rem, id, err := keys.SystemSQLCodec.DecodeTablePrefix(key.AsRawKey())
return SystemTenantObjectID(id), rem, err == nil
}
6 changes: 3 additions & 3 deletions pkg/config/keys_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
)

func TestDecodeObjectID(t *testing.T) {
func TestDecodeSystemTenantObjectID(t *testing.T) {
defer leaktest.AfterTest(t)()

testCases := []struct {
key roachpb.RKey
keySuffix []byte
success bool
id uint32
id config.SystemTenantObjectID
}{
// Before the structured span.
{roachpb.RKeyMin, nil, false, 0},
Expand All @@ -43,7 +43,7 @@ func TestDecodeObjectID(t *testing.T) {
}

for tcNum, tc := range testCases {
id, keySuffix, success := config.DecodeObjectID(tc.key)
id, keySuffix, success := config.DecodeSystemTenantObjectID(tc.key)
if success != tc.success {
t.Errorf("#%d: expected success=%t", tcNum, tc.success)
continue
Expand Down
Loading

0 comments on commit 9b50756

Please sign in to comment.