sql: remove sql.catalog.unsafe_skip_system_config_trigger.enabled

This cluster setting is no longer useful.

Release note: None

pkg/sql/catalog/descs/txn.go

@@ -17,7 +17,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/kv"
-	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/lease"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlutil"
@@ -28,21 +27,6 @@ import (
 
 var errTwoVersionInvariantViolated = errors.Errorf("two version invariant violated")
 
-// UnsafeSkipSystemConfigTrigger will prevent setting the system config
-// trigger for transactions which write to tables in the system config. The
-// implication of setting this to true is that various subsystems which
-// rely on that trigger, such as zone configs and replication reports, will
-// not work. This can be used to accelerate high-frequency schema changes
-// like during an ORM test suite.
-var UnsafeSkipSystemConfigTrigger = settings.RegisterBoolSetting(
-	settings.SystemOnly,
-	"sql.catalog.unsafe_skip_system_config_trigger.enabled",
-	"avoid setting the system config trigger in transactions which write to "+
-		"the system config. This will unlock performance at the cost of breaking "+
-		"table splits, zone configuration propagation, and cluster settings",
-	false,
-)
-
 // Txn enables callers to run transactions with a *Collection such that all
 // retrieved immutable descriptors are properly leased and all mutable
 // descriptors are handled. The function deals with verifying the two version
@@ -89,10 +73,9 @@ func (cf *CollectionFactory) Txn(
 			deletedDescs = catalog.DescriptorIDSet{}
 			descsCol = cf.MakeCollection(ctx, nil /* temporarySchemaProvider */)
 			defer descsCol.ReleaseAll(ctx)
-			if !UnsafeSkipSystemConfigTrigger.Get(&cf.settings.SV) &&
-				!cf.settings.Version.IsActive(
-					ctx, clusterversion.DisableSystemConfigGossipTrigger,
-				) {
+			if !cf.settings.Version.IsActive(
+				ctx, clusterversion.DisableSystemConfigGossipTrigger,
+			) {
 				if err := txn.DeprecatedSetSystemConfigTrigger(
 					cf.leaseMgr.Codec().ForSystemTenant(),
 				); err != nil {

pkg/sql/gcjob/descriptor_utils.go

@@ -75,10 +75,9 @@ func deleteDatabaseZoneConfig(
 		return nil
 	}
 	return db.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
-		if !descs.UnsafeSkipSystemConfigTrigger.Get(&settings.SV) &&
-			!settings.Version.IsActive(
-				ctx, clusterversion.DisableSystemConfigGossipTrigger,
-			) {
+		if !settings.Version.IsActive(
+			ctx, clusterversion.DisableSystemConfigGossipTrigger,
+		) {
 			if err := txn.DeprecatedSetSystemConfigTrigger(codec.ForSystemTenant()); err != nil {
 				return err
 			}

pkg/sql/opt/exec/execbuilder/BUILD.bazel

@@ -18,7 +18,6 @@ go_library(
         "//pkg/server/telemetry",
         "//pkg/sql/catalog/colinfo",
         "//pkg/sql/catalog/descpb",
-        "//pkg/sql/catalog/descs",
         "//pkg/sql/lexbase",
         "//pkg/sql/mutations",
         "//pkg/sql/opt",

pkg/sql/opt/exec/execbuilder/relational.go

@@ -20,7 +20,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
-	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/constraint"
@@ -160,10 +159,9 @@ func (b *Builder) buildRelational(e memo.RelExpr) (execPlan, error) {
 		// `BEGIN; INSERT INTO ...; CREATE TABLE IF NOT EXISTS ...; COMMIT;`
 		// where the table already exists. This will generate some false schema
 		// cache refreshes, but that's expected to be quite rare in practice.
-		if !descs.UnsafeSkipSystemConfigTrigger.Get(&b.evalCtx.Settings.SV) &&
-			!b.evalCtx.Settings.Version.IsActive(
-				b.evalCtx.Ctx(), clusterversion.DisableSystemConfigGossipTrigger,
-			) {
+		if !b.evalCtx.Settings.Version.IsActive(
+			b.evalCtx.Ctx(), clusterversion.DisableSystemConfigGossipTrigger,
+		) {
 			if err := b.evalCtx.Txn.DeprecatedSetSystemConfigTrigger(b.evalCtx.Codec.ForSystemTenant()); err != nil {
 				return execPlan{}, errors.WithSecondaryError(
 					unimplemented.NewWithIssuef(26508,

pkg/sql/plan.go

@@ -17,7 +17,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
-	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
 	"github.com/cockroachdb/cockroach/pkg/sql/execstats"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/exec"
@@ -542,11 +541,9 @@ func (p *planner) maybePlanHook(ctx context.Context, stmt tree.Statement) (planN
 // Mark transaction as operating on the system DB if the descriptor id
 // is within the SystemConfig range.
 func (p *planner) maybeSetSystemConfig(id descpb.ID) error {
-	if !descpb.IsSystemConfigID(id) ||
-		descs.UnsafeSkipSystemConfigTrigger.Get(&p.EvalContext().Settings.SV) ||
-		p.execCfg.Settings.Version.IsActive(
-			p.EvalContext().Ctx(), clusterversion.DisableSystemConfigGossipTrigger,
-		) {
+	if !descpb.IsSystemConfigID(id) || p.execCfg.Settings.Version.IsActive(
+		p.EvalContext().Ctx(), clusterversion.DisableSystemConfigGossipTrigger,
+	) {
 		return nil
 	}
 	// Mark transaction as operating on the system DB.

pkg/sql/schema_changer.go

@@ -2645,7 +2645,7 @@ func DeleteTableDescAndZoneConfig(
 ) error {
 	log.Infof(ctx, "removing table descriptor and zone config for table %d", tableDesc.GetID())
 	return db.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
-		if !descs.UnsafeSkipSystemConfigTrigger.Get(&settings.SV) && !settings.Version.IsActive(
+		if !settings.Version.IsActive(
 			ctx, clusterversion.DisableSystemConfigGossipTrigger,
 		) {
 			if err := txn.DeprecatedSetSystemConfigTrigger(codec.ForSystemTenant()); err != nil {

pkg/sql/set_cluster_setting.go

@@ -171,21 +171,6 @@ func (n *setClusterSettingNode) startExec(params runParams) error {
 		return errors.Errorf("SET CLUSTER SETTING cannot be used inside a transaction")
 	}
 
-	// Set the system config trigger explicitly here as it might not happen
-	// implicitly due to the setting of the
-	// sql.catalog.unsafe_skip_system_config_trigger.enabled cluster setting.
-	// The usage of gossip to propagate cluster settings in the system tenant
-	// will be fixed in an upcoming PR with #70566.
-	if !params.EvalContext().Settings.Version.IsActive(
-		params.ctx, clusterversion.DisableSystemConfigGossipTrigger,
-	) {
-		if err := params.p.EvalContext().Txn.DeprecatedSetSystemConfigTrigger(
-			params.EvalContext().Codec.ForSystemTenant(),
-		); err != nil {
-			return err
-		}
-	}
-
 	execCfg := params.extendedEvalCtx.ExecCfg
 	var expectedEncodedValue string
 	if err := execCfg.DB.Txn(params.ctx, func(ctx context.Context, txn *kv.Txn) error {