From 329f99ce12a9dfb7d86c5320825f7e07c07905f7 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 26 Jul 2021 17:57:47 +1200 Subject: [PATCH] Fix reference count bug in parallel serializable. Make sure that we don't decrement SxactGlobalXminCount twice when the SXACT_FLAG_RO_SAFE optimization is reached in a parallel query using SERIALIZABLE READ ONLY. This could trigger a sanity check failure in assert builds. Release builds recompute the count in SetNewSxactGlobalXmin(), hiding the problem. Back-patch to 12. Fixes bug #17116. Defect in commit 47a338cf. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/17116-d6ca217acc180e30%40postgresql.org --- src/backend/storage/lmgr/predicate.c | 10 +- .../expected/serializable-parallel-3.out | 97 +++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/serializable-parallel-3.spec | 47 +++++++++ 4 files changed, 154 insertions(+), 1 deletion(-) 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..3bc9e4c64e 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) 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-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