Skip to content

Commit

Permalink
server: Reject SQL Pods with an invalid tenant id.
Browse files Browse the repository at this point in the history
This commit fixes an issue where attempting to start a SQL Pod
through cockroach mt start-sql command returns an error if the
tenant id has not been created before. Previously, attempting to
start a SQL Pod with a non existent tenant id would crash. Even
worse, it would poison the tenant id causing future
crdb_internal.create_sql_tenant calls to fail.
With this fix, the cockroach mt start-sql command returns an error
for non existent tenant ids and future calls to
crdb_internal.create_sql_tenant are successful.
Resolves: #64963
Release note (bug fix): The cockroach mt start-sql command with a
non existent tenant id returns an error. Previously, it would
crash and poison the tenant id for future usage.
  • Loading branch information
rimadeodhar committed May 25, 2021
1 parent c04887f commit eadc7f7
Showing 1 changed file with 25 additions and 1 deletion.
26 changes: 25 additions & 1 deletion pkg/server/server_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ package server

import (
"context"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema"
"math"
"net"
"os"
Expand Down Expand Up @@ -252,7 +254,6 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
codec = keys.MakeSQLCodec(override)
}
}

// Create blob service for inter-node file sharing.
blobService, err := blobs.NewBlobService(cfg.Settings.ExternalIODir)
if err != nil {
Expand Down Expand Up @@ -745,6 +746,22 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
}, nil
}

// Checks if tenant exists. This function does a very superficial check to see if the system db
// has been bootstrapped for the tenant. This is not a complete check and is only sufficient
// to be used in the dev environment.
func checkTenantExists(ctx context.Context, codec keys.SQLCodec, db *kv.DB) error {
key := catalogkeys.NewDatabaseKey(systemschema.SystemDatabaseName).Key(codec)
result, err := db.Get(ctx, key)
if err != nil {
return err
}
if result.Value == nil || result.ValueInt() != keys.SystemDatabaseID {
return errors.New("System DB uninitialized, check if tenant is non existent")
}
// Tenant has been bootstrapped successfully
return nil
}

func (s *SQLServer) preStart(
ctx context.Context,
stopper *stop.Stopper,
Expand All @@ -763,6 +780,13 @@ func (s *SQLServer) preStart(
return err
}
}
// Confirm tenant exists prior to initialization. This is a sanity
// check for the dev environment to ensure that a tenant has been
// successfully created before attempting to initialize a SQL
// server for it.
if err := checkTenantExists(ctx, s.execCfg.Codec, s.execCfg.DB); err != nil {
return err
}
s.connManager = connManager
s.pgL = pgL
s.execCfg.GCJobNotifier.Start(ctx)
Expand Down

0 comments on commit eadc7f7

Please sign in to comment.