Skip to content

Commit

Permalink
sql: improve tenant records
Browse files Browse the repository at this point in the history
This commit contains the following changes:
- rename "state" to "data_state", "active" to "ready"
- stored, non-virtual columns for "name", "data_state", "service_mode"
- deprecate the column "active" since it mirrors "data_state'
- move `descpb.TenantInfo` to new package `mtinfopb`.

Details follow.

**rename TenantInfo.State to DataState, "ACTIVE" to "READY"**

We've discovered that we'd like to separate the readiness of the data
from the activation of the service. To emphasize this, this commit
renames the field "State" to "DataState".

Additionally, the state "ACTIVE" was confusing as it suggests that
something is running, whereas it merely marks the tenant data as ready
for use. So this commit also renames that state accordingly.

**new tenant info field ServiceMode**

Summary of changes:
- the new TenantInfo.ServiceMode field indicates how to run servers.
- new syntax: `ALTER TENANT ... START SERVICE EXTERNAL/SHARED`,
  `ALTER TENANT ... STOP SERVICE`.
- tenants created via `create_tenant(<id>)`
  (via CC serverless control plane) start in service mode EXTERNAL.
- other tenants start in service mode NONE.
- need ALTER TENANT STOP SERVICE before dropping a tenant.
  - except in the case of `crdb_internal.destroy_tenant`
    for compat with CC serverless control plane.

**make the output columns of SHOW TENANT lowercase**

All the SHOW statements report status-like data in lowercase.
SHOW TENANT(s) should not be different.

**use actual SQL columns for the TenantInfo fields**

Release note: None
  • Loading branch information
