Skip to content

kvclient: metamorphically enable write buffering #143860

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/cluster_locks_tenant
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# LogicTest: 3node-tenant

statement ok
SET kv_transaction_buffered_writes_enabled = false;

# Create a table, write a row, lock it, then switch users.
statement ok
CREATE TABLE t (k STRING PRIMARY KEY, v STRING, FAMILY (k,v))
Expand Down Expand Up @@ -35,6 +38,9 @@ SHOW session_id

user testuser

statement ok
SET kv_transaction_buffered_writes_enabled = false;

let $testuser_session
SHOW session_id

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,273 @@
# LogicTest: 3node-tenant

statement ok
SET kv_transaction_buffered_writes_enabled = true;

# Create a table, write a row, lock it, then switch users.
statement ok
CREATE TABLE t (k STRING PRIMARY KEY, v STRING, FAMILY (k,v))

statement ok
GRANT ALL ON t TO testuser

statement ok
INSERT INTO t VALUES ('a', 'val1'), ('b', 'val2'), ('c', 'val3'), ('l', 'val4'), ('m', 'val5'), ('p', 'val6'), ('s', 'val7'), ('t', 'val8'), ('z', 'val9')

# Also create an additional user with VIEWACTIVITYREDACTED, with only permissions on t
statement ok
CREATE USER testuser2 WITH VIEWACTIVITYREDACTED

statement ok
GRANT ALL ON t TO testuser2

statement ok
CREATE TABLE t2 (k STRING PRIMARY KEY, v STRING, FAMILY (k,v))

statement ok
INSERT INTO t2 VALUES ('a', 'val1'), ('b', 'val2')

# Start txn1 where we acquire replicated locks
statement ok
BEGIN PRIORITY HIGH

statement ok
UPDATE t SET v = '_updated' WHERE k >= 'b' AND k < 'x'

let $root_session
SHOW session_id

user testuser

statement ok
SET kv_transaction_buffered_writes_enabled = true;

let $testuser_session
SHOW session_id

statement ok
BEGIN

# switch back to root, collect data needed for validation
user root

let $txn1
SELECT txns.id FROM crdb_internal.cluster_transactions txns WHERE txns.session_id = '$root_session'

let $txn2
SELECT txns.id FROM crdb_internal.cluster_transactions txns WHERE txns.session_id = '$testuser_session'

user testuser

query TT async,rowsort readReq
SELECT * FROM t
----
a val1
b _updated
c _updated
l _updated
m _updated
p _updated
s _updated
t _updated
z val9

user root

query TTT colnames,retry
SELECT user_name, query, phase FROM crdb_internal.cluster_queries WHERE txn_id='$txn2'
----
user_name query phase
testuser SELECT * FROM t executing

# looking at each transaction separately, validate the expected results in the lock table
query TTTTTTTBB colnames,rowsort,retry
SELECT database_name, schema_name, table_name, lock_key_pretty, lock_strength, durability, isolation_level, granted, contended
FROM crdb_internal.cluster_locks WHERE table_name='t' AND txn_id='$txn1'
----
database_name schema_name table_name lock_key_pretty lock_strength durability isolation_level granted contended
test public t /Table/106/1/"b"/0 Exclusive Unreplicated SERIALIZABLE true true
test public t /Table/106/1/"c"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"l"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"m"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"p"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"s"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"t"/0 Exclusive Unreplicated SERIALIZABLE true false

query TTTTTTTBB colnames
SELECT database_name, schema_name, table_name, lock_key_pretty, lock_strength, durability, isolation_level, granted, contended
FROM crdb_internal.cluster_locks WHERE table_name='t' AND txn_id='$txn2'
----
database_name schema_name table_name lock_key_pretty lock_strength durability isolation_level granted contended
test public t /Table/106/1/"b"/0 None Unreplicated SERIALIZABLE false true

# check that we can't see keys, potentially revealing PII, with VIEWACTIVITYREDACTED
user testuser2

query TTTTTTTBB colnames,rowsort
SELECT database_name, schema_name, table_name, lock_key_pretty, lock_strength, durability, isolation_level, granted, contended
FROM crdb_internal.cluster_locks WHERE table_name='t' AND txn_id='$txn1'
----
database_name schema_name table_name lock_key_pretty lock_strength durability isolation_level granted contended
test public t · Exclusive Unreplicated SERIALIZABLE true true
test public t · Exclusive Unreplicated SERIALIZABLE true false
test public t · Exclusive Unreplicated SERIALIZABLE true false
test public t · Exclusive Unreplicated SERIALIZABLE true false
test public t · Exclusive Unreplicated SERIALIZABLE true false
test public t · Exclusive Unreplicated SERIALIZABLE true false
test public t · Exclusive Unreplicated SERIALIZABLE true false

user root

query I
SELECT count(*) FROM crdb_internal.cluster_locks WHERE table_name = 't'
----
8

statement ok
COMMIT

query I retry
SELECT count(*) FROM crdb_internal.cluster_locks WHERE table_name = 't'
----
0

user testuser

awaitquery readReq

statement ok
COMMIT

user root

# start txn3
statement ok
BEGIN

user testuser

# start txn4
statement ok
BEGIN

user root

query TT rowsort
SELECT * FROM t FOR UPDATE
----
a val1
b _updated
c _updated
l _updated
m _updated
p _updated
s _updated
t _updated
z val9

