sql: mark use of Gossip as deprecated

This commit wraps Gossip in a wrapper when it is used in SQL. This small wrapper helpfully keeps track, in code, of work that needs to be done to enable multi-tenancy, and ensures that any deprecated use of Gossip is associated with a Github issue. The wrapper also makes sure we're not introducing new dependencies on Gossip. Release note: None
cockroachdb · Apr 23, 2020 · d0896b5 · d0896b5
1 parent c73fb58
commit d0896b5
Show file tree

Hide file tree

Showing 31 changed files with 266 additions and 83 deletions.
diff --git a/pkg/ccl/backupccl/backup_job.go b/pkg/ccl/backupccl/backup_job.go
@@ -140,13 +140,19 @@ func splitAndFilterSpans(
 }
 
 // clusterNodeCount returns the approximate number of nodes in the cluster.
-func clusterNodeCount(g *gossip.Gossip) int {
+func clusterNodeCount(gw gossip.DeprecatedGossip) (int, error) {
+	g, err := gw.Optional(47970)
+	if err != nil {
+		return 0, err
+	}
 	var nodes int
-	_ = g.IterateInfos(gossip.KeyNodeIDPrefix, func(_ string, _ gossip.Info) error {
-		nodes++
-		return nil
-	})
-	return nodes
+	_ = g.IterateInfos(
+		gossip.KeyNodeIDPrefix, func(_ string, _ gossip.Info) error {
+			nodes++
+			return nil
+		},
+	)
+	return nodes, nil
 }
 
 type spanAndTime struct {
@@ -516,7 +522,10 @@ func (b *backupResumer) Resume(
 		log.Warningf(ctx, "unable to load backup checkpoint while resuming job %d: %v", *b.job.ID(), err)
 	}
 
-	numClusterNodes := clusterNodeCount(p.ExecCfg().Gossip)
+	numClusterNodes, err := clusterNodeCount(p.ExecCfg().Gossip)
+	if err != nil {
+		return err
+	}
 
 	res, err := backup(
 		ctx,

diff --git a/pkg/ccl/backupccl/restore_job.go b/pkg/ccl/backupccl/restore_job.go
@@ -975,7 +975,11 @@ func (r *restoreResumer) Resume(
 		return nil
 	}
 
-	numClusterNodes := clusterNodeCount(p.ExecCfg().Gossip)
+	numClusterNodes, err := clusterNodeCount(p.ExecCfg().Gossip)
+	if err != nil {
+		return err
+	}
+
 	res, err := restore(
 		ctx,
 		p.ExecCfg().DB,

diff --git a/pkg/ccl/changefeedccl/bench_test.go b/pkg/ccl/changefeedccl/bench_test.go
@@ -214,10 +214,13 @@ func createBenchmarkChangefeed(
 	}
 	_, withDiff := details.Opts[changefeedbase.OptDiff]
 	kvfeedCfg := kvfeed.Config{
-		Settings:         settings,
-		DB:               s.DB(),
-		Clock:            feedClock,
-		Gossip:           s.GossipI().(*gossip.Gossip),
+		Settings: settings,
+		DB:       s.DB(),
+		Clock:    feedClock,
+		Gossip: gossip.MakeDeprecatedGossip(
+			s.GossipI().(*gossip.Gossip),
+			false, /* tenant */
+		),
 		Spans:            spans,
 		Targets:          details.Targets,
 		Sink:             buf,

diff --git a/pkg/ccl/changefeedccl/kvfeed/kv_feed.go b/pkg/ccl/changefeedccl/kvfeed/kv_feed.go
@@ -38,7 +38,7 @@ type Config struct {
 	Settings           *cluster.Settings
 	DB                 *kv.DB
 	Clock              *hlc.Clock
-	Gossip             *gossip.Gossip
+	Gossip             gossip.DeprecatedGossip
 	Spans              []roachpb.Span
 	Targets            jobspb.ChangefeedTargets
 	Sink               EventBufferWriter

diff --git a/pkg/ccl/changefeedccl/kvfeed/scanner.go b/pkg/ccl/changefeedccl/kvfeed/scanner.go
@@ -36,7 +36,7 @@ type kvScanner interface {
 
 type scanRequestScanner struct {
 	settings *cluster.Settings
-	gossip   *gossip.Gossip
+	gossip   gossip.DeprecatedGossip
 	db       *kv.DB
 }
 
@@ -58,7 +58,11 @@ func (p *scanRequestScanner) Scan(
 	// Export requests for the various watched spans are executed in parallel,
 	// with a semaphore-enforced limit based on a cluster setting.
 	// The spans here generally correspond with range boundaries.
-	maxConcurrentExports := clusterNodeCount(p.gossip) *
+	approxNodeCount, err := clusterNodeCount(p.gossip)
+	if err != nil {
+		return err
+	}
+	maxConcurrentExports := approxNodeCount *
 		int(kvserver.ExportRequestsLimit.Get(&p.settings.SV))
 	exportsSem := make(chan struct{}, maxConcurrentExports)
 	g := ctxgroup.WithContext(ctx)
@@ -240,11 +244,15 @@ func allRangeDescriptors(ctx context.Context, txn *kv.Txn) ([]roachpb.RangeDescr
 }
 
 // clusterNodeCount returns the approximate number of nodes in the cluster.
-func clusterNodeCount(g *gossip.Gossip) int {
+func clusterNodeCount(gw gossip.DeprecatedGossip) (int, error) {
+	g, err := gw.Optional(47971)
+	if err != nil {
+		return 0, err
+	}
 	var nodes int
 	_ = g.IterateInfos(gossip.KeyNodeIDPrefix, func(_ string, _ gossip.Info) error {
 		nodes++
 		return nil
 	})
-	return nodes
+	return nodes, nil
 }
diff --git a/pkg/gossip/gossip.go b/pkg/gossip/gossip.go
@@ -68,6 +68,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/rpc"
 	"github.com/cockroachdb/cockroach/pkg/rpc/nodedialer"
 	"github.com/cockroachdb/cockroach/pkg/util"
+	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/metric"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
@@ -1614,3 +1615,39 @@ func (g *Gossip) findClient(match func(*client) bool) *client {
 	}
 	return nil
 }
+
+// MakeDeprecatedGossip initializes a DeprecatedGossip instance. See
+// MakeTenantSQLDeprecatedWrapper for details.
+func MakeDeprecatedGossip(g *Gossip, tenant bool) DeprecatedGossip {
+	return DeprecatedGossip{
+		w: errorutil.MakeTenantSQLDeprecatedWrapper(g, tenant),
+	}
+}
+
+// DeprecatedGossip is a Gossip instance in a SQL tenant server, which is in the
+// process of being phased out.
+//
+// See the comments on TenantSQLDeprecatedWrapper for details.
+type DeprecatedGossip struct {
+	w errorutil.TenantSQLDeprecatedWrapper
+}
+
+// Deprecated trades a Github issue tracking the removal of the call for the
+// wrapped Gossip instance.
+func (dg DeprecatedGossip) Deprecated(issueNo int) *Gossip {
+	// NB: some tests use a nil Gossip.
+	g, _ := dg.w.Deprecated(issueNo).(*Gossip)
+	return g
+}
+
+// Optional returns the Gossip instance if the wrapper was set up to allow it.
+// Otherwise, it returns an error referring to the optionally passed in issues.
+func (dg DeprecatedGossip) Optional(issueNos ...int) (*Gossip, error) {
+	v, err := dg.w.Optional(issueNos...)
+	if err != nil {
+		return nil, err
+	}
+	// NB: some tests use a nil Gossip.
+	g, _ := v.(*Gossip)
+	return g, nil
+}
diff --git a/pkg/server/server.go b/pkg/server/server.go
@@ -514,7 +514,7 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) {
 				return sStatus, true
 			},
 			nodeLiveness:           nodeLiveness,
-			gossip:                 g,
+			gossip:                 gossip.MakeDeprecatedGossip(g, false /* tenant */),
 			nodeDialer:             nodeDialer,
 			grpcServer:             grpcServer.Server,
 			recorder:               recorder,

diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go
@@ -112,7 +112,7 @@ type sqlServerOptionalArgs struct {
 	// Gossip is relied upon by distSQLCfg (execinfra.ServerConfig), the executor
 	// config, the DistSQL planner, the table statistics cache, the statements
 	// diagnostics registry, and the lease manager.
-	gossip *gossip.Gossip
+	gossip gossip.DeprecatedGossip
 	// Used by DistSQLConfig and DistSQLPlanner.
 	nodeDialer *nodedialer.Dialer
 	// To register blob and DistSQL servers.
@@ -400,15 +400,15 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*sqlServer, error) {
 			cfg.rpcContext,
 			distSQLServer,
 			cfg.distSender,
-			cfg.gossip,
+			cfg.gossip.Deprecated(47900),
 			cfg.stopper,
 			cfg.nodeLiveness,
 			cfg.nodeDialer,
 		),
 
 		TableStatsCache: stats.NewTableStatisticsCache(
 			cfg.SQLTableStatCacheSize,
-			cfg.gossip,
+			cfg.gossip.Deprecated(47925),
 			cfg.db,
 			cfg.circularInternalExecutor,
 		),
@@ -527,13 +527,17 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*sqlServer, error) {
 	)
 	execCfg.InternalExecutor = cfg.circularInternalExecutor
 	stmtDiagnosticsRegistry := stmtdiagnostics.NewRegistry(
-		cfg.circularInternalExecutor, cfg.db, cfg.gossip, cfg.Settings)
+		cfg.circularInternalExecutor,
+		cfg.db,
+		cfg.gossip.Deprecated(47893),
+		cfg.Settings,
+	)
 	if statusServer, ok := cfg.statusServer(); ok {
 		statusServer.setStmtDiagnosticsRequester(stmtDiagnosticsRegistry)
 	}
 	execCfg.StmtDiagnosticsRecorder = stmtDiagnosticsRegistry
 
-	leaseMgr.RefreshLeases(cfg.stopper, cfg.db, cfg.gossip)
+	leaseMgr.RefreshLeases(cfg.stopper, cfg.db, cfg.gossip.Deprecated(47150))
 	leaseMgr.PeriodicallyRefreshSomeLeases()
 
 	temporaryObjectCleaner := sql.NewTemporaryObjectCleaner(

diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go
@@ -486,7 +486,7 @@ func testSQLServerArgs(ts *TestServer) sqlServerArgs {
 			distSender:   ds,
 			statusServer: noStatusServer,
 			nodeLiveness: nl,
-			gossip:       g,
+			gossip:       gossip.MakeDeprecatedGossip(g, true /* tenant */),
 			nodeDialer:   nd,
 			grpcServer:   dummyRPCServer,
 			recorder:     dummyRecorder,

diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go
@@ -1213,7 +1213,8 @@ func injectTableStats(
 	// update is handled asynchronously).
 	params.extendedEvalCtx.ExecCfg.TableStatsCache.InvalidateTableStats(params.ctx, desc.ID)
 
-	return stats.GossipTableStatAdded(params.extendedEvalCtx.ExecCfg.Gossip, desc.ID)
+	return stats.GossipTableStatAdded(
+		params.extendedEvalCtx.ExecCfg.Gossip.Deprecated(47925), desc.ID)
 }
 
 func (p *planner) removeColumnComment(

diff --git a/pkg/sql/cancel_queries.go b/pkg/sql/cancel_queries.go
@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/errors"
 )
 
@@ -46,7 +47,7 @@ func (n *cancelQueriesNode) Next(params runParams) (bool, error) {
 
 	statusServer, ok := params.extendedEvalCtx.StatusServer()
 	if !ok {
-		return false, pgerror.UnsupportedWithMultiTenancy()
+		return false, errorutil.UnsupportedWithMultiTenancy()
 	}
 
 	queryIDString, ok := tree.AsDString(datum)

diff --git a/pkg/sql/cancel_sessions.go b/pkg/sql/cancel_sessions.go
@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/errors"
 )
 
@@ -46,7 +47,7 @@ func (n *cancelSessionsNode) Next(params runParams) (bool, error) {
 
 	statusServer, ok := params.extendedEvalCtx.StatusServer()
 	if !ok {
-		return false, pgerror.UnsupportedWithMultiTenancy()
+		return false, errorutil.UnsupportedWithMultiTenancy()
 	}
 	sessionIDString, ok := tree.AsDString(datum)
 	if !ok {

diff --git a/pkg/sql/conn_executor.go b/pkg/sql/conn_executor.go
@@ -318,12 +318,13 @@ func makeMetrics(internal bool) Metrics {
 
 // Start starts the Server's background processing.
 func (s *Server) Start(ctx context.Context, stopper *stop.Stopper) {
-	gossipUpdateC := s.cfg.Gossip.RegisterSystemConfigChannel()
+	g := s.cfg.Gossip.Deprecated(47150)
+	gossipUpdateC := g.RegisterSystemConfigChannel()
 	stopper.RunWorker(ctx, func(ctx context.Context) {
 		for {
 			select {
 			case <-gossipUpdateC:
-				sysCfg := s.cfg.Gossip.GetSystemConfig()
+				sysCfg := g.GetSystemConfig()
 				s.dbCache.updateSystemConfig(sysCfg)
 			case <-stopper.ShouldStop():
 				return