sql: remove enforce_home_region_follower_reads_enabled

In #97827 we added a new ability to dynamically determine the home
region of a row when a query failed due to `enforce_home_region`.
Unfortunately, the strategy of switching to follower reads after a
savepoint rollback is no longer permitted by the KV layer. This commit
removes the dynamic home region lookup ability.

Fixes: #113765

Release note (sql change): The functionality provided by session
variable `enforce_home_region_follower_reads_enabled` was deprecated in
v24.2.4 and is now removed. (The variable itself remains for backward
compatibility but has no effect.) (Note that related session variable
`enforce_home_region` is _not_ deprecated and still functions normally.)

Author: michae2

pkg/ccl/logictestccl/testdata/logic_test/multi_region_remote_access_error

@@ -839,19 +839,6 @@ func (l *Locality) Set(value string) error {
 	return nil
 }
 
-// CopyReplaceKeyValue makes a copy of this locality, replacing any tier in the
-// copy having the specified `key` with the new specified `value`.
-func (l *Locality) CopyReplaceKeyValue(key, value string) Locality {
-	tiers := make([]Tier, len(l.Tiers))
-	for i := range l.Tiers {
-		tiers[i] = l.Tiers[i]
-		if tiers[i].Key == key {
-			tiers[i].Value = value
-		}
-	}
-	return Locality{Tiers: tiers}
-}
-
 // Find searches the locality's tiers for the input key, returning its value if
 // present.
 func (l *Locality) Find(key string) (value string, ok bool) {

pkg/roachpb/metadata.go

@@ -977,9 +977,6 @@ func (s *vectorizedFlowCreator) setupInput(
 			var err error
 			if input.EnforceHomeRegionError != nil {
 				err = input.EnforceHomeRegionError.ErrorDetail(ctx)
-				if flowCtx.EvalCtx.SessionData().EnforceHomeRegionFollowerReadsEnabled {
-					err = execinfra.NewDynamicQueryHasNoHomeRegionError(err)
-				}
 			}
 			sync := colexec.NewSerialUnorderedSynchronizer(
 				flowCtx, processorID, allocator, input.ColumnTypes, inputStreamOps,

pkg/sql/colflow/vectorized_flow.go

@@ -42,7 +42,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/schematelemetry/schematelemetrycontroller"
 	"github.com/cockroachdb/cockroach/pkg/sql/clusterunique"
 	"github.com/cockroachdb/cockroach/pkg/sql/contention/txnidcache"
-	"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/execstats"
 	"github.com/cockroachdb/cockroach/pkg/sql/idxrecommendations"
 	"github.com/cockroachdb/cockroach/pkg/sql/idxusage"
@@ -3543,14 +3542,6 @@ func (ex *connExecutor) makeErrEvent(err error, stmt tree.Statement) (fsm.Event,
 	}
 
 	retryable := errIsRetryable(err)
-	if retryable && execinfra.IsDynamicQueryHasNoHomeRegionError(err) {
-		// Retry only # of remote regions times if the retry is due to the
-		// enforce_home_region setting.
-		retryable = int(ex.state.mu.autoRetryCounter) < len(ex.planner.EvalContext().RemoteRegions)
-		if !retryable {
-			err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(err)
-		}
-	}
 	if retryable {
 		var rc rewindCapability
 		var canAutoRetry bool
@@ -4009,22 +4000,6 @@ func (ex *connExecutor) resetPlanner(
 ) {
 	p.resetPlanner(ctx, txn, ex.sessionData(), ex.state.mon, ex.sessionMon)
 	ex.maybeAdjustMaxTimestampBound(p, txn)
-	// Make sure the default locality specifies the actual gateway region at the
-	// start of query compilation. It could have been overridden to a remote
-	// region when the enforce_home_region session setting is true.
-	p.EvalContext().Locality = p.EvalContext().OriginalLocality
-	if execinfra.IsDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason) {
-		if int(ex.state.mu.autoRetryCounter) <= len(p.EvalContext().RemoteRegions) {
-			// Set a fake gateway region for use by the optimizer to inform its
-			// decision on which region to access first in locality-optimized
-			// scan and join operations. This setting does not affect the
-			// distsql planner, and local plans will continue to be run from the
-			// actual gateway region.
-			p.EvalContext().Locality = p.EvalContext().Locality.CopyReplaceKeyValue(
-				"region" /* key */, string(p.EvalContext().RemoteRegions[ex.state.mu.autoRetryCounter-1]),
-			)
-		}
-	}
 	ex.resetEvalCtx(&p.extendedEvalCtx, txn, stmtTS)
 }
 

pkg/sql/conn_executor.go

@@ -42,7 +42,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/physicalplan"
 	"github.com/cockroachdb/cockroach/pkg/sql/prep"
 	"github.com/cockroachdb/cockroach/pkg/sql/regions"
-	"github.com/cockroachdb/cockroach/pkg/sql/sem/asof"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
@@ -1046,36 +1045,6 @@ func (ex *connExecutor) execStmtInOpenState(
 		stmtCtx = ctx
 	}
 
-	var rollbackHomeRegionSavepoint *tree.RollbackToSavepoint
-	var releaseHomeRegionSavepoint *tree.ReleaseSavepoint
-	enforceHomeRegion := p.EnforceHomeRegion()
-	_, isSelectStmt := stmt.AST.(*tree.Select)
-	if enforceHomeRegion && ex.state.mu.txn.IsOpen() && isSelectStmt {
-		// Create a savepoint at a point before which rows were read so that we can
-		// roll back to it, which will allow the txn to be modified with a
-		// historical timestamp (so that the locality-optimized ops used for error
-		// reporting can run locally and not incur latency). This is currently only
-		// supported for SELECT statements.
-		// Add some unprintable ASCII characters to the name of the savepoint to
-		// decrease the likelihood of collision with a user-created savepoint.
-		const enforceHomeRegionSavepointName = "enforce_home_region_sp\x11\x12\x13"
-		s := &tree.Savepoint{Name: enforceHomeRegionSavepointName}
-		var event fsm.Event
-		var eventPayload fsm.EventPayload
-		if event, eventPayload, err = ex.execSavepointInOpenState(ctx, s, res); err != nil {
-			return event, eventPayload, err
-		}
-
-		releaseHomeRegionSavepoint = &tree.ReleaseSavepoint{Savepoint: enforceHomeRegionSavepointName}
-		rollbackHomeRegionSavepoint = &tree.RollbackToSavepoint{Savepoint: enforceHomeRegionSavepointName}
-		defer func() {
-			// The default case is to roll back the internally-generated savepoint
-			// after every request. We only need it if a retryable "query has no home
-			// region" error occurs.
-			ex.execRelease(ctx, releaseHomeRegionSavepoint, res)
-		}()
-	}
-
 	if ex.state.mu.autoRetryReason != nil {
 		p.autoRetryCounter = int(ex.state.mu.autoRetryCounter)
 		ex.sessionTracing.TraceRetryInformation(
@@ -1127,54 +1096,6 @@ func (ex *connExecutor) execStmtInOpenState(
 	}
 
 	if err = res.Err(); err != nil {
-		setErrorAndRestoreLocality := func(err error) {
-			res.SetError(err)
-			// We won't be faking the gateway region any more. Restore the original
-			// locality.
-			p.EvalContext().Locality = p.EvalContext().OriginalLocality
-		}
-		if execinfra.IsDynamicQueryHasNoHomeRegionError(err) {
-			if rollbackHomeRegionSavepoint != nil {
-				// A retryable "query has no home region" error has occurred.
-				// Roll back to the internal savepoint in preparation for the next
-				// planning and execution of this query with a different gateway region
-				// (as considered by the optimizer).
-				p.StmtNoConstantsWithHomeRegionEnforced = p.stmt.StmtNoConstants
-				event, eventPayload := ex.execRollbackToSavepointInOpenState(
-					ctx, rollbackHomeRegionSavepoint, res,
-				)
-				_, isTxnRestart := event.(eventTxnRestart)
-				rollbackToSavepointFailed := !isTxnRestart || eventPayload != nil
-				if ex.implicitTxn() && rollbackToSavepointFailed {
-					err = errors.AssertionFailedf(
-						"unable to roll back to internal savepoint for enforce_home_region",
-					)
-					setErrorAndRestoreLocality(err)
-				} else if rollbackToSavepointFailed || int(ex.state.mu.autoRetryCounter) == len(ex.planner.EvalContext().RemoteRegions) {
-					// If rollback to savepoint in the transaction failed (perhaps because
-					// the txn was aborted) and we're in an explicit transaction, or we
-					// have retried the statement using each remote region as a fake
-					// gateway region, then give up and return the generic "query has no
-					// home region" error message.
-					err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(err)
-					setErrorAndRestoreLocality(err)
-				}
-			} else {
-				err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(err)
-				setErrorAndRestoreLocality(err)
-			}
-		} else if execinfra.IsDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason) {
-			// If we are retrying a dynamic "query has no home region" error and
-			// we get a different error message when executing with locality-optimized
-			// ops using a different local region (for example, relation does not
-			// exist, due to the AOST read), return the original error message in
-			// non-retryable form.
-			errorMessage := err.Error()
-			if !strings.HasPrefix(errorMessage, execinfra.QueryNotRunningInHomeRegionMessagePrefix) {
-				err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason)
-				setErrorAndRestoreLocality(err)
-			}
-		}
 		return makeErrEvent(err)
 	}
 
@@ -2098,38 +2019,6 @@ func (ex *connExecutor) execStmtInOpenStateWithPausablePortal(
 		stmtCtx = ctx
 	}
 
-	var rollbackHomeRegionSavepoint *tree.RollbackToSavepoint
-	var releaseHomeRegionSavepoint *tree.ReleaseSavepoint
-	enforceHomeRegion := p.EnforceHomeRegion()
-	_, isSelectStmt := vars.stmt.AST.(*tree.Select)
-	// TODO(sql-sessions): ensure this is not broken for pausable portals.
-	// https://github.com/cockroachdb/cockroach/issues/99408
-	if enforceHomeRegion && ex.state.mu.txn.IsOpen() && isSelectStmt {
-		// Create a savepoint at a point before which rows were read so that we can
-		// roll back to it, which will allow the txn to be modified with a
-		// historical timestamp (so that the locality-optimized ops used for error
-		// reporting can run locally and not incur latency). This is currently only
-		// supported for SELECT statements.
-		// Add some unprintable ASCII characters to the name of the savepoint to
-		// decrease the likelihood of collision with a user-created savepoint.
-		const enforceHomeRegionSavepointName = "enforce_home_region_sp\x11\x12\x13"
-		s := &tree.Savepoint{Name: enforceHomeRegionSavepointName}
-		var event fsm.Event
-		var eventPayload fsm.EventPayload
-		if event, eventPayload, err = ex.execSavepointInOpenState(ctx, s, res); err != nil {
-			return event, eventPayload, err
-		}
-
-		releaseHomeRegionSavepoint = &tree.ReleaseSavepoint{Savepoint: enforceHomeRegionSavepointName}
-		rollbackHomeRegionSavepoint = &tree.RollbackToSavepoint{Savepoint: enforceHomeRegionSavepointName}
-		defer func() {
-			// The default case is to roll back the internally-generated savepoint
-			// after every request. We only need it if a retryable "query has no home
-			// region" error occurs.
-			ex.execRelease(ctx, releaseHomeRegionSavepoint, res)
-		}()
-	}
-
 	if ex.state.mu.autoRetryReason != nil {
 		p.autoRetryCounter = int(ex.state.mu.autoRetryCounter)
 		ex.sessionTracing.TraceRetryInformation(
@@ -2181,54 +2070,6 @@ func (ex *connExecutor) execStmtInOpenStateWithPausablePortal(
 	}
 
 	if err = res.Err(); err != nil {
-		setErrorAndRestoreLocality := func(err error) {
-			res.SetError(err)
-			// We won't be faking the gateway region any more. Restore the original
-			// locality.
-			p.EvalContext().Locality = p.EvalContext().OriginalLocality
-		}
-		if execinfra.IsDynamicQueryHasNoHomeRegionError(err) {
-			if rollbackHomeRegionSavepoint != nil {
-				// A retryable "query has no home region" error has occurred.
-				// Roll back to the internal savepoint in preparation for the next
-				// planning and execution of this query with a different gateway region
-				// (as considered by the optimizer).
-				p.StmtNoConstantsWithHomeRegionEnforced = p.stmt.StmtNoConstants
-				event, eventPayload := ex.execRollbackToSavepointInOpenState(
-					ctx, rollbackHomeRegionSavepoint, res,
-				)
-				_, isTxnRestart := event.(eventTxnRestart)
-				rollbackToSavepointFailed := !isTxnRestart || eventPayload != nil
-				if ex.implicitTxn() && rollbackToSavepointFailed {
-					err = errors.AssertionFailedf(
-						"unable to roll back to internal savepoint for enforce_home_region",
-					)
-					setErrorAndRestoreLocality(err)
-				} else if rollbackToSavepointFailed || int(ex.state.mu.autoRetryCounter) == len(ex.planner.EvalContext().RemoteRegions) {
-					// If rollback to savepoint in the transaction failed (perhaps because
-					// the txn was aborted) and we're in an explicit transaction, or we
-					// have retried the statement using each remote region as a fake
-					// gateway region, then give up and return the generic "query has no
-					// home region" error message.
-					err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(err)
-					setErrorAndRestoreLocality(err)
-				}
-			} else {
-				err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(err)
-				setErrorAndRestoreLocality(err)
-			}
-		} else if execinfra.IsDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason) {
-			// If we are retrying a dynamic "query has no home region" error and
-			// we get a different error message when executing with locality-optimized
-			// ops using a different local region (for example, relation does not
-			// exist, due to the AOST read), return the original error message in
-			// non-retryable form.
-			errorMessage := err.Error()
-			if !strings.HasPrefix(errorMessage, execinfra.QueryNotRunningInHomeRegionMessagePrefix) {
-				err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason)
-				setErrorAndRestoreLocality(err)
-			}
-		}
 		return makeErrEvent(err)
 	}
 
@@ -2288,23 +2129,6 @@ func (ex *connExecutor) handleAOST(ctx context.Context, stmt tree.Statement) err
 		return errors.AssertionFailedf(
 			"cannot handle AOST clause without a transaction",
 		)
-	} else if execinfra.IsDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason) {
-		asOfClause := tree.AsOfClause{Expr: followerReadTimestampExpr}
-		// Set the timestamp used by current_timestamp().
-		asOf, err := p.EvalAsOfTimestamp(ctx, asOfClause, asof.OptionAllowBoundedStaleness)
-		if err != nil {
-			return errors.AssertionFailedf(
-				"problem evaluating follower read timestamp for enforce_home_region dynamic error checking",
-			)
-		}
-		// Set up AOST in the txn so re-running of the query with different possible
-		// home regions does not have to read rows from remote regions.
-		p.extendedEvalCtx.SetTxnTimestamp(asOf.Timestamp.GoTime())
-		if err := ex.state.setHistoricalTimestamp(ctx, asOf.Timestamp); err != nil {
-			// If the table was just created, we may not be able to set a historical
-			// timestamp.
-			return execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason)
-		}
 	}
 	asOf, err := p.isAsOf(ctx, stmt)
 	if err != nil {
@@ -3141,36 +2965,6 @@ func (ex *connExecutor) dispatchToExecutionEngine(
 	if limitsErr := ex.handleTxnRowsWrittenReadLimits(ctx); limitsErr != nil && res.Err() == nil {
 		res.SetError(limitsErr)
 	}
-	if res.Err() == nil && err == nil {
-		autoRetryReason := ex.state.mu.autoRetryReason
-		if execinfra.IsDynamicQueryHasNoHomeRegionError(autoRetryReason) {
-			if homeRegion, ok := planner.EvalContext().Locality.Find("region"); ok &&
-				planner.StmtNoConstantsWithHomeRegionEnforced == planner.stmt.StmtNoConstants {
-				// If this is the same query as ran when the dynamic "query has no home
-				// region" error occurred, but this time it didn't error out, report
-				// back the query's home region.
-				err = pgerror.Newf(pgcode.QueryNotRunningInHomeRegion,
-					`%s. Try running the query from region '%s'. %s`,
-					execinfra.QueryNotRunningInHomeRegionMessagePrefix,
-					homeRegion,
-					sqlerrors.EnforceHomeRegionFurtherInfo,
-				)
-				res.SetError(err)
-				// We won't be faking the gateway region any more. Restore the original
-				// locality.
-				planner.EvalContext().Locality = planner.EvalContext().OriginalLocality
-				return nil
-			}
-			// If for some reason we're not running the same query as before, report
-			// the original "query has no home region" error in non-retryable form.
-			err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(autoRetryReason)
-			res.SetError(err)
-			// We won't be faking the gateway region any more. Restore the original
-			// locality.
-			planner.EvalContext().Locality = planner.EvalContext().OriginalLocality
-			return nil
-		}
-	}
 
 	return err
 }

pkg/sql/conn_executor_exec.go

@@ -354,7 +354,6 @@ func (ds *ServerImpl) setupFlow(
 			ReCache:                   ds.regexpCache,
 			ToCharFormatCache:         ds.toCharFormatCache,
 			Locality:                  ds.ServerConfig.Locality,
-			OriginalLocality:          ds.ServerConfig.Locality,
 			Tracer:                    ds.ServerConfig.Tracer,
 			Planner:                   &faketreeeval.DummyEvalPlanner{Monitor: monitor},
 			StreamManagerFactory:      &faketreeeval.DummyStreamManagerFactory{},

pkg/sql/distsql/server.go

@@ -4108,10 +4108,6 @@ func (m *sessionDataMutator) SetInjectRetryErrorsOnCommitEnabled(val bool) {
 	m.data.InjectRetryErrorsOnCommitEnabled = val
 }
 
-func (m *sessionDataMutator) SetEnforceHomeRegionFollowerReadsEnabled(val bool) {
-	m.data.EnforceHomeRegionFollowerReadsEnabled = val
-}
-
 func (m *sessionDataMutator) SetOptimizerAlwaysUseHistograms(val bool) {
 	m.data.OptimizerAlwaysUseHistograms = val
 }

pkg/sql/exec_util.go

@@ -6,7 +6,6 @@ go_library(
     name = "execinfra",
     srcs = [
         "base.go",
-        "errors.go",
         "flow_context.go",
         "metrics.go",
         "outboxbase.go",
@@ -52,7 +51,6 @@ go_library(
         "//pkg/sql/evalcatalog",
         "//pkg/sql/execinfra/execexpr",
         "//pkg/sql/execinfrapb",
-        "//pkg/sql/pgwire/pgerror",
         "//pkg/sql/rowenc",
         "//pkg/sql/rowenc/valueside",
         "//pkg/sql/rowinfra",