Skip to content

Commit c74f6a3

Browse files
craig[bot]michae2
andcommitted
Merge #148314
148314: sql: remove enforce_home_region_follower_reads_enabled r=DrewKimball a=michae2 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.) Co-authored-by: Michael Erickson <[email protected]>
2 parents 9e83fa3 + a3de7ea commit c74f6a3

File tree

26 files changed

+57
-526
lines changed

26 files changed

+57
-526
lines changed

pkg/ccl/logictestccl/testdata/logic_test/multi_region_remote_access_error

Lines changed: 24 additions & 84 deletions
Large diffs are not rendered by default.

pkg/roachpb/metadata.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -839,19 +839,6 @@ func (l *Locality) Set(value string) error {
839839
return nil
840840
}
841841

842-
// CopyReplaceKeyValue makes a copy of this locality, replacing any tier in the
843-
// copy having the specified `key` with the new specified `value`.
844-
func (l *Locality) CopyReplaceKeyValue(key, value string) Locality {
845-
tiers := make([]Tier, len(l.Tiers))
846-
for i := range l.Tiers {
847-
tiers[i] = l.Tiers[i]
848-
if tiers[i].Key == key {
849-
tiers[i].Value = value
850-
}
851-
}
852-
return Locality{Tiers: tiers}
853-
}
854-
855842
// Find searches the locality's tiers for the input key, returning its value if
856843
// present.
857844
func (l *Locality) Find(key string) (value string, ok bool) {

pkg/sql/colflow/vectorized_flow.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -977,9 +977,6 @@ func (s *vectorizedFlowCreator) setupInput(
977977
var err error
978978
if input.EnforceHomeRegionError != nil {
979979
err = input.EnforceHomeRegionError.ErrorDetail(ctx)
980-
if flowCtx.EvalCtx.SessionData().EnforceHomeRegionFollowerReadsEnabled {
981-
err = execinfra.NewDynamicQueryHasNoHomeRegionError(err)
982-
}
983980
}
984981
sync := colexec.NewSerialUnorderedSynchronizer(
985982
flowCtx, processorID, allocator, input.ColumnTypes, inputStreamOps,

pkg/sql/conn_executor.go

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ import (
4242
"github.com/cockroachdb/cockroach/pkg/sql/catalog/schematelemetry/schematelemetrycontroller"
4343
"github.com/cockroachdb/cockroach/pkg/sql/clusterunique"
4444
"github.com/cockroachdb/cockroach/pkg/sql/contention/txnidcache"
45-
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
4645
"github.com/cockroachdb/cockroach/pkg/sql/execstats"
4746
"github.com/cockroachdb/cockroach/pkg/sql/idxrecommendations"
4847
"github.com/cockroachdb/cockroach/pkg/sql/idxusage"
@@ -3543,14 +3542,6 @@ func (ex *connExecutor) makeErrEvent(err error, stmt tree.Statement) (fsm.Event,
35433542
}
35443543

35453544
retryable := errIsRetryable(err)
3546-
if retryable && execinfra.IsDynamicQueryHasNoHomeRegionError(err) {
3547-
// Retry only # of remote regions times if the retry is due to the
3548-
// enforce_home_region setting.
3549-
retryable = int(ex.state.mu.autoRetryCounter) < len(ex.planner.EvalContext().RemoteRegions)
3550-
if !retryable {
3551-
err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(err)
3552-
}
3553-
}
35543545
if retryable {
35553546
var rc rewindCapability
35563547
var canAutoRetry bool
@@ -4009,22 +4000,6 @@ func (ex *connExecutor) resetPlanner(
40094000
) {
40104001
p.resetPlanner(ctx, txn, ex.sessionData(), ex.state.mon, ex.sessionMon)
40114002
ex.maybeAdjustMaxTimestampBound(p, txn)
4012-
// Make sure the default locality specifies the actual gateway region at the
4013-
// start of query compilation. It could have been overridden to a remote
4014-
// region when the enforce_home_region session setting is true.
4015-
p.EvalContext().Locality = p.EvalContext().OriginalLocality
4016-
if execinfra.IsDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason) {
4017-
if int(ex.state.mu.autoRetryCounter) <= len(p.EvalContext().RemoteRegions) {
4018-
// Set a fake gateway region for use by the optimizer to inform its
4019-
// decision on which region to access first in locality-optimized
4020-
// scan and join operations. This setting does not affect the
4021-
// distsql planner, and local plans will continue to be run from the
4022-
// actual gateway region.
4023-
p.EvalContext().Locality = p.EvalContext().Locality.CopyReplaceKeyValue(
4024-
"region" /* key */, string(p.EvalContext().RemoteRegions[ex.state.mu.autoRetryCounter-1]),
4025-
)
4026-
}
4027-
}
40284003
ex.resetEvalCtx(&p.extendedEvalCtx, txn, stmtTS)
40294004
}
40304005

pkg/sql/conn_executor_exec.go

Lines changed: 0 additions & 206 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ import (
4242
"github.com/cockroachdb/cockroach/pkg/sql/physicalplan"
4343
"github.com/cockroachdb/cockroach/pkg/sql/prep"
4444
"github.com/cockroachdb/cockroach/pkg/sql/regions"
45-
"github.com/cockroachdb/cockroach/pkg/sql/sem/asof"
4645
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
4746
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
4847
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
@@ -1046,36 +1045,6 @@ func (ex *connExecutor) execStmtInOpenState(
10461045
stmtCtx = ctx
10471046
}
10481047

1049-
var rollbackHomeRegionSavepoint *tree.RollbackToSavepoint
1050-
var releaseHomeRegionSavepoint *tree.ReleaseSavepoint
1051-
enforceHomeRegion := p.EnforceHomeRegion()
1052-
_, isSelectStmt := stmt.AST.(*tree.Select)
1053-
if enforceHomeRegion && ex.state.mu.txn.IsOpen() && isSelectStmt {
1054-
// Create a savepoint at a point before which rows were read so that we can
1055-
// roll back to it, which will allow the txn to be modified with a
1056-
// historical timestamp (so that the locality-optimized ops used for error
1057-
// reporting can run locally and not incur latency). This is currently only
1058-
// supported for SELECT statements.
1059-
// Add some unprintable ASCII characters to the name of the savepoint to
1060-
// decrease the likelihood of collision with a user-created savepoint.
1061-
const enforceHomeRegionSavepointName = "enforce_home_region_sp\x11\x12\x13"
1062-
s := &tree.Savepoint{Name: enforceHomeRegionSavepointName}
1063-
var event fsm.Event
1064-
var eventPayload fsm.EventPayload
1065-
if event, eventPayload, err = ex.execSavepointInOpenState(ctx, s, res); err != nil {
1066-
return event, eventPayload, err
1067-
}
1068-
1069-
releaseHomeRegionSavepoint = &tree.ReleaseSavepoint{Savepoint: enforceHomeRegionSavepointName}
1070-
rollbackHomeRegionSavepoint = &tree.RollbackToSavepoint{Savepoint: enforceHomeRegionSavepointName}
1071-
defer func() {
1072-
// The default case is to roll back the internally-generated savepoint
1073-
// after every request. We only need it if a retryable "query has no home
1074-
// region" error occurs.
1075-
ex.execRelease(ctx, releaseHomeRegionSavepoint, res)
1076-
}()
1077-
}
1078-
10791048
if ex.state.mu.autoRetryReason != nil {
10801049
p.autoRetryCounter = int(ex.state.mu.autoRetryCounter)
10811050
ex.sessionTracing.TraceRetryInformation(
@@ -1127,54 +1096,6 @@ func (ex *connExecutor) execStmtInOpenState(
11271096
}
11281097

11291098
if err = res.Err(); err != nil {
1130-
setErrorAndRestoreLocality := func(err error) {
1131-
res.SetError(err)
1132-
// We won't be faking the gateway region any more. Restore the original
1133-
// locality.
1134-
p.EvalContext().Locality = p.EvalContext().OriginalLocality
1135-
}
1136-
if execinfra.IsDynamicQueryHasNoHomeRegionError(err) {
1137-
if rollbackHomeRegionSavepoint != nil {
1138-
// A retryable "query has no home region" error has occurred.
1139-
// Roll back to the internal savepoint in preparation for the next
1140-
// planning and execution of this query with a different gateway region
1141-
// (as considered by the optimizer).
1142-
p.StmtNoConstantsWithHomeRegionEnforced = p.stmt.StmtNoConstants
1143-
event, eventPayload := ex.execRollbackToSavepointInOpenState(
1144-
ctx, rollbackHomeRegionSavepoint, res,
1145-
)
1146-
_, isTxnRestart := event.(eventTxnRestart)
1147-
rollbackToSavepointFailed := !isTxnRestart || eventPayload != nil
1148-
if ex.implicitTxn() && rollbackToSavepointFailed {
1149-
err = errors.AssertionFailedf(
1150-
"unable to roll back to internal savepoint for enforce_home_region",
1151-
)
1152-
setErrorAndRestoreLocality(err)
1153-
} else if rollbackToSavepointFailed || int(ex.state.mu.autoRetryCounter) == len(ex.planner.EvalContext().RemoteRegions) {
1154-
// If rollback to savepoint in the transaction failed (perhaps because
1155-
// the txn was aborted) and we're in an explicit transaction, or we
1156-
// have retried the statement using each remote region as a fake
1157-
// gateway region, then give up and return the generic "query has no
1158-
// home region" error message.
1159-
err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(err)
1160-
setErrorAndRestoreLocality(err)
1161-
}
1162-
} else {
1163-
err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(err)
1164-
setErrorAndRestoreLocality(err)
1165-
}
1166-
} else if execinfra.IsDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason) {
1167-
// If we are retrying a dynamic "query has no home region" error and
1168-
// we get a different error message when executing with locality-optimized
1169-
// ops using a different local region (for example, relation does not
1170-
// exist, due to the AOST read), return the original error message in
1171-
// non-retryable form.
1172-
errorMessage := err.Error()
1173-
if !strings.HasPrefix(errorMessage, execinfra.QueryNotRunningInHomeRegionMessagePrefix) {
1174-
err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason)
1175-
setErrorAndRestoreLocality(err)
1176-
}
1177-
}
11781099
return makeErrEvent(err)
11791100
}
11801101

@@ -2098,38 +2019,6 @@ func (ex *connExecutor) execStmtInOpenStateWithPausablePortal(
20982019
stmtCtx = ctx
20992020
}
21002021

2101-
var rollbackHomeRegionSavepoint *tree.RollbackToSavepoint
2102-
var releaseHomeRegionSavepoint *tree.ReleaseSavepoint
2103-
enforceHomeRegion := p.EnforceHomeRegion()
2104-
_, isSelectStmt := vars.stmt.AST.(*tree.Select)
2105-
// TODO(sql-sessions): ensure this is not broken for pausable portals.
2106-
// https://github.com/cockroachdb/cockroach/issues/99408
2107-
if enforceHomeRegion && ex.state.mu.txn.IsOpen() && isSelectStmt {
2108-
// Create a savepoint at a point before which rows were read so that we can
2109-
// roll back to it, which will allow the txn to be modified with a
2110-
// historical timestamp (so that the locality-optimized ops used for error
2111-
// reporting can run locally and not incur latency). This is currently only
2112-
// supported for SELECT statements.
2113-
// Add some unprintable ASCII characters to the name of the savepoint to
2114-
// decrease the likelihood of collision with a user-created savepoint.
2115-
const enforceHomeRegionSavepointName = "enforce_home_region_sp\x11\x12\x13"
2116-
s := &tree.Savepoint{Name: enforceHomeRegionSavepointName}
2117-
var event fsm.Event
2118-
var eventPayload fsm.EventPayload
2119-
if event, eventPayload, err = ex.execSavepointInOpenState(ctx, s, res); err != nil {
2120-
return event, eventPayload, err
2121-
}
2122-
2123-
releaseHomeRegionSavepoint = &tree.ReleaseSavepoint{Savepoint: enforceHomeRegionSavepointName}
2124-
rollbackHomeRegionSavepoint = &tree.RollbackToSavepoint{Savepoint: enforceHomeRegionSavepointName}
2125-
defer func() {
2126-
// The default case is to roll back the internally-generated savepoint
2127-
// after every request. We only need it if a retryable "query has no home
2128-
// region" error occurs.
2129-
ex.execRelease(ctx, releaseHomeRegionSavepoint, res)
2130-
}()
2131-
}
2132-
21332022
if ex.state.mu.autoRetryReason != nil {
21342023
p.autoRetryCounter = int(ex.state.mu.autoRetryCounter)
21352024
ex.sessionTracing.TraceRetryInformation(
@@ -2181,54 +2070,6 @@ func (ex *connExecutor) execStmtInOpenStateWithPausablePortal(
21812070
}
21822071

21832072
if err = res.Err(); err != nil {
2184-
setErrorAndRestoreLocality := func(err error) {
2185-
res.SetError(err)
2186-
// We won't be faking the gateway region any more. Restore the original
2187-
// locality.
2188-
p.EvalContext().Locality = p.EvalContext().OriginalLocality
2189-
}
2190-
if execinfra.IsDynamicQueryHasNoHomeRegionError(err) {
2191-
if rollbackHomeRegionSavepoint != nil {
2192-
// A retryable "query has no home region" error has occurred.
2193-
// Roll back to the internal savepoint in preparation for the next
2194-
// planning and execution of this query with a different gateway region
2195-
// (as considered by the optimizer).
2196-
p.StmtNoConstantsWithHomeRegionEnforced = p.stmt.StmtNoConstants
2197-
event, eventPayload := ex.execRollbackToSavepointInOpenState(
2198-
ctx, rollbackHomeRegionSavepoint, res,
2199-
)
2200-
_, isTxnRestart := event.(eventTxnRestart)
2201-
rollbackToSavepointFailed := !isTxnRestart || eventPayload != nil
2202-
if ex.implicitTxn() && rollbackToSavepointFailed {
2203-
err = errors.AssertionFailedf(
2204-
"unable to roll back to internal savepoint for enforce_home_region",
2205-
)
2206-
setErrorAndRestoreLocality(err)
2207-
} else if rollbackToSavepointFailed || int(ex.state.mu.autoRetryCounter) == len(ex.planner.EvalContext().RemoteRegions) {
2208-
// If rollback to savepoint in the transaction failed (perhaps because
2209-
// the txn was aborted) and we're in an explicit transaction, or we
2210-
// have retried the statement using each remote region as a fake
2211-
// gateway region, then give up and return the generic "query has no
2212-
// home region" error message.
2213-
err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(err)
2214-
setErrorAndRestoreLocality(err)
2215-
}
2216-
} else {
2217-
err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(err)
2218-
setErrorAndRestoreLocality(err)
2219-
}
2220-
} else if execinfra.IsDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason) {
2221-
// If we are retrying a dynamic "query has no home region" error and
2222-
// we get a different error message when executing with locality-optimized
2223-
// ops using a different local region (for example, relation does not
2224-
// exist, due to the AOST read), return the original error message in
2225-
// non-retryable form.
2226-
errorMessage := err.Error()
2227-
if !strings.HasPrefix(errorMessage, execinfra.QueryNotRunningInHomeRegionMessagePrefix) {
2228-
err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason)
2229-
setErrorAndRestoreLocality(err)
2230-
}
2231-
}
22322073
return makeErrEvent(err)
22332074
}
22342075

@@ -2288,23 +2129,6 @@ func (ex *connExecutor) handleAOST(ctx context.Context, stmt tree.Statement) err
22882129
return errors.AssertionFailedf(
22892130
"cannot handle AOST clause without a transaction",
22902131
)
2291-
} else if execinfra.IsDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason) {
2292-
asOfClause := tree.AsOfClause{Expr: followerReadTimestampExpr}
2293-
// Set the timestamp used by current_timestamp().
2294-
asOf, err := p.EvalAsOfTimestamp(ctx, asOfClause, asof.OptionAllowBoundedStaleness)
2295-
if err != nil {
2296-
return errors.AssertionFailedf(
2297-
"problem evaluating follower read timestamp for enforce_home_region dynamic error checking",
2298-
)
2299-
}
2300-
// Set up AOST in the txn so re-running of the query with different possible
2301-
// home regions does not have to read rows from remote regions.
2302-
p.extendedEvalCtx.SetTxnTimestamp(asOf.Timestamp.GoTime())
2303-
if err := ex.state.setHistoricalTimestamp(ctx, asOf.Timestamp); err != nil {
2304-
// If the table was just created, we may not be able to set a historical
2305-
// timestamp.
2306-
return execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(ex.state.mu.autoRetryReason)
2307-
}
23082132
}
23092133
asOf, err := p.isAsOf(ctx, stmt)
23102134
if err != nil {
@@ -3141,36 +2965,6 @@ func (ex *connExecutor) dispatchToExecutionEngine(
31412965
if limitsErr := ex.handleTxnRowsWrittenReadLimits(ctx); limitsErr != nil && res.Err() == nil {
31422966
res.SetError(limitsErr)
31432967
}
3144-
if res.Err() == nil && err == nil {
3145-
autoRetryReason := ex.state.mu.autoRetryReason
3146-
if execinfra.IsDynamicQueryHasNoHomeRegionError(autoRetryReason) {
3147-
if homeRegion, ok := planner.EvalContext().Locality.Find("region"); ok &&
3148-
planner.StmtNoConstantsWithHomeRegionEnforced == planner.stmt.StmtNoConstants {
3149-
// If this is the same query as ran when the dynamic "query has no home
3150-
// region" error occurred, but this time it didn't error out, report
3151-
// back the query's home region.
3152-
err = pgerror.Newf(pgcode.QueryNotRunningInHomeRegion,
3153-
`%s. Try running the query from region '%s'. %s`,
3154-
execinfra.QueryNotRunningInHomeRegionMessagePrefix,
3155-
homeRegion,
3156-
sqlerrors.EnforceHomeRegionFurtherInfo,
3157-
)
3158-
res.SetError(err)
3159-
// We won't be faking the gateway region any more. Restore the original
3160-
// locality.
3161-
planner.EvalContext().Locality = planner.EvalContext().OriginalLocality
3162-
return nil
3163-
}
3164-
// If for some reason we're not running the same query as before, report
3165-
// the original "query has no home region" error in non-retryable form.
3166-
err = execinfra.MaybeGetNonRetryableDynamicQueryHasNoHomeRegionError(autoRetryReason)
3167-
res.SetError(err)
3168-
// We won't be faking the gateway region any more. Restore the original
3169-
// locality.
3170-
planner.EvalContext().Locality = planner.EvalContext().OriginalLocality
3171-
return nil
3172-
}
3173-
}
31742968

31752969
return err
31762970
}

pkg/sql/distsql/server.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,6 @@ func (ds *ServerImpl) setupFlow(
354354
ReCache: ds.regexpCache,
355355
ToCharFormatCache: ds.toCharFormatCache,
356356
Locality: ds.ServerConfig.Locality,
357-
OriginalLocality: ds.ServerConfig.Locality,
358357
Tracer: ds.ServerConfig.Tracer,
359358
Planner: &faketreeeval.DummyEvalPlanner{Monitor: monitor},
360359
StreamManagerFactory: &faketreeeval.DummyStreamManagerFactory{},

pkg/sql/exec_util.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4108,10 +4108,6 @@ func (m *sessionDataMutator) SetInjectRetryErrorsOnCommitEnabled(val bool) {
41084108
m.data.InjectRetryErrorsOnCommitEnabled = val
41094109
}
41104110

4111-
func (m *sessionDataMutator) SetEnforceHomeRegionFollowerReadsEnabled(val bool) {
4112-
m.data.EnforceHomeRegionFollowerReadsEnabled = val
4113-
}
4114-
41154111
func (m *sessionDataMutator) SetOptimizerAlwaysUseHistograms(val bool) {
41164112
m.data.OptimizerAlwaysUseHistograms = val
41174113
}

pkg/sql/execinfra/BUILD.bazel

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ go_library(
66
name = "execinfra",
77
srcs = [
88
"base.go",
9-
"errors.go",
109
"flow_context.go",
1110
"metrics.go",
1211
"outboxbase.go",
@@ -52,7 +51,6 @@ go_library(
5251
"//pkg/sql/evalcatalog",
5352
"//pkg/sql/execinfra/execexpr",
5453
"//pkg/sql/execinfrapb",
55-
"//pkg/sql/pgwire/pgerror",
5654
"//pkg/sql/rowenc",
5755
"//pkg/sql/rowenc/valueside",
5856
"//pkg/sql/rowinfra",

0 commit comments

Comments
 (0)