Skip to content

Commit 37e29dc

Browse files
committed
sql: version-gate sql.txn.write_buffering_for_weak_isolation.enabled
Buffering writes in weak isolation levels such as read committed requires lock loss detection to avoid intra-statement anomalies. Lock loss detection is only available when all nodes are on 25.3 or greater. Epic: none Release note: None
1 parent add581c commit 37e29dc

File tree

5 files changed

+27
-15
lines changed

5 files changed

+27
-15
lines changed

pkg/ccl/logictestccl/testdata/logic_test/buffered_writes_lock_loss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# LogicTest: !3node-tenant
1+
# LogicTest: !3node-tenant !local-mixed-25.2
22
# cluster-opt: disable-mvcc-range-tombstones-for-point-deletes
33

44
statement ok

pkg/ccl/logictestccl/tests/local-mixed-25.2/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ go_test(
99
"//pkg/ccl/logictestccl:testdata", # keep
1010
],
1111
exec_properties = {"test.Pool": "large"},
12-
shard_count = 34,
12+
shard_count = 33,
1313
tags = ["cpu:1"],
1414
deps = [
1515
"//pkg/base",

pkg/ccl/logictestccl/tests/local-mixed-25.2/generated_test.go

Lines changed: 0 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/sql/conn_executor.go

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
3131
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
3232
"github.com/cockroachdb/cockroach/pkg/settings"
33+
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
3334
"github.com/cockroachdb/cockroach/pkg/sql/appstatspb"
3435
"github.com/cockroachdb/cockroach/pkg/sql/auditlogging"
3536
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
@@ -3543,9 +3544,7 @@ func (ex *connExecutor) setTransactionModes(
35433544
if err := ex.state.setIsolationLevel(level); err != nil {
35443545
return pgerror.WithCandidateCode(err, pgcode.ActiveSQLTransaction)
35453546
}
3546-
if (level != isolation.Serializable) && !allowBufferedWritesForWeakIsolation.Get(&ex.server.cfg.Settings.SV) {
3547-
// TODO(#143497): we currently only support buffered writes under
3548-
// serializable isolation.
3547+
if !ex.bufferedWritesIsAllowedForIsolationLevel(ctx, level) {
35493548
ex.state.mu.txn.SetBufferedWritesEnabled(false)
35503549
}
35513550
}
@@ -3731,6 +3730,28 @@ func (ex *connExecutor) bufferedWritesEnabled(ctx context.Context) bool {
37313730
return ex.sessionData().BufferedWritesEnabled && ex.server.cfg.Settings.Version.IsActive(ctx, clusterversion.V25_2)
37323731
}
37333732

3733+
func (ex *connExecutor) bufferedWritesIsAllowedForIsolationLevel(
3734+
ctx context.Context, isoLevel isolation.Level,
3735+
) bool {
3736+
return bufferedWritesIsAllowedForIsolationLevel(ctx, ex.server.cfg.Settings, isoLevel)
3737+
}
3738+
3739+
func bufferedWritesIsAllowedForIsolationLevel(
3740+
ctx context.Context, st *cluster.Settings, isoLevel isolation.Level,
3741+
) bool {
3742+
if isoLevel == isolation.Serializable {
3743+
return true
3744+
}
3745+
3746+
// We are at a weaker isolation level that requires lock loss detection which
3747+
// is only available on 25.3 or greater.
3748+
if !st.Version.IsActive(ctx, clusterversion.V25_3) {
3749+
return false
3750+
}
3751+
3752+
return allowBufferedWritesForWeakIsolation.Get(&st.SV)
3753+
}
3754+
37343755
// initEvalCtx initializes the fields of an extendedEvalContext that stay the
37353756
// same across multiple statements. resetEvalCtx must also be called before each
37363757
// statement, to reinitialize other fields.

pkg/sql/txn_state.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,7 @@ func (ts *txnState) resetForNewSQLTxn(
241241
if err := ts.setIsolationLevelLocked(isoLevel); err != nil {
242242
panic(err)
243243
}
244-
if isoLevel != isolation.Serializable && !allowBufferedWritesForWeakIsolation.Get(&tranCtx.settings.SV) {
245-
// TODO(#143497): we currently only support buffered writes
246-
// under serializable isolation.
244+
if !bufferedWritesIsAllowedForIsolationLevel(connCtx, tranCtx.settings, isoLevel) {
247245
bufferedWritesEnabled = false
248246
}
249247
if bufferedWritesEnabled {

0 commit comments

Comments
 (0)