Merge #37533

37533: sql: few minor refactoring on the road to getting schema in its own package r=vivekmenezes a=vivekmenezes I couldn't finish this work because I hit roadblocks, but I've included three commits here that will be useful to have. I think it's worth merging these since they are improvements to the code. Co-authored-by: Vivek Menezes <[email protected]>
cockroachdb · May 16, 2019 · e4806ba · e4806ba
2 parents 9ecfb8b + ed4bffc
commit e4806ba
Show file tree

Hide file tree

Showing 19 changed files with 200 additions and 172 deletions.
diff --git a/pkg/server/server.go b/pkg/server/server.go
@@ -353,19 +353,6 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) {
 	s.internalMemMetrics = sql.MakeMemMetrics("internal", cfg.HistogramWindowInterval())
 	s.registry.AddMetricStruct(s.internalMemMetrics)
 
-	// Set up Lease Manager
-	var lmKnobs sql.LeaseManagerTestingKnobs
-	if leaseManagerTestingKnobs := cfg.TestingKnobs.SQLLeaseManager; leaseManagerTestingKnobs != nil {
-		lmKnobs = *leaseManagerTestingKnobs.(*sql.LeaseManagerTestingKnobs)
-	}
-	s.leaseMgr = sql.NewLeaseManager(
-		s.cfg.AmbientCtx,
-		nil, /* execCfg - will be set later because of circular dependencies */
-		lmKnobs,
-		s.stopper,
-		s.cfg.LeaseManagerConfig,
-	)
-
 	// We do not set memory monitors or a noteworthy limit because the children of
 	// this monitor will be setting their own noteworthy limits.
 	rootSQLMemoryMonitor := mon.MakeMonitor(
@@ -514,6 +501,23 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) {
 	distSQLMetrics := distsqlrun.MakeDistSQLMetrics(cfg.HistogramWindowInterval())
 	s.registry.AddMetricStruct(distSQLMetrics)
 
+	// Set up Lease Manager
+	var lmKnobs sql.LeaseManagerTestingKnobs
+	if leaseManagerTestingKnobs := cfg.TestingKnobs.SQLLeaseManager; leaseManagerTestingKnobs != nil {
+		lmKnobs = *leaseManagerTestingKnobs.(*sql.LeaseManagerTestingKnobs)
+	}
+	s.leaseMgr = sql.NewLeaseManager(
+		s.cfg.AmbientCtx,
+		&s.nodeIDContainer,
+		s.db,
+		s.clock,
+		nil, /* internalExecutor - will be set later because of circular dependencies */
+		st,
+		lmKnobs,
+		s.stopper,
+		s.cfg.LeaseManagerConfig,
+	)
+
 	// Set up the DistSQL server.
 	distSQLCfg := distsqlrun.ServerConfig{
 		AmbientContext: s.cfg.AmbientCtx,
@@ -724,7 +728,7 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) {
 
 	s.execCfg = &execCfg
 
-	s.leaseMgr.SetExecCfg(&execCfg)
+	s.leaseMgr.SetInternalExecutor(execCfg.InternalExecutor)
 	s.leaseMgr.RefreshLeases(s.stopper, s.db, s.gossip)
 	s.leaseMgr.PeriodicallyRefreshSomeLeases()
 

diff --git a/pkg/sql/backfill.go b/pkg/sql/backfill.go
@@ -1097,7 +1097,7 @@ func runSchemaChangesInTxn(
 		// cleanup for later.
 		b := txn.NewBatch()
 		for _, drain := range tableDesc.DrainingNames {
-			tbKey := tableKey{drain.ParentID, drain.Name}.Key()
+			tbKey := sqlbase.NewTableKey(drain.ParentID, drain.Name).Key()
 			b.Del(tbKey)
 		}
 		tableDesc.DrainingNames = nil

diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go
@@ -376,7 +376,7 @@ CREATE TABLE crdb_internal.leases (
 )`,
 	populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error {
 		leaseMgr := p.LeaseMgr()
-		nodeID := tree.NewDInt(tree.DInt(int64(leaseMgr.execCfg.NodeID.Get())))
+		nodeID := tree.NewDInt(tree.DInt(int64(leaseMgr.nodeIDContainer.Get())))
 
 		leaseMgr.mu.Lock()
 		defer leaseMgr.mu.Unlock()
@@ -605,7 +605,7 @@ CREATE TABLE crdb_internal.node_statement_statistics (
 		}
 
 		leaseMgr := p.LeaseMgr()
-		nodeID := tree.NewDInt(tree.DInt(int64(leaseMgr.execCfg.NodeID.Get())))
+		nodeID := tree.NewDInt(tree.DInt(int64(leaseMgr.nodeIDContainer.Get())))
 
 		// Retrieve the application names and sort them to ensure the
 		// output is deterministic.
@@ -1802,6 +1802,32 @@ CREATE TABLE crdb_internal.ranges_no_leases (
 	},
 }
 
+type namespaceKey struct {
+	parentID sqlbase.ID
+	name     string
+}
+
+// getAllNames returns a map from ID to namespaceKey for every entry in
+// system.namespace.
+func (p *planner) getAllNames(ctx context.Context) (map[sqlbase.ID]namespaceKey, error) {
+	namespace := map[sqlbase.ID]namespaceKey{}
+	rows, err := p.ExtendedEvalContext().ExecCfg.InternalExecutor.Query(
+		ctx, "get-all-names", p.txn,
+		`SELECT id, "parentID", name FROM system.namespace`,
+	)
+	if err != nil {
+		return nil, err
+	}
+	for _, r := range rows {
+		id, parentID, name := tree.MustBeDInt(r[0]), tree.MustBeDInt(r[1]), tree.MustBeDString(r[2])
+		namespace[sqlbase.ID(id)] = namespaceKey{
+			parentID: sqlbase.ID(parentID),
+			name:     string(name),
+		}
+	}
+	return namespace, nil
+}
+
 // crdbInternalZonesTable decodes and exposes the zone configs in the
 // system.zones table.
 // The cli_specifier column is deprecated and only exists to be used

diff --git a/pkg/sql/create_sequence.go b/pkg/sql/create_sequence.go
@@ -49,7 +49,7 @@ func (p *planner) CreateSequence(ctx context.Context, n *tree.CreateSequence) (p
 }
 
 func (n *createSequenceNode) startExec(params runParams) error {
-	tKey := getSequenceKey(n.dbDesc, n.n.Name.Table())
+	tKey := sqlbase.NewTableKey(n.dbDesc.ID, n.n.Name.Table())
 	if exists, err := descExists(params.ctx, params.p.txn, tKey.Key()); err == nil && exists {
 		if n.n.IfNotExists {
 			// If the sequence exists but the user specified IF NOT EXISTS, return without doing anything.
@@ -63,10 +63,6 @@ func (n *createSequenceNode) startExec(params runParams) error {
 	return doCreateSequence(params, n.n.String(), n.dbDesc, &n.n.Name, n.n.Options)
 }
 
-func getSequenceKey(dbDesc *DatabaseDescriptor, seqName string) tableKey {
-	return tableKey{parentID: dbDesc.ID, name: seqName}
-}
-
 // doCreateSequence performs the creation of a sequence in KV. The
 // context argument is a string to use in the event log.
 func doCreateSequence(
@@ -93,7 +89,7 @@ func doCreateSequence(
 	// makeSequenceTableDesc already validates the table. No call to
 	// desc.ValidateTable() needed here.
 
-	key := getSequenceKey(dbDesc, name.Table()).Key()
+	key := sqlbase.NewTableKey(dbDesc.ID, name.Table()).Key()
 	if err = params.p.createDescriptorWithID(params.ctx, key, id, &desc, params.EvalContext().Settings); err != nil {
 		return err
 	}

diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go
@@ -104,7 +104,7 @@ type createTableRun struct {
 }
 
 func (n *createTableNode) startExec(params runParams) error {
-	tKey := tableKey{parentID: n.dbDesc.ID, name: n.n.Table.Table()}
+	tKey := sqlbase.NewTableKey(n.dbDesc.ID, n.n.Table.Table())
 	key := tKey.Key()
 	if exists, err := descExists(params.ctx, params.p.txn, key); err == nil && exists {
 		if n.n.IfNotExists {

diff --git a/pkg/sql/create_view.go b/pkg/sql/create_view.go
@@ -110,7 +110,7 @@ func (p *planner) CreateView(ctx context.Context, n *tree.CreateView) (planNode,
 
 func (n *createViewNode) startExec(params runParams) error {
 	viewName := n.n.Name.Table()
-	tKey := tableKey{parentID: n.dbDesc.ID, name: viewName}
+	tKey := sqlbase.NewTableKey(n.dbDesc.ID, viewName)
 	key := tKey.Key()
 	if exists, err := descExists(params.ctx, params.p.txn, key); err == nil && exists {
 		// TODO(a-robinson): Support CREATE OR REPLACE commands.

diff --git a/pkg/sql/database.go b/pkg/sql/database.go
@@ -37,19 +37,6 @@ import (
 // suitable; consider instead schema_accessors.go and resolver.go.
 //
 
-// databaseKey implements sqlbase.DescriptorKey.
-type databaseKey struct {
-	name string
-}
-
-func (dk databaseKey) Key() roachpb.Key {
-	return sqlbase.MakeNameMetadataKey(keys.RootNamespaceID, dk.name)
-}
-
-func (dk databaseKey) Name() string {
-	return dk.name
-}
-
 // databaseCache holds a cache from database name to database ID. It is
 // populated as database IDs are requested and a new cache is created whenever
 // the system config changes. As such, no attempt is made to limit its size
@@ -108,7 +95,7 @@ func getDatabaseID(
 	if name == sqlbase.SystemDB.Name {
 		return sqlbase.SystemDB.ID, nil
 	}
-	dbID, err := getDescriptorID(ctx, txn, databaseKey{name})
+	dbID, err := getDescriptorID(ctx, txn, sqlbase.NewDatabaseKey(name))
 	if err != nil {
 		return sqlbase.InvalidID, err
 	}
@@ -273,7 +260,7 @@ func (dc *databaseCache) getCachedDatabaseID(name string) (sqlbase.ID, error) {
 		return sqlbase.SystemDB.ID, nil
 	}
 
-	nameKey := databaseKey{name}
+	nameKey := sqlbase.NewDatabaseKey(name)
 	nameVal := dc.systemConfig.GetValue(nameKey.Key())
 	if nameVal == nil {
 		return sqlbase.InvalidID, nil
@@ -293,8 +280,8 @@ func (p *planner) renameDatabase(
 		return err
 	}
 
-	oldKey := databaseKey{oldName}.Key()
-	newKey := databaseKey{newName}.Key()
+	oldKey := sqlbase.NewDatabaseKey(oldName).Key()
+	newKey := sqlbase.NewDatabaseKey(newName).Key()
 	descID := oldDesc.GetID()
 	descKey := sqlbase.MakeDescMetadataKey(descID)
 	descDesc := sqlbase.WrapDescriptor(oldDesc)

diff --git a/pkg/sql/descriptor.go b/pkg/sql/descriptor.go
@@ -62,7 +62,7 @@ func GenerateUniqueDescID(ctx context.Context, db *client.DB) (sqlbase.ID, error
 func (p *planner) createDatabase(
 	ctx context.Context, desc *sqlbase.DatabaseDescriptor, ifNotExists bool,
 ) (bool, error) {
-	plainKey := databaseKey{desc.Name}
+	plainKey := sqlbase.NewDatabaseKey(desc.Name)
 	idKey := plainKey.Key()
 
 	if exists, err := descExists(ctx, p.txn, idKey); err == nil && exists {

diff --git a/pkg/sql/helpers_test.go b/pkg/sql/helpers_test.go
@@ -23,6 +23,12 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 )
 
+// A unique id for a particular table descriptor version.
+type tableVersionID struct {
+	id      sqlbase.ID
+	version sqlbase.DescriptorVersion
+}
+
 // LeaseRemovalTracker can be used to wait for leases to be removed from the
 // store (leases are removed from the store async w.r.t. LeaseManager
 // operations).