Open nvanbenschoten opened 1 year ago
To support pg_isolation_regress
, we're going to need to support the pg_catalog.pg_isolation_test_session_is_blocked
builtin function, which is used by the test to detect lock contention between sessions.
I looked into implementing the pg_catalog.pg_isolation_test_session_is_blocked
builtin function. It would be straightforward if https://github.com/cockroachdb/cockroach/issues/98581 was addressed. With that change, we could implement this using a join across crdb_internal.cluster_locks
and crdb_internal.cluster_sessions
. Something like:
WITH blocked_txn AS (
SELECT kv_txn::UUID FROM crdb_internal.cluster_sessions WHERE kv_txn IS NOT NULL AND pg_backend_pid = $1
), blocking_txns AS (
SELECT kv_txn::UUID FROM crdb_internal.cluster_sessions WHERE kv_txn IS NOT NULL AND pg_backend_pid = ANY($2)
), blocked_keys AS (
SELECT lock_key FROM crdb_internal.cluster_locks WHERE txn_id = (SELECT kv_txn FROM blocked_txn) AND contended AND NOT granted
), blocking_keys AS (
SELECT lock_key FROM crdb_internal.cluster_locks WHERE txn_id = ANY(SELECT kv_txn FROM blocking_txns) AND contended AND granted
)
SELECT count(*) > 0 FROM blocked_keys JOIN blocking_keys USING (lock_key)
Cross-posting from Slack:
We were hoping to use pg_isolation_regress (#95771) to test Read Committed and ensure our error handling and blocking behavior is consistent with PG’s. It turns out that the test suite is more of a stress test of PG and CRDB’s Serializable implementation than it is of weaker isolation levels. And beyond isolation, it’s a great test of PG compatibility. There is still some coverage of RC and Snapshot, and we seem to do well on all of those tests, but they’re not exhaustive.
The cases that are tripping us up are:
- inconsistent error messaging compared to PG
- missing deadlock_timeout variable (#116305)
- detecting serializable conflicts lazily at commit time vs. eagerly
- releasing locks immediacy on serializable conflict vs. on abort
- table locks / advisory locks
- triggers / notify / partitions
- MERGE statement
For future reference, the changes I ran with to get this far were:
PR 1: https://github.com/cockroachdb/cockroach/pull/116673 PR 2: https://github.com/cockroachdb/cockroach/pull/116708
This allowed 14 of the 112 tests to pass with no or only minor tweaks:
➜ ./pg_isolation_regress --host=localhost --port=26257 --user=root --schedule=./isolation_schedule
(using postmaster on localhost, port 26257)
============== dropping database "isolation_regression" ==============
SET
DROP DATABASE
============== creating database "isolation_regression" ==============
CREATE DATABASE
============== running regression test queries ==============
test read-only-anomaly ... ok 360 ms
test read-only-anomaly-2 ... ok 721 ms
test read-write-unique ... ok 732 ms
test read-write-unique-2 ... ok 690 ms
test simple-write-skew ... ok 2050 ms
test fk-contention ... ok 1078 ms
test fk-deadlock2 ... ok 4695 ms
test insert-conflict-do-nothing ... ok 783 ms
test insert-conflict-do-update ... ok 765 ms
test multixact-no-deadlock ... ok 523 ms
test nowait ... ok 1909 ms
test nowait-2 ... ok 969 ms
test nowait-3 ... ok 458 ms
test skip-locked ... ok 6924 ms
======================
All 14 tests passed.
======================
pg_isolation_regress is a multi-connection variant of pg_regress used to test the behavior of concurrent transactions. It contains hundred of test cases for Postgres behavior, each of which exercises a set of ordering permutations.
In https://github.com/cockroachdb/cockroach/issues/95768, we detailed a plan to port pg_regress over to a roachtest. We should do the same for pg_isolation_regress.
Jira issue: CRDB-23727
Epic: CRDB-34183