let $txn3
SELECT txns.id FROM crdb_internal.cluster_transactions txns WHERE txns.session_id = '$root_session'

let $txn4
SELECT txns.id FROM crdb_internal.cluster_transactions txns WHERE txns.session_id = '$testuser_session'

user testuser

statement async deleteReq count 7
DELETE FROM t WHERE k >= 'b' AND k < 'x'

user root

query TTT colnames,retry
SELECT user_name, query, phase FROM crdb_internal.cluster_queries WHERE txn_id='$txn4'
----
user_name query phase
testuser DELETE FROM t WHERE (k >= 'b') AND (k < 'x') executing

# looking at each transaction separately, validate the expected results in the lock table
query TTTTTTTBB colnames,rowsort,retry
SELECT database_name, schema_name, table_name, lock_key_pretty, lock_strength, durability, isolation_level, granted, contended FROM crdb_internal.cluster_locks WHERE table_name='t' AND txn_id='$txn3'
----
database_name schema_name table_name lock_key_pretty lock_strength durability isolation_level granted contended
test public t /Table/106/1/"a"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"b"/0 Exclusive Unreplicated SERIALIZABLE true true
test public t /Table/106/1/"c"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"l"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"m"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"p"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"s"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"t"/0 Exclusive Unreplicated SERIALIZABLE true false
test public t /Table/106/1/"z"/0 Exclusive Unreplicated SERIALIZABLE true false

query TTTTTTTBB colnames,rowsort
SELECT database_name, schema_name, table_name, lock_key_pretty, lock_strength, durability, isolation_level, granted, contended FROM crdb_internal.cluster_locks WHERE table_name='t' AND txn_id='$txn4'
----
database_name schema_name table_name lock_key_pretty lock_strength durability isolation_level granted contended
test public t /Table/106/1/"b"/0 Exclusive Unreplicated SERIALIZABLE false true

query I
SELECT count(*) FROM crdb_internal.cluster_locks WHERE table_name = 't'
----
10

statement ok
ROLLBACK

user testuser

awaitstatement deleteReq

statement ok
COMMIT

user root

query I retry
SELECT count(*) FROM crdb_internal.cluster_locks WHERE table_name = 't'
----
0

# validate that only locks on keys in privileged tables can be seen
statement ok
BEGIN

query TT rowsort
SELECT * FROM t FOR UPDATE
----
a val1
z val9

query TT rowsort
SELECT * FROM t2 FOR UPDATE
----
a val1
b val2

query I retry
SELECT count(*) FROM crdb_internal.cluster_locks WHERE table_name IN ('t','t2')
----
4

user testuser

query error pq: user testuser does not have VIEWACTIVITY or VIEWACTIVITYREDACTED privilege
SELECT database_name, schema_name, table_name, lock_key_pretty, lock_strength, durability, isolation_level, granted, contended FROM crdb_internal.cluster_locks

user testuser2

query TTTTTTTBB colnames
SELECT database_name, schema_name, table_name, lock_key_pretty, lock_strength, durability, isolation_level, granted, contended FROM crdb_internal.cluster_locks WHERE table_name IN ('t', 't2')
----
database_name schema_name table_name lock_key_pretty lock_strength durability isolation_level granted contended
test public t · Exclusive Unreplicated SERIALIZABLE true false
test public t · Exclusive Unreplicated SERIALIZABLE true false

user root

statement ok
ROLLBACK

query I retry
SELECT count(*) FROM crdb_internal.cluster_locks WHERE table_name IN ('t','t2')
----
0
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/ccl/multitenantccl/tenantcapabilitiesccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ go_test(
"//pkg/testutils/testcluster",
"//pkg/util/hlc",
"//pkg/util/leaktest",
"//pkg/util/log",
"//pkg/util/randutil",
"//pkg/util/syncutil",
"@com_github_cockroachdb_datadriven//:datadriven",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/datadriven"
"github.com/cockroachdb/errors"
Expand Down Expand Up @@ -53,6 +54,7 @@ import (
// update.
func TestDataDriven(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
datadriven.Walk(t, datapathutils.TestDataPath(t), func(t *testing.T, path string) {
ctx := context.Background()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ SELECT * FROM [SHOW TENANT [10] WITH CAPABILITIES] WHERE capability_name = 'can_
----
10 cluster-10 ready external can_prepare_txns false

# TODO(#144252): remove this.
exec-sql-tenant
SET kv_transaction_buffered_writes_enabled = false
----
ok

exec-sql-tenant
CREATE TABLE t(a INT PRIMARY KEY)
----
Expand Down
3 changes: 2 additions & 1 deletion pkg/kv/kvclient/kvcoord/txn_interceptor_write_buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/buildutil"
"github.com/cockroachdb/cockroach/pkg/util/humanizeutil"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/metamorphic"
"github.com/cockroachdb/errors"
)

Expand All @@ -31,7 +32,7 @@ var BufferedWritesEnabled = settings.RegisterBoolSetting(
settings.ApplicationLevel,
"kv.transaction.write_buffering.enabled",
"if enabled, transactional writes are buffered on the client",
false,
metamorphic.ConstantWithTestBool("kv.transactional.write_buffering.enabled", false /* defaultValue */),
settings.WithPublic,
)

Expand Down
Loading
Loading