knz committed Jan 23, 2023
1 parent b21379b commit 0355340
Show file tree
Hide file tree
Showing 86 changed files with 1,199 additions and 509 deletions.
3 changes: 3 additions & 0 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -1338,11 +1338,13 @@ unreserved_keyword ::=
| 'SEQUENCE'
| 'SEQUENCES'
| 'SERVER'
| 'SERVICE'
| 'SESSION'
| 'SESSIONS'
| 'SET'
| 'SETS'
| 'SHARE'
| 'SHARED'
| 'SHOW'
| 'SIMPLE'
| 'SKIP'
Expand All @@ -1361,6 +1363,7 @@ unreserved_keyword ::=
| 'STATEMENTS'
| 'STATISTICS'
| 'STDIN'
| 'STOP'
| 'STORAGE'
| 'STORE'
| 'STORED'
Expand Down
2 changes: 2 additions & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,7 @@ GO_TARGETS = [
"//pkg/kv/kvserver:kvserver_test",
"//pkg/kv:kv",
"//pkg/kv:kv_test",
"//pkg/multitenant/mtinfopb:mtinfopb",
"//pkg/multitenant/multitenantcpu:multitenantcpu",
"//pkg/multitenant/multitenantio:multitenantio",
"//pkg/multitenant/tenantcapabilities/tenantcapabilitiesauthorizer:tenantcapabilitiesauthorizer",
Expand Down Expand Up @@ -2626,6 +2627,7 @@ GET_X_DATA_TARGETS = [
"//pkg/kv/kvserver/txnwait:get_x_data",
"//pkg/kv/kvserver/uncertainty:get_x_data",
"//pkg/multitenant:get_x_data",
"//pkg/multitenant/mtinfopb:get_x_data",
"//pkg/multitenant/multitenantcpu:get_x_data",
"//pkg/multitenant/multitenantio:get_x_data",
"//pkg/multitenant/tenantcapabilities:get_x_data",
Expand Down
2 changes: 2 additions & 0 deletions pkg/ccl/backupccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ go_library(
"//pkg/kv/kvserver/concurrency/lock",
"//pkg/kv/kvserver/protectedts",
"//pkg/kv/kvserver/protectedts/ptpb",
"//pkg/multitenant/mtinfopb",
"//pkg/roachpb",
"//pkg/scheduledjobs",
"//pkg/scheduledjobs/schedulebase",
Expand Down Expand Up @@ -227,6 +228,7 @@ go_test(
"//pkg/kv/kvserver/protectedts",
"//pkg/kv/kvserver/protectedts/ptpb",
"//pkg/kv/kvserver/protectedts/ptutil",
"//pkg/multitenant/mtinfopb",
"//pkg/roachpb",
"//pkg/scheduledjobs",
"//pkg/scheduledjobs/schedulebase",
Expand Down
5 changes: 3 additions & 2 deletions pkg/ccl/backupccl/backup_metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuppb"
"github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuptestutils"
"github.com/cockroachdb/cockroach/pkg/cloud"
"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/sql"
Expand Down Expand Up @@ -242,8 +243,8 @@ func checkIntroducedSpans(
func checkTenants(
ctx context.Context, t *testing.T, m *backuppb.BackupManifest, bm *backupinfo.BackupMetadata,
) {
var metaTenants []descpb.TenantInfoWithUsage
var tenant descpb.TenantInfoWithUsage
var metaTenants []mtinfopb.TenantInfoWithUsage
var tenant mtinfopb.TenantInfoWithUsage
it := bm.TenantIter(ctx)
defer it.Close()

Expand Down
7 changes: 4 additions & 3 deletions pkg/ccl/backupccl/backup_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/protectedts"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/protectedts/ptpb"
"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/scheduledjobs"
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
Expand Down Expand Up @@ -1325,9 +1326,9 @@ func checkForNewTables(

func getTenantInfo(
ctx context.Context, codec keys.SQLCodec, txn isql.Txn, jobDetails jobspb.BackupDetails,
) ([]roachpb.Span, []descpb.TenantInfoWithUsage, error) {
) ([]roachpb.Span, []mtinfopb.TenantInfoWithUsage, error) {
var spans []roachpb.Span
var tenants []descpb.TenantInfoWithUsage
var tenants []mtinfopb.TenantInfoWithUsage
var err error
if jobDetails.FullCluster && codec.ForSystemTenant() {
// Include all tenants.
Expand Down Expand Up @@ -1422,7 +1423,7 @@ func createBackupManifest(
}

var spans []roachpb.Span
var tenants []descpb.TenantInfoWithUsage
var tenants []mtinfopb.TenantInfoWithUsage
tenantSpans, tenantInfos, err := getTenantInfo(
ctx, execCfg.Codec, txn, jobDetails,
)
Expand Down
95 changes: 64 additions & 31 deletions pkg/ccl/backupccl/backup_planning_tenant.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ package backupccl
import (
"context"

"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/isql"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
Expand All @@ -22,82 +22,115 @@ import (
const tenantMetadataQuery = `
SELECT
tenants.id, /* 0 */
tenants.active, /* 1 */
tenants.info, /* 2 */
tenant_usage.ru_burst_limit, /* 3 */
tenant_usage.ru_refill_rate, /* 4 */
tenant_usage.ru_current, /* 5 */
tenant_usage.total_consumption /* 6 */
tenants.info, /* 1 */
tenants.name, /* 2 */
tenants.data_state, /* 3 */
tenants.service_mode, /* 4 */
tenant_usage.ru_burst_limit, /* 5 */
tenant_usage.ru_refill_rate, /* 6 */
tenant_usage.ru_current, /* 7 */
tenant_usage.total_consumption /* 8 */
FROM
system.tenants
LEFT JOIN system.tenant_usage ON
tenants.id = tenant_usage.tenant_id AND tenant_usage.instance_id = 0
`

func tenantMetadataFromRow(row tree.Datums) (descpb.TenantInfoWithUsage, error) {
if len(row) != 7 {
return descpb.TenantInfoWithUsage{}, errors.AssertionFailedf(
func tenantMetadataFromRow(row tree.Datums) (mtinfopb.TenantInfoWithUsage, error) {
if len(row) != 9 {
return mtinfopb.TenantInfoWithUsage{}, errors.AssertionFailedf(
"unexpected row size %d from tenant metadata query", len(row),
)
}

id := uint64(tree.MustBeDInt(row[0]))
res := descpb.TenantInfoWithUsage{
TenantInfo: descpb.TenantInfo{
res := mtinfopb.TenantInfoWithUsage{
TenantInfo: mtinfopb.TenantInfo{
// for compatibility
DeprecatedID: id,
},
TenantInfoWithUsage_ExtraColumns: mtinfopb.TenantInfoWithUsage_ExtraColumns{
ID: id,
},
}
infoBytes := []byte(tree.MustBeDBytes(row[2]))
infoBytes := []byte(tree.MustBeDBytes(row[1]))
if err := protoutil.Unmarshal(infoBytes, &res.TenantInfo); err != nil {
return descpb.TenantInfoWithUsage{}, err
return mtinfopb.TenantInfoWithUsage{}, err
}
if row[2] != tree.DNull {
res.Name = roachpb.TenantName(tree.MustBeDString(row[2]))
}
if row[3] != tree.DNull {
res.DataState = mtinfopb.TenantDataState(tree.MustBeDInt(row[3]))
} else {
// Pre-v23.1 info struct.
switch res.TenantInfo.DeprecatedDataState {
case mtinfopb.TenantInfo_READY:
res.DataState = mtinfopb.DataStateReady
case mtinfopb.TenantInfo_ADD:
res.DataState = mtinfopb.DataStateAdd
case mtinfopb.TenantInfo_DROP:
res.DataState = mtinfopb.DataStateDrop
return res, errors.AssertionFailedf("unhandled: %d", res.TenantInfo.DeprecatedDataState)
}
}
res.ServiceMode = mtinfopb.ServiceModeNone
if row[4] != tree.DNull {
res.ServiceMode = mtinfopb.TenantServiceMode(tree.MustBeDInt(row[4]))
} else if res.DataState == mtinfopb.DataStateReady {
// Records created for CC Serverless pre-v23.1.
res.ServiceMode = mtinfopb.ServiceModeExternal
}
// If this tenant had no reported consumption and its token bucket was not
// configured, the tenant_usage values are all NULL.
//
// It should be sufficient to check any one value, but we check all of them
// just to be defensive (in case the table contains invalid data).
for _, d := range row[3:5] {
for _, d := range row[5:] {
if d == tree.DNull {
return res, nil
}
}
res.Usage = &descpb.TenantInfoWithUsage_Usage{
RUBurstLimit: float64(tree.MustBeDFloat(row[3])),
RURefillRate: float64(tree.MustBeDFloat(row[4])),
RUCurrent: float64(tree.MustBeDFloat(row[5])),
res.Usage = &mtinfopb.TenantInfoWithUsage_Usage{
RUBurstLimit: float64(tree.MustBeDFloat(row[5])),
RURefillRate: float64(tree.MustBeDFloat(row[6])),
RUCurrent: float64(tree.MustBeDFloat(row[7])),
}
if row[6] != tree.DNull {
consumptionBytes := []byte(tree.MustBeDBytes(row[6]))
if row[8] != tree.DNull {
consumptionBytes := []byte(tree.MustBeDBytes(row[8]))
if err := protoutil.Unmarshal(consumptionBytes, &res.Usage.Consumption); err != nil {
return descpb.TenantInfoWithUsage{}, err
return mtinfopb.TenantInfoWithUsage{}, err
}
}
return res, nil
}

func retrieveSingleTenantMetadata(
ctx context.Context, txn isql.Txn, tenantID roachpb.TenantID,
) (descpb.TenantInfoWithUsage, error) {
) (mtinfopb.TenantInfoWithUsage, error) {
row, err := txn.QueryRow(
ctx, "backupccl.retrieveSingleTenantMetadata", txn.KV(),
tenantMetadataQuery+` WHERE id = $1`, tenantID.ToUint64(),
)
if err != nil {
return descpb.TenantInfoWithUsage{}, err
return mtinfopb.TenantInfoWithUsage{}, err
}
if row == nil {
return descpb.TenantInfoWithUsage{}, errors.Errorf("tenant %s does not exist", tenantID)
return mtinfopb.TenantInfoWithUsage{}, errors.Errorf("tenant %s does not exist", tenantID)
}
info, err := tenantMetadataFromRow(row)
if err != nil {
return mtinfopb.TenantInfoWithUsage{}, err
}
if !tree.MustBeDBool(row[1]) {
return descpb.TenantInfoWithUsage{}, errors.Errorf("tenant %s is not active", tenantID)
if info.DataState != mtinfopb.DataStateReady {
return mtinfopb.TenantInfoWithUsage{}, errors.Errorf("tenant %s is not active", tenantID)
}

return tenantMetadataFromRow(row)
return info, nil
}

func retrieveAllTenantsMetadata(
ctx context.Context, txn isql.Txn,
) ([]descpb.TenantInfoWithUsage, error) {
) ([]mtinfopb.TenantInfoWithUsage, error) {
rows, err := txn.QueryBuffered(
ctx, "backupccl.retrieveAllTenantsMetadata", txn.KV(),
// TODO(?): Should we add a `WHERE active`? We require the tenant to be active
Expand All @@ -109,7 +142,7 @@ func retrieveAllTenantsMetadata(
if err != nil {
return nil, err
}
res := make([]descpb.TenantInfoWithUsage, len(rows))
res := make([]mtinfopb.TenantInfoWithUsage, len(rows))
for i := range rows {
res[i], err = tenantMetadataFromRow(rows[i])
if err != nil {
Expand Down
Loading

0 comments on commit 0355340

Please sign in to comment.