From a3f34a9db20a455e0db47a5936ce10dd86d6f479 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 26 Jul 2021 17:57:47 +1200 Subject: [PATCH v4 1/2] Fix assert failures in parallel SERIALIZABLE READ ONLY. 1. Make sure that we don't decrement SxactGlobalXminCount twice when the SXACT_FLAG_RO_SAFE optimization is reached in a parallel query. This could trigger a sanity check failure in assert builds. Release builds recompute the count in SetNewSxactGlobalXmin(), hiding the problem. Add a new isolation test for that case. 2. Remove an assertion that the DOOMED flag can't be set on a partially released SERIALIZABLEXACT. Instead, ignore the flag (our transaction was already determined to be read-only safe, and DOOMED is only set incidentally during partial release). Improve an existing isolation test so that it reaches that case. Back-patch to 12. Bug #17116. Defects in commit 47a338cf. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/17116-d6ca217acc180e30%40postgresql.org --- src/backend/storage/lmgr/predicate.c | 20 +++- .../expected/serializable-parallel-2.out | 57 +++-------- .../expected/serializable-parallel-3.out | 97 +++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/serializable-parallel-2.spec | 12 ++- .../specs/serializable-parallel-3.spec | 47 +++++++++ 6 files changed, 184 insertions(+), 50 deletions(-) create mode 100644 src/test/isolation/expected/serializable-parallel-3.out create mode 100644 src/test/isolation/specs/serializable-parallel-3.spec diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 56267bdc3c..41688d8078 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -3331,6 +3331,7 @@ SetNewSxactGlobalXmin(void) void ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe) { + bool partiallyReleasing = false; bool needToClear; RWConflict conflict, nextConflict, @@ -3431,6 +3432,7 @@ ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe) else { MySerializableXact->flags |= SXACT_FLAG_PARTIALLY_RELEASED; + partiallyReleasing = true; /* ... and proceed to perform the partial release below. */ } } @@ -3681,9 +3683,15 @@ ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe) * serializable transactions completes. We then find the "new oldest" * xmin and purge any transactions which finished before this transaction * was launched. + * + * For parallel queries in read-only transactions, it might run twice. + * We only release the reference on the first call. */ needToClear = false; - if (TransactionIdEquals(MySerializableXact->xmin, PredXact->SxactGlobalXmin)) + if ((partiallyReleasing || + !SxactIsPartiallyReleased(MySerializableXact)) && + TransactionIdEquals(MySerializableXact->xmin, + PredXact->SxactGlobalXmin)) { Assert(PredXact->SxactGlobalXminCount > 0); if (--(PredXact->SxactGlobalXminCount) == 0) @@ -4839,10 +4847,14 @@ PreCommit_CheckForSerializationFailure(void) LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE); - /* Check if someone else has already decided that we need to die */ - if (SxactIsDoomed(MySerializableXact)) + /* + * Check if someone else has already decided that we need to die. Since + * we set our own DOOMED flag when partially releasing, ignore in that + * case. + */ + if (SxactIsDoomed(MySerializableXact) && + !SxactIsPartiallyReleased(MySerializableXact)) { - Assert(!SxactIsPartiallyReleased(MySerializableXact)); LWLockRelease(SerializableXactHashLock); ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), diff --git a/src/test/isolation/expected/serializable-parallel-2.out b/src/test/isolation/expected/serializable-parallel-2.out index 92753ccf39..904fdd9080 100644 --- a/src/test/isolation/expected/serializable-parallel-2.out +++ b/src/test/isolation/expected/serializable-parallel-2.out @@ -1,50 +1,23 @@ Parsed test spec with 2 sessions starting permutation: s1r s2r1 s1c s2r2 s2c -step s1r: SELECT * FROM foo; - a --- - 1 - 2 - 3 - 4 - 5 - 6 - 7 - 8 - 9 -10 -(10 rows) +step s1r: SELECT COUNT(*) FROM foo; +count +----- + 100 +(1 row) -step s2r1: SELECT * FROM foo; - a --- - 1 - 2 - 3 - 4 - 5 - 6 - 7 - 8 - 9 -10 -(10 rows) +step s2r1: SELECT COUNT(*) FROM foo; +count +----- + 100 +(1 row) step s1c: COMMIT; -step s2r2: SELECT * FROM foo; - a --- - 1 - 2 - 3 - 4 - 5 - 6 - 7 - 8 - 9 -10 -(10 rows) +step s2r2: SELECT COUNT(*) FROM foo; +count +----- + 100 +(1 row) step s2c: COMMIT; diff --git a/src/test/isolation/expected/serializable-parallel-3.out b/src/test/isolation/expected/serializable-parallel-3.out new file mode 100644 index 0000000000..654276a385 --- /dev/null +++ b/src/test/isolation/expected/serializable-parallel-3.out @@ -0,0 +1,97 @@ +Parsed test spec with 4 sessions + +starting permutation: s1r s3r s2r1 s4r1 s1c s2r2 s3c s4r2 s4c s2c +step s1r: SELECT * FROM foo; + a +-- + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 +10 +(10 rows) + +step s3r: SELECT * FROM foo; + a +-- + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 +10 +(10 rows) + +step s2r1: SELECT * FROM foo; + a +-- + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 +10 +(10 rows) + +step s4r1: SELECT * FROM foo; + a +-- + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 +10 +(10 rows) + +step s1c: COMMIT; +step s2r2: SELECT * FROM foo; + a +-- + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 +10 +(10 rows) + +step s3c: COMMIT; +step s4r2: SELECT * FROM foo; + a +-- + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 +10 +(10 rows) + +step s4c: COMMIT; +step s2c: COMMIT; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index f4c01006fc..67ef93b440 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -96,3 +96,4 @@ test: plpgsql-toast test: truncate-conflict test: serializable-parallel test: serializable-parallel-2 +test: serializable-parallel-3 diff --git a/src/test/isolation/specs/serializable-parallel-2.spec b/src/test/isolation/specs/serializable-parallel-2.spec index f3941f7863..c975d96d77 100644 --- a/src/test/isolation/specs/serializable-parallel-2.spec +++ b/src/test/isolation/specs/serializable-parallel-2.spec @@ -3,7 +3,8 @@ setup { - CREATE TABLE foo AS SELECT generate_series(1, 10)::int a; + CREATE TABLE foo AS SELECT generate_series(1, 100)::int a; + CREATE INDEX ON foo(a); ALTER TABLE foo SET (parallel_workers = 2); } @@ -14,7 +15,7 @@ teardown session s1 setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; } -step s1r { SELECT * FROM foo; } +step s1r { SELECT COUNT(*) FROM foo; } step s1c { COMMIT; } session s2 @@ -22,9 +23,12 @@ setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY; SET parallel_setup_cost = 0; SET parallel_tuple_cost = 0; + SET min_parallel_index_scan_size = 0; + SET parallel_leader_participation = off; + SET enable_seqscan = off; } -step s2r1 { SELECT * FROM foo; } -step s2r2 { SELECT * FROM foo; } +step s2r1 { SELECT COUNT(*) FROM foo; } +step s2r2 { SELECT COUNT(*) FROM foo; } step s2c { COMMIT; } permutation s1r s2r1 s1c s2r2 s2c diff --git a/src/test/isolation/specs/serializable-parallel-3.spec b/src/test/isolation/specs/serializable-parallel-3.spec new file mode 100644 index 0000000000..c27298c24f --- /dev/null +++ b/src/test/isolation/specs/serializable-parallel-3.spec @@ -0,0 +1,47 @@ +# Exercise the case where a read-only serializable transaction has +# SXACT_FLAG_RO_SAFE set in a parallel query. This variant is like +# two copies of #2 running at the same time, and excercises the case +# where another transaction has the same xmin, and it is the oldest. + +setup +{ + CREATE TABLE foo AS SELECT generate_series(1, 10)::int a; + ALTER TABLE foo SET (parallel_workers = 2); +} + +teardown +{ + DROP TABLE foo; +} + +session s1 +setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; } +step s1r { SELECT * FROM foo; } +step s1c { COMMIT; } + +session s2 +setup { + BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY; + SET parallel_setup_cost = 0; + SET parallel_tuple_cost = 0; + } +step s2r1 { SELECT * FROM foo; } +step s2r2 { SELECT * FROM foo; } +step s2c { COMMIT; } + +session s3 +setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; } +step s3r { SELECT * FROM foo; } +step s3c { COMMIT; } + +session s4 +setup { + BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY; + SET parallel_setup_cost = 0; + SET parallel_tuple_cost = 0; + } +step s4r1 { SELECT * FROM foo; } +step s4r2 { SELECT * FROM foo; } +step s4c { COMMIT; } + +permutation s1r s3r s2r1 s4r1 s1c s2r2 s3c s4r2 s4c s2c -- 2.30.2