multitenant: merge serverpb.RegionsServer into serverpb.TenantStatusS…

…erver Fixes #92598 Merge these 2 interfaces in anticipation for future work where more tenant functionality will be added to TenantStatusServer instead of creating an interface per function. Release note: None
cockroachdb · Nov 28, 2022 · aff028a · aff028a
1 parent 1a6e9f8
commit aff028a
Show file tree

Hide file tree

Showing 11 changed files with 17 additions and 40 deletions.
diff --git a/pkg/ccl/kvccl/kvtenantccl/connector.go b/pkg/ccl/kvccl/kvtenantccl/connector.go
@@ -116,11 +116,6 @@ var _ rangecache.RangeDescriptorDB = (*Connector)(nil)
 // network.
 var _ config.SystemConfigProvider = (*Connector)(nil)
 
-// Connector is capable of find the region of every node in the cluster.
-// This is necessary for region validation for zone configurations and
-// multi-region primitives.
-var _ serverpb.RegionsServer = (*Connector)(nil)
-
 // Connector is capable of finding debug information about the current
 // tenant within the cluster. This is necessary for things such as
 // debug zip and range reports.
@@ -415,7 +410,7 @@ func (c *Connector) RangeLookup(
 	return nil, nil, ctx.Err()
 }
 
-// Regions implements the serverpb.RegionsServer interface.
+// Regions implements the serverpb.TenantStatusServer interface.
 func (c *Connector) Regions(
 	ctx context.Context, req *serverpb.RegionsRequest,
 ) (resp *serverpb.RegionsResponse, _ error) {

diff --git a/pkg/kv/kvclient/kvtenant/connector.go b/pkg/kv/kvclient/kvtenant/connector.go
@@ -58,12 +58,7 @@ type Connector interface {
 	// (e.g. is the Range being requested owned by the requesting tenant?).
 	rangecache.RangeDescriptorDB
 
-	// RegionsServer provides access to a tenant's available regions. This is
-	// necessary for region validation for zone configurations and multi-region
-	// primitives.
-	serverpb.RegionsServer
-
-	// TenantStatusServer is the subset of the serverpb.StatusInterface that is
+	// TenantStatusServer is the subset of the serverpb.StatusServer that is
 	// used by the SQL system to query for debug information, such as tenant-specific
 	// range reports.
 	serverpb.TenantStatusServer

diff --git a/pkg/server/server.go b/pkg/server/server.go
@@ -891,7 +891,6 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) {
 		protectedtsProvider:      protectedtsProvider,
 		rangeFeedFactory:         rangeFeedFactory,
 		sqlStatusServer:          sStatus,
-		regionsServer:            sStatus,
 		tenantStatusServer:       sStatus,
 		tenantUsageServer:        tenantUsage,
 		monitorAndMetrics:        sqlMonitorAndMetrics,

diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go
@@ -340,9 +340,6 @@ type sqlServerArgs struct {
 	// Used to watch settings and descriptor changes.
 	rangeFeedFactory *rangefeed.Factory
 
-	// Used to query valid regions on the server.
-	regionsServer serverpb.RegionsServer
-
 	// Used to query status information useful for debugging on the server.
 	tenantStatusServer serverpb.TenantStatusServer
 
@@ -847,7 +844,6 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
 		DistSQLSrv:                distSQLServer,
 		NodesStatusServer:         cfg.nodesStatusServer,
 		SQLStatusServer:           cfg.sqlStatusServer,
-		RegionsServer:             cfg.regionsServer,
 		SessionRegistry:           cfg.sessionRegistry,
 		ClosedSessionCache:        cfg.closedSessionCache,
 		ContentionRegistry:        contentionRegistry,

diff --git a/pkg/server/serverpb/status.go b/pkg/server/serverpb/status.go
@@ -16,7 +16,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 )
 
-// SQLStatusServer is a smaller version of the serverpb.StatusInterface which
+// SQLStatusServer is a smaller version of the serverpb.StatusServer which
 // includes only the methods used by the SQL subsystem.
 type SQLStatusServer interface {
 	ListSessions(context.Context, *ListSessionsRequest) (*ListSessionsResponse, error)
@@ -71,20 +71,14 @@ type NodesStatusServer interface {
 	ListNodesInternal(context.Context, *NodesRequest) (*NodesResponse, error)
 }
 
-// RegionsServer is the subset of the serverpb.StatusInterface that is used
-// by the SQL system to query for available regions.
-// It is available for tenants.
-type RegionsServer interface {
-	Regions(context.Context, *RegionsRequest) (*RegionsResponse, error)
-}
-
-// TenantStatusServer is the subset of the serverpb.StatusInterface that is
+// TenantStatusServer is the subset of the serverpb.StatusServer that is
 // used by tenants to query for debug information, such as tenant-specific
 // range reports.
 //
 // It is available for all tenants.
 type TenantStatusServer interface {
 	TenantRanges(context.Context, *TenantRangesRequest) (*TenantRangesResponse, error)
+	Regions(context.Context, *RegionsRequest) (*RegionsResponse, error)
 }
 
 // OptionalNodesStatusServer returns the wrapped NodesStatusServer, if it is

diff --git a/pkg/server/status.go b/pkg/server/status.go
@@ -1374,7 +1374,7 @@ func (s *statusServer) Profile(
 	return profileLocal(ctx, req, s.st)
 }
 
-// Regions implements the serverpb.Status interface.
+// Regions implements the serverpb.StatusServer interface.
 func (s *statusServer) Regions(
 	ctx context.Context, req *serverpb.RegionsRequest,
 ) (*serverpb.RegionsResponse, error) {

diff --git a/pkg/server/tenant.go b/pkg/server/tenant.go
@@ -913,7 +913,6 @@ func makeTenantSQLServerArgs(
 		circularJobRegistry:      circularJobRegistry,
 		protectedtsProvider:      protectedTSProvider,
 		rangeFeedFactory:         rangeFeedFactory,
-		regionsServer:            tenantConnect,
 		tenantStatusServer:       tenantConnect,
 		costController:           costController,
 		monitorAndMetrics:        monitorAndMetrics,

diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go
@@ -2817,7 +2817,7 @@ CREATE TABLE crdb_internal.create_function_statements (
 				tree.NewDInt(tree.DInt(fnIDToScID[fnDesc.GetID()])), // schema_id
 				tree.NewDString(fnIDToScName[fnDesc.GetID()]),       // schema_name
 				tree.NewDInt(tree.DInt(fnDesc.GetID())),             // function_id
-				tree.NewDString(fnDesc.GetName()),                   //function_name
+				tree.NewDString(fnDesc.GetName()),                   // function_name
 				tree.NewDString(tree.AsString(treeNode)),            // create_statement
 			)
 			if err != nil {
@@ -4592,7 +4592,7 @@ CREATE TABLE crdb_internal.regions (
 )
 	`,
 	populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error {
-		resp, err := p.extendedEvalCtx.RegionsServer.Regions(ctx, &serverpb.RegionsRequest{})
+		resp, err := p.extendedEvalCtx.TenantStatusServer.Regions(ctx, &serverpb.RegionsRequest{})
 		if err != nil {
 			return err
 		}

diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go
@@ -1177,7 +1177,6 @@ type ExecutorConfig struct {
 	// available when not running as a system tenant.
 	SQLStatusServer    serverpb.SQLStatusServer
 	TenantStatusServer serverpb.TenantStatusServer
-	RegionsServer      serverpb.RegionsServer
 	MetricsRecorder    nodeStatusGenerator
 	SessionRegistry    *SessionRegistry
 	ClosedSessionCache *ClosedSessionCache

diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go
@@ -73,10 +73,10 @@ type extendedEvalContext struct {
 	// tenants.
 	NodesStatusServer serverpb.OptionalNodesStatusServer
 
-	// RegionsServer gives access to valid regions in the cluster.
-	RegionsServer serverpb.RegionsServer
+	// TenantStatusServer gives access to tenant status in the cluster.
+	TenantStatusServer serverpb.TenantStatusServer
 
-	// SQLStatusServer gives access to a subset of the serverpb.Status service
+	// SQLStatusServer gives access to a subset of the serverpb.StatusServer
 	// that is available to both system and non-system tenants.
 	SQLStatusServer serverpb.SQLStatusServer
 
@@ -127,7 +127,7 @@ func (evalCtx *extendedEvalContext) copyFromExecCfg(execCfg *ExecutorConfig) {
 	evalCtx.NodeID = execCfg.NodeInfo.NodeID
 	evalCtx.Locality = execCfg.Locality
 	evalCtx.NodesStatusServer = execCfg.NodesStatusServer
-	evalCtx.RegionsServer = execCfg.RegionsServer
+	evalCtx.TenantStatusServer = execCfg.TenantStatusServer
 	evalCtx.SQLStatusServer = execCfg.SQLStatusServer
 	evalCtx.DistSQLPlanner = execCfg.DistSQLPlanner
 	evalCtx.VirtualSchemas = execCfg.VirtualSchemas

diff --git a/pkg/sql/set_zone_config.go b/pkg/sql/set_zone_config.go
@@ -988,7 +988,7 @@ func validateZoneAttrsAndLocalities(
 		}
 		return validateZoneAttrsAndLocalitiesForSystemTenant(ctx, ss.ListNodesInternal, zone)
 	}
-	return validateZoneLocalitiesForSecondaryTenants(ctx, execCfg.RegionsServer.Regions, zone)
+	return validateZoneLocalitiesForSecondaryTenants(ctx, execCfg.TenantStatusServer.Regions, zone)
 }
 
 // validateZoneAttrsAndLocalitiesForSystemTenant performs all the constraint/
@@ -1046,13 +1046,13 @@ func validateZoneAttrsAndLocalitiesForSystemTenant(
 // validateZoneLocalitiesForSecondaryTenants performs all the constraint/lease
 // preferences validation for secondary tenants. Secondary tenants are only
 // allowed to reference locality attributes as they only have access to region
-// information via the RegionServer. Even then, they're only allowed to
-// reference the "region" and "zone" tiers.
+// information via the serverpb.TenantStatusServer. Even then, they're only
+// allowed to reference the "region" and "zone" tiers.
 //
 // Unlike the system tenant, we also validate prohibited constraints. This is
 // because secondary tenant must operate in the narrow view exposed via the
-// RegionServer and are not allowed to configure arbitrary constraints
-// (required or otherwise).
+// serverpb.TenantStatusServer and are not allowed to configure arbitrary
+// constraints (required or otherwise).
 func validateZoneLocalitiesForSecondaryTenants(
 	ctx context.Context, getRegions regionsGetter, zone *zonepb.ZoneConfig,
 ) error {