*: re-audit uses of ForSystemTenant

[stevendanna]

Describe the problem

Each call to ForSystemTenant represents a potential feature gap or bug in UA/MT. As of 3e4643e there are 125 of these in non-test files.

The following issues were a result of manually scanning the list of these call sites and making a judgement about whether they might represent a problem.

Major Concerns

• sql: reconsider remaining uses of DistributionTypeSystemTenantOnly #116534 (target fix: ??)
  • pkg/sql/distsql_physical_planner.go:4855:133: distribute := distributionType == DistributionTypeAlways || (distributionType == DistributionTypeSystemTenantOnly && evalCtx.Codec.ForSystemTenant())
  • sql,*: remove DistributionTypeSystemTenantOnly #116559
• sql: investigate whether mandatory splits at table and index boundaries should be used  #116536 (target fix: 23.2.1)
  • pkg/sql/index_split_scatter.go:55:15: if !is.codec.ForSystemTenant() || indexToBackfill.IsTemporaryIndexForBackfill() {
  • pkg/sql/importer/import_processor_planning.go:217:19: if evalCtx.Codec.ForSystemTenant() {
  • pkg/sql/backfill.go:2242:22: if sc.execCfg.Codec.ForSystemTenant() {
• sql: zone configuration not delete in secondary tenant #116539 (target fix: 23.2.1)
  • pkg/sql/schema_changer.go:3100:36: if codec := execCfg.Codec; codec.ForSystemTenant() {
  • sql: delete zone configuration in secondary tenant #116547
• sql: attempt to unsplit ranges in GC job for secondary tenants #116312 (target fix: 23.2.0)
  • pkg/sql/gcjob/gc_job.go:203:20: if !execCfg.Codec.ForSystemTenant() {
  • pkg/sql/gcjob/gc_job.go:228:20: if !execCfg.Codec.ForSystemTenant() {

Minor Concerns

• backupccl: attempt lowering the GCTTL in secondary tenant #116529
  • pkg/ccl/backupccl/restore_job.go:2549:12: if codec.ForSystemTenant() {
  • backupccl,sql: failed restore can set gcttl in tenant #116555
• sql: investigate whether session revival should be made available to UA clusters #116537
  • pkg/sql/conn_executor_exec.go:2754:79: AllowSessionRevival.Get(&ex.server.cfg.Settings.SV) && !ex.server.cfg.Codec.ForSystemTenant(),
  • pkg/sql/session_revival_token.go:43:75: AllowSessionRevival.Get(&p.ExecCfg().Settings.SV) && !p.ExecCfg().Codec.ForSystemTenant(),
  • pkg/sql/session_revival_token.go:79:77: if !AllowSessionRevival.Get(&p.ExecCfg().Settings.SV) || p.ExecCfg().Code
• pkg/sql/conn_executor.go:3925:26: if !ex.server.cfg.Codec.ForSystemTenant() || (the comment suggests that it was written with ephemeral SQL pods of serverless in mind, and now it will apply in UA too)
• pkg/sql/temporary_schema.go:476:13: if c.codec.ForSystemTenant() {: The comment says there is no harm here, but I wonder if that comment considered the UA case.
• pkg/sql/explain_bundle.go:69:26: } else if execCfg.Codec.ForSystemTenant() {
  • Yahor: this is only a very minor UX difference where in system tenant we get a direct link to download a stmt bundle whereas in the secondary tenant we'll need to navigate to it on the UI or use the SQL / CLI command
• pkg/server/server_sql.go:1317:11: if codec.ForSystemTenant() {
  • Yahor: currently, the key visualizer page is only implemented for system tenant, so in UA users will need to switch there to see it. Also, this feature is disabled by default, so it seems like a pretty minor limitation.

Call sites not believed to be a problem

This list out of order because of concurrent updates.

• pkg/sql/distsql_physical_planner.go:1853:19: return dsp.codec.ForSystemTenant() && planCtx.localityFilter.Empty()
• pkg/sql/gcjob/refresh_statuses.go:323:20: if execCfg.Codec.ForSystemTenant() &&
• pkg/sql/gcjob/refresh_statuses.go:411:20: if !execCfg.Codec.ForSystemTenant() {
• pkg/sql/set_cluster_setting.go:134:39: forSystemTenant := p.ExecCfg().Codec.ForSystemTenant()
• pkg/sql/set_cluster_setting.go:337:32: params.extendedEvalCtx.Codec.ForSystemTenant(),
• pkg/sql/set_var.go:68:93: if _, ok, _ := settings.LookupForLocalAccess(settings.SettingName(name), p.ExecCfg().Codec.ForSystemTenant()); ok {
• pkg/sql/conn_executor_exec.go:1884:30: if !planner.ExecCfg().Codec.ForSystemTenant() {
• pkg/sql/tenant_accessors.go:42:12: if !codec.ForSystemTenant() {
• pkg/sql/tenant_settings.go:55:22: if !p.execCfg.Codec.ForSystemTenant() {
• pkg/sql/tenant_settings.go:180:83: setting, ok, nameStatus := settings.LookupForLocalAccess(name, p.ExecCfg().Codec.ForSystemTenant())
• pkg/sql/tenant_settings.go:191:22: if !p.execCfg.Codec.ForSystemTenant() {
• pkg/sql/crdb_internal.go:2362:53: for _, k := range settings.Keys(p.ExecCfg().Codec.ForSystemTenant()) {
• pkg/sql/crdb_internal.go:2367:74: setting, _ := settings.LookupForLocalAccessByKey(k, p.ExecCfg().Codec.ForSystemTenant())
• pkg/sql/alter_database.go:583:22: if p.execCfg.Codec.ForSystemTenant() {
• pkg/sql/distsql_plan_bulk.go:48:15: if dsp.codec.ForSystemTenant() {
• pkg/sql/delegate/show_all_cluster_settings.go:102:22: if !d.evalCtx.Codec.ForSystemTenant() {
• pkg/sql/catalog/descs/txn.go:148:23: if !descsCol.codec().ForSystemTenant() {
• pkg/sql/catalog/bootstrap/initial_values.go:109:17: if !opts.Codec.ForSystemTenant() {
• pkg/sql/catalog/bootstrap/metadata.go:101:17:// AddDescriptorForSystemTenant is like AddDescriptor but only for the system
• pkg/sql/catalog/bootstrap/metadata.go:103:40:func (ms *MetadataSchema) AddDescriptorForSystemTenant(desc catalog.Descriptor) {
• pkg/sql/catalog/bootstrap/metadata.go:104:15: if !ms.codec.ForSystemTenant() {
• pkg/sql/catalog/bootstrap/metadata.go:113:14: if ms.codec.ForSystemTenant() {
• pkg/sql/catalog/bootstrap/metadata.go:174:15: if ms.codec.ForSystemTenant() {
• pkg/sql/catalog/bootstrap/metadata.go:176:47: // until clusterversion.V23_1DescIDSequenceForSystemTenant is removed.
• pkg/sql/catalog/bootstrap/metadata.go:238:14: if ms.codec.ForSystemTenant() {
• pkg/sql/catalog/bootstrap/metadata.go:353:12: if !codec.ForSystemTenant() {
• pkg/sql/catalog/bootstrap/metadata.go:404:22: target.AddDescriptorForSystemTenant(systemschema.TenantsTable)
• pkg/sql/catalog/bootstrap/metadata.go:451:22: target.AddDescriptorForSystemTenant(systemschema.TenantUsageTable)
• pkg/sql/catalog/bootstrap/metadata.go:453:22: target.AddDescriptorForSystemTenant(systemschema.SpanConfigurationsTable)
• pkg/sql/catalog/bootstrap/metadata.go:457:22: target.AddDescriptorForSystemTenant(systemschema.TenantSettingsTable)
• pkg/sql/catalog/bootstrap/metadata.go:471:22: target.AddDescriptorForSystemTenant(systemschema.SystemTaskPayloadsTable)
• pkg/sql/catalog/bootstrap/metadata.go:472:22: target.AddDescriptorForSystemTenant(systemschema.SystemTenantTasksTable)
• pkg/sql/catalog/bootstrap/metadata.go:475:22: target.AddDescriptorForSystemTenant(systemschema.TenantIDSequence)
• pkg/sql/catalog/bootstrap/metadata.go:485:55: // If adding a call to AddDescriptor or AddDescriptorForSystemTenant, please
• pkg/sql/catalog/bootstrap/metadata.go:486:38: // bump the value of NumSystemTablesForSystemTenant below. This constant is
• pkg/sql/catalog/bootstrap/metadata.go:492:19:// NumSystemTablesForSystemTenant is the number of system tables defined on
• pkg/sql/catalog/bootstrap/metadata.go:495:22:const NumSystemTablesForSystemTenant = 55
• pkg/sql/catalog/bootstrap/metadata.go:528:12: if !codec.ForSystemTenant() {
• pkg/sql/descriptor.go:294:22: if !p.execCfg.Codec.ForSystemTenant() && isSystemDatabase {
• pkg/sql/descriptor.go:361:22: if !p.execCfg.Codec.ForSystemTenant() &&
• pkg/sql/explain_plan.go:223:13: if !codec.ForSystemTenant() {
• pkg/sql/set_zone_config.go:544:30: if !params.p.execCfg.Codec.ForSystemTenant() {
• pkg/sql/set_zone_config.go:1040:19: if execCfg.Codec.ForSystemTenant() {
• pkg/sql/set_zone_config.go:1045:40: return validateZoneAttrsAndLocalitiesForSystemTenant(ctx, ss.ListNodesInternal, currentZone, newZone)
• pkg/sql/set_zone_config.go:1052:34:// validateZoneAttrsAndLocalitiesForSystemTenant performs constraint/ lease
• pkg/sql/set_zone_config.go:1065:36:func validateZoneAttrsAndLocalitiesForSystemTenant(
• pkg/sql/restricted_system_interface.go:55:27: forSystemTenant := codec.ForSystemTenant()
• pkg/sql/restricted_system_interface.go:86:23: if p.ExecCfg().Codec.ForSystemTenant() &&
• pkg/sql/distsql_spec_exec_factory.go:72:41: singleTenant: p.execCfg.Codec.ForSystemTenant(),
• pkg/sql/regions/region_provider.go:71:13: if p.codec.ForSystemTenant() {
• pkg/sql/exec_util.go:3868:11: if codec.ForSystemTenant() || setting.Get(&settings.SV) {
• pkg/server/server_systemlog_gc.go:201:45: forSystemTenant := sqlServer.execCfg.Codec.ForSystemTenant()
• pkg/server/settingswatcher/settings_watcher.go:347:72: setting, ok := settings.LookupForLocalAccessByKey(settingKey, s.codec.ForSystemTenant())
• pkg/server/settingswatcher/settings_watcher.go:352:14: if !s.codec.ForSystemTenant() {
• pkg/server/settingswatcher/settings_watcher.go:440:57: if key == clusterversion.KeyVersionSetting && !s.codec.ForSystemTenant() {
• pkg/server/settingswatcher/settings_watcher.go:640:66: setting, ok := settings.LookupForLocalAccessByKey(key, settings.ForSystemTenant)
• pkg/server/api_v2.go:203:57: if !route.tenantEnabled && !a.sqlServer.execCfg.Codec.ForSystemTenant() {
• pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go:248:14: if !s.codec.ForSystemTenant() && name != zonepb.DefaultZoneName {
• pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go:361:15: if !s.codec.ForSystemTenant() {
• pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go:586:13: if s.codec.ForSystemTenant() {
• pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go:603:14: if !s.codec.ForSystemTenant() {
• pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go:663:48: if !s.knobs.ConfigureScratchRange || !s.codec.ForSystemTenant() {
• pkg/spanconfig/spanconfigreconciler/reconciler.go:305:14: if !f.codec.ForSystemTenant() {
• pkg/spanconfig/spanconfigreconciler/reconciler.go:345:13: if f.codec.ForSystemTenant() {
• pkg/spanconfig/spanconfigreconciler/reconciler.go:630:15: if r.codec.ForSystemTenant() {
• pkg/sql/show_cluster_setting.go:79:33: } else if !p.execCfg.Codec.ForSystemTenant() {
• pkg/upgrade/upgrademanager/manager.go:122:11: if codec.ForSystemTenant() {
• pkg/upgrade/upgrademanager/manager.go:184:34: if isSystemUpgrade && !m.codec.ForSystemTenant() {
• pkg/upgrade/upgrademanager/manager.go:410:38: mustPersistFenceVersion := !m.codec.ForSystemTenant()
• pkg/upgrade/upgrademanager/manager.go:676:35: if isSystemMigration && !m.codec.ForSystemTenant() {
• pkg/sql/pgwire/auth_methods.go:669:75: if !sql.AllowSessionRevival.Get(&execCfg.Settings.SV) || execCfg.Codec.ForSystemTenant() {
• pkg/settings/registry.go:447:4:// ForSystemTenant can be passed to Lookup for code that runs only on the system
• pkg/settings/registry.go:449:7:const ForSystemTenant = true
• pkg/sql/show_cluster_setting.go:163:83: setting, ok, nameStatus := settings.LookupForLocalAccess(name, p.ExecCfg().Codec.ForSystemTenant())
• pkg/spanconfig/spanconfigstore/span_store.go:163:81: s.settings.Version.IsActive(ctx, clusterversion.V23_2_EnableRangeCoalescingForSystemTenant)) {
• pkg/server/server_sql.go:578:11: if codec.ForSystemTenant() {
• pkg/server/server_sql.go:623:70: canUseNodeDialerAsSQLInstanceDialer := isMixedSQLAndKVNode && codec.ForSystemTenant()
• pkg/server/server_sql.go:770:11: if codec.ForSystemTenant() {
• pkg/server/server_sql.go:1041:11: if codec.ForSystemTenant() {
• pkg/server/server_sql.go:1239:12: if codec.ForSystemTenant() {
• pkg/server/server_sql.go:1550:22: if !s.execCfg.Codec.ForSystemTenant() {
• pkg/server/server_sql.go:1591:21: if s.execCfg.Codec.ForSystemTenant() {
• pkg/server/server_sql.go:1716:22: if !s.execCfg.Codec.ForSystemTenant() && (s.serviceMode != mtinfopb.ServiceModeExternal) {
• pkg/cmd/github-pull-request-make/testdata/five_tests.diff:1937:38:@@ -27,7 +28,7 @@ func descIDSequenceForSystemTenant(
• pkg/sql/sem/builtins/builtins.go:4949:73: setting, ok, _ := settings.LookupForLocalAccess(name, evalCtx.Codec.ForSystemTenant())
• pkg/sql/sem/builtins/builtins.go:4979:73: setting, ok, _ := settings.LookupForLocalAccess(name, evalCtx.Codec.ForSystemTenant())
• pkg/clusterversion/cockroach_versions.go:219:32: // V23_2_EnableRangeCoalescingForSystemTenant enables range coalescing for
• pkg/clusterversion/cockroach_versions.go:221:29: V23_2_EnableRangeCoalescingForSystemTenant
• pkg/clusterversion/cockroach_versions.go:346:29: V23_2_EnableRangeCoalescingForSystemTenant: {Major: 23, Minor: 1, Internal: 8},
• pkg/server/server_controller_orchestration_method.go:112:20: // makeServerStateForSystemTenant returns a serverState for the
• pkg/server/server_controller_orchestration_method.go:114:17: makeServerStateForSystemTenant(nc *roachpb.TenantNameContainer, systemSrv orchestratedServer) serverState
• pkg/server/server_controller.go:142:64: catconstants.SystemTenantName: c.orchestrator.makeServerStateForSystemTenant(systemTenantNameContainer, systemServer),
• pkg/server/admin.go:2024:42: showSystem := s.sqlServer.execCfg.Codec.ForSystemTenant()
• pkg/server/admin.go:2027:21: target = settings.ForSystemTenant
• pkg/server/server_controller_channel_orchestrator.go:149:19:// makeServerStateForSystemTenant is part of the orchestrator interface.
• pkg/server/server_controller_channel_orchestrator.go:150:46:func (o *channelOrchestrator) makeServerStateForSystemTenant(
• pkg/ccl/backupccl/restore_planning.go:1924:25: if !p.ExecCfg().Codec.ForSystemTenant() {
• pkg/ccl/backupccl/restore_planning.go:2358:23: if p.ExecCfg().Codec.ForSystemTenant() ||
• pkg/ccl/backupccl/restore_planning.go:2390:24: if !p.ExecCfg().Codec.ForSystemTenant() &&
• pkg/ccl/backupccl/backup_job.go:1449:37: if jobDetails.FullCluster && codec.ForSystemTenant() && jobDetails.IncludeAllSecondaryTenants {
• pkg/ccl/backupccl/backup_planning.go:704:26: if !p.ExecCfg().Codec.ForSystemTenant() {
• pkg/ccl/backupccl/system_schema.go:144:16: if deps.codec.ForSystemTenant() {
• pkg/ccl/backupccl/restore_job.go:1380:68: if backupTenantID != roachpb.SystemTenantID && p.ExecCfg().Codec.ForSystemTenant() {
• pkg/keys/sql.go:179:4:// ForSystemTenant returns whether the encoder is bound to the system tenant.
• pkg/keys/sql.go:180:21:func (e sqlEncoder) ForSystemTenant() bool {
• pkg/keys/constants.go:295:73: // TODO(postamar): remove along with clusterversion.V23_1DescIDSequenceForSystemTenant
• pkg/cli/gen.go:239:46: for _, key := range settings.Keys(settings.ForSystemTenant) {
• pkg/cli/gen.go:240:68: setting, ok := settings.LookupForLocalAccessByKey(key, settings.ForSystemTenant)
• pkg/ccl/streamingccl/streamingest/stream_ingestion_planning.go:110:24: if !p.ExecCfg().Codec.ForSystemTenant() {
• pkg/ccl/streamingccl/streamingest/alter_replication_job.go:153:24: if !p.ExecCfg().Codec.ForSystemTenant() {

Jira issue: CRDB-34629