Skip to content

Commit

Permalink
server: fix DataDistribution server error when creating a tenant
Browse files Browse the repository at this point in the history
This is a stop-gap commit that enables the DataDistribution endpoint
to handle the parts of the key space belonging to secondary tenants
without error.

Despite no error, the result returned for secondary tenants is not correct.
The DataDistribution endpoint was written before cockroachdb#79700, and therefore
doesn't know that multiple tables can exist within a range.

Additionally, the results for the system tenant will be incorrect soon
because cockroachdb#81008 is in progress.

Fixes: cockroachdb#97993
Release note: None
  • Loading branch information
zachlite authored and Zach Lite committed Mar 21, 2023
1 parent 5f5cf26 commit 342e42e
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 4 deletions.
14 changes: 10 additions & 4 deletions pkg/ccl/serverccl/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,14 @@ func TestAdminAPIDataDistributionPartitioning(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

// Need to disable the test tenant because this test fails
// when run through a tenant (with internal server error).
// More investigation is required. Tracked with #76387.
disableDefaultTestTenant := true
testCluster := serverutils.StartNewTestCluster(t, 3,
base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
// Need to disable the test tenant because this test fails
// when run through a tenant (with internal server error).
// More investigation is required. Tracked with #76387.
DisableDefaultTestTenant: true,
DisableDefaultTestTenant: disableDefaultTestTenant,
},
})
defer testCluster.Stopper().Stop(context.Background())
Expand Down Expand Up @@ -81,6 +82,11 @@ func TestAdminAPIDataDistributionPartitioning(t *testing.T) {
sqlDB.Exec(t, `ALTER PARTITION us OF TABLE comments CONFIGURE ZONE USING gc.ttlseconds = 9001`)
sqlDB.Exec(t, `ALTER PARTITION eu OF TABLE comments CONFIGURE ZONE USING gc.ttlseconds = 9002`)

if disableDefaultTestTenant {
// Make sure secondary tenants don't cause the endpoint to error.
sqlDB.Exec(t, "CREATE TENANT 'app'")
}

// Assert that we get all roachblog zone configs back.
expectedZoneConfigNames := map[string]struct{}{
"PARTITION eu OF INDEX roachblog.public.comments@comments_pkey": {},
Expand Down
11 changes: 11 additions & 0 deletions pkg/server/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -3082,6 +3082,17 @@ func (s *adminServer) dataDistributionHelper(
if err != nil {
return err
}

// A range descriptor for a secondary tenant may not contain
// a table prefix. Often, the start key for a tenant will be just
// the tenant prefix itself, e.g. `/Tenant/2`. Once the tenant prefix
// is stripped inside `DecodeTablePrefix`, nothing (aka `/Min`) is left.
keySansPrefix, _ := keys.MakeSQLCodec(tenID).StripTenantPrefix(rangeDesc.StartKey.AsRawKey())
if keys.MinKey.Equal(keySansPrefix) {
// There's no table prefix to be decoded.
// Try the next descriptor.
continue
}
_, tableID, err := keys.MakeSQLCodec(tenID).DecodeTablePrefix(rangeDesc.StartKey.AsRawKey())
if err != nil {
return err
Expand Down
3 changes: 3 additions & 0 deletions pkg/server/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2220,6 +2220,9 @@ func TestAdminAPIDataDistribution(t *testing.T) {
sqlDB.Exec(t, `CREATE DATABASE "sp'ec\ch""ars"`)
sqlDB.Exec(t, `CREATE TABLE "sp'ec\ch""ars"."more\spec'chars" (id INT PRIMARY KEY)`)

// Make sure secondary tenants don't cause the endpoint to error.
sqlDB.Exec(t, "CREATE TENANT 'app'")

// Verify that we see their replicas in the DataDistribution response, evenly spread
// across the test cluster's three nodes.

Expand Down

0 comments on commit 342e42e

Please sign in to comment.