Skip to content

Commit

Permalink
sql: don't consider repeatable read to snapshot an upgrade
Browse files Browse the repository at this point in the history
One isolation level is not stronger than the other. We consider
them aliases of each other.

Epic: None
Release note: None
  • Loading branch information
nvanbenschoten committed Aug 28, 2024
1 parent f53a4ca commit 1629944
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 34 deletions.
21 changes: 8 additions & 13 deletions pkg/sql/conn_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -3529,9 +3529,7 @@ var allowReadCommittedIsolation = settings.RegisterBoolSetting(
settings.WithPublic,
)

// TODO(nvanbenschoten): rename this variable and update uses of it to favor
// "repeatable read" over "snapshot".
var allowSnapshotIsolation = settings.RegisterBoolSetting(
var allowRepeatableReadIsolation = settings.RegisterBoolSetting(
settings.ApplicationLevel,
"sql.txn.snapshot_isolation.enabled",
"set to true to allow transactions to use the REPEATABLE READ isolation "+
Expand Down Expand Up @@ -3589,20 +3587,17 @@ func (ex *connExecutor) txnIsolationLevelToKV(
upgradedDueToLicense = true
}
}
case tree.RepeatableReadIsolation:
// REPEATABLE READ is mapped to SNAPSHOT.
upgraded = true
fallthrough
case tree.SnapshotIsolation:
// SNAPSHOT is only allowed if the cluster setting is enabled and the
// cluster has a license. Otherwise it is mapped to SERIALIZABLE.
allowSnapshot := allowSnapshotIsolation.Get(&ex.server.cfg.Settings.SV)
if allowSnapshot && hasLicense {
case tree.RepeatableReadIsolation, tree.SnapshotIsolation:
// REPEATABLE READ and SNAPSHOT are considered aliases. The isolation levels
// are only allowed if the cluster setting is enabled and the cluster has a
// license. Otherwise, they are mapped to SERIALIZABLE.
allowRepeatableRead := allowRepeatableReadIsolation.Get(&ex.server.cfg.Settings.SV)
if allowRepeatableRead && hasLicense {
ret = isolation.Snapshot
} else {
upgraded = true
ret = isolation.Serializable
if allowSnapshot && !hasLicense {
if allowRepeatableRead && !hasLicense {
upgradedDueToLicense = true
}
}
Expand Down
11 changes: 4 additions & 7 deletions pkg/sql/set_session_characteristics.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func (p *planner) SetSessionCharacteristics(
upgradedLevel := false
upgradedDueToLicense := false
allowReadCommitted := allowReadCommittedIsolation.Get(&p.execCfg.Settings.SV)
allowSnapshot := allowSnapshotIsolation.Get(&p.execCfg.Settings.SV)
allowRepeatableRead := allowRepeatableReadIsolation.Get(&p.execCfg.Settings.SV)
hasLicense := base.CCLDistributionAndEnterpriseEnabled(p.ExecCfg().Settings)
if err := p.sessionDataMutatorIterator.applyOnEachMutatorError(func(m sessionDataMutator) error {
// Note: We also support SET DEFAULT_TRANSACTION_ISOLATION TO ' .... '.
Expand All @@ -49,16 +49,13 @@ func (p *planner) SetSessionCharacteristics(
}
}
m.SetDefaultTransactionIsolationLevel(level)
case tree.RepeatableReadIsolation:
upgradedLevel = true
fallthrough
case tree.SnapshotIsolation:
case tree.RepeatableReadIsolation, tree.SnapshotIsolation:
level := tree.SerializableIsolation
if allowSnapshot && hasLicense {
if allowRepeatableRead && hasLicense {
level = tree.SnapshotIsolation
} else {
upgradedLevel = true
if allowSnapshot && !hasLicense {
if allowRepeatableRead && !hasLicense {
upgradedDueToLicense = true
}
}
Expand Down
4 changes: 0 additions & 4 deletions pkg/sql/testdata/telemetry/isolation_level
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ feature-usage
SET default_transaction_isolation = 'repeatable read'
----
sql.txn.isolation.executed_at.read_committed
sql.txn.isolation.upgraded_from.repeatable_read

feature-usage
SET default_transaction_isolation = 'snapshot'
Expand All @@ -134,7 +133,6 @@ feature-usage
SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL REPEATABLE READ
----
sql.txn.isolation.executed_at.read_committed
sql.txn.isolation.upgraded_from.repeatable_read

feature-usage
SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL SNAPSHOT
Expand All @@ -156,7 +154,6 @@ feature-usage
BEGIN TRANSACTION ISOLATION LEVEL REPEATABLE READ; COMMIT
----
sql.txn.isolation.executed_at.snapshot
sql.txn.isolation.upgraded_from.repeatable_read

feature-usage
BEGIN TRANSACTION ISOLATION LEVEL SNAPSHOT; COMMIT
Expand All @@ -180,7 +177,6 @@ feature-usage
BEGIN; SET TRANSACTION ISOLATION LEVEL REPEATABLE READ; COMMIT
----
sql.txn.isolation.executed_at.snapshot
sql.txn.isolation.upgraded_from.repeatable_read

feature-usage
BEGIN; SET TRANSACTION ISOLATION LEVEL SNAPSHOT; COMMIT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,4 +228,3 @@ BEGIN; SELECT 1; SELECT 2; SELECT 3; SELECT 4; COMMIT;
"TransactionFingerprintID": "2831371359051261045",
"User": "root"
}

15 changes: 6 additions & 9 deletions pkg/sql/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,10 +418,10 @@ var varGen = map[string]sessionVar{
`default_transaction_isolation`: {
Set: func(ctx context.Context, m sessionDataMutator, s string) error {
allowReadCommitted := allowReadCommittedIsolation.Get(&m.settings.SV)
allowSnapshot := allowSnapshotIsolation.Get(&m.settings.SV)
allowRepeatableRead := allowRepeatableReadIsolation.Get(&m.settings.SV)
hasLicense := base.CCLDistributionAndEnterpriseEnabled(m.settings)
var allowedValues = []string{"serializable"}
if allowSnapshot {
if allowRepeatableRead {
// TODO(nvanbenschoten): switch to "repeatable read".
allowedValues = append(allowedValues, "snapshot")
}
Expand Down Expand Up @@ -449,16 +449,13 @@ var varGen = map[string]sessionVar{
upgradedDueToLicense = true
}
}
case tree.RepeatableReadIsolation:
upgraded = true
fallthrough
case tree.SnapshotIsolation:
case tree.RepeatableReadIsolation, tree.SnapshotIsolation:
level = tree.SerializableIsolation
if allowSnapshot && hasLicense {
if allowRepeatableRead && hasLicense {
level = tree.SnapshotIsolation
} else {
upgraded = true
if allowSnapshot && !hasLicense {
if allowRepeatableRead && !hasLicense {
upgradedDueToLicense = true
}
}
Expand Down Expand Up @@ -1615,7 +1612,7 @@ var varGen = map[string]sessionVar{
level, ok := tree.IsolationLevelMap[strings.ToLower(s)]
if !ok {
var allowedValues = []string{"serializable"}
if allowSnapshotIsolation.Get(&evalCtx.ExecCfg.Settings.SV) {
if allowRepeatableReadIsolation.Get(&evalCtx.ExecCfg.Settings.SV) {
// TODO(nvanbenschoten): switch to "repeatable read".
allowedValues = append(allowedValues, "snapshot")
}
Expand Down

0 comments on commit 1629944

Please sign in to comment.