Skip to content

Commit e9051ec

Browse files
macdicemacdice
committed
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. Non-assert builds recompute the count in SetNewSxactGlobalXmin(), so the problem was hidden, explaining the lack of field reports. Add a new isolation test to exercise 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 in fact set during partial release, and there was already an assertion that it wasn't set sooner). Improve an existing isolation test so that it reaches that case (previously it wasn't quite testing what it was supposed to be testing; see discussion). Back-patch to 12. Bug #17116. Defects in commit 47a338c. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/17116-d6ca217acc180e30%40postgresql.org
1 parent 5ad63ee commit e9051ec

File tree

6 files changed

+184
-50
lines changed

6 files changed

+184
-50
lines changed

‎src/backend/storage/lmgr/predicate.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3331,6 +3331,7 @@ SetNewSxactGlobalXmin(void)
33313331
void
33323332
ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe)
33333333
{
3334+
bool partiallyReleasing = false;
33343335
bool needToClear;
33353336
RWConflict conflict,
33363337
nextConflict,
@@ -3431,6 +3432,7 @@ ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe)
34313432
else
34323433
{
34333434
MySerializableXact->flags |= SXACT_FLAG_PARTIALLY_RELEASED;
3435+
partiallyReleasing = true;
34343436
/* ... and proceed to perform the partial release below. */
34353437
}
34363438
}
@@ -3681,9 +3683,15 @@ ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe)
36813683
* serializable transactions completes. We then find the "new oldest"
36823684
* xmin and purge any transactions which finished before this transaction
36833685
* was launched.
3686+
*
3687+
* For parallel queries in read-only transactions, it might run twice.
3688+
* We only release the reference on the first call.
36843689
*/
36853690
needToClear = false;
3686-
if (TransactionIdEquals(MySerializableXact->xmin, PredXact->SxactGlobalXmin))
3691+
if ((partiallyReleasing ||
3692+
!SxactIsPartiallyReleased(MySerializableXact)) &&
3693+
TransactionIdEquals(MySerializableXact->xmin,
3694+
PredXact->SxactGlobalXmin))
36873695
{
36883696
Assert(PredXact->SxactGlobalXminCount > 0);
36893697
if (--(PredXact->SxactGlobalXminCount) == 0)
@@ -4839,10 +4847,14 @@ PreCommit_CheckForSerializationFailure(void)
48394847

48404848
LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE);
48414849

4842-
/* Check if someone else has already decided that we need to die */
4843-
if (SxactIsDoomed(MySerializableXact))
4850+
/*
4851+
* Check if someone else has already decided that we need to die. Since
4852+
* we set our own DOOMED flag when partially releasing, ignore in that
4853+
* case.
4854+
*/
4855+
if (SxactIsDoomed(MySerializableXact) &&
4856+
!SxactIsPartiallyReleased(MySerializableXact))
48444857
{
4845-
Assert(!SxactIsPartiallyReleased(MySerializableXact));
48464858
LWLockRelease(SerializableXactHashLock);
48474859
ereport(ERROR,
48484860
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),

‎src/test/isolation/expected/serializable-parallel-2.out

Lines changed: 15 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,23 @@
11
Parsed test spec with 2 sessions
22

33
starting permutation: s1r s2r1 s1c s2r2 s2c
4-
step s1r: SELECT * FROM foo;
5-
a
6-
--
7-
1
8-
2
9-
3
10-
4
11-
5
12-
6
13-
7
14-
8
15-
9
16-
10
17-
(10 rows)
4+
step s1r: SELECT COUNT(*) FROM foo;
5+
count
6+
-----
7+
100
8+
(1 row)
189

19-
step s2r1: SELECT * FROM foo;
20-
a
21-
--
22-
1
23-
2
24-
3
25-
4
26-
5
27-
6
28-
7
29-
8
30-
9
31-
10
32-
(10 rows)
10+
step s2r1: SELECT COUNT(*) FROM foo;
11+
count
12+
-----
13+
100
14+
(1 row)
3315

3416
step s1c: COMMIT;
35-
step s2r2: SELECT * FROM foo;
36-
a
37-
--
38-
1
39-
2
40-
3
41-
4
42-
5
43-
6
44-
7
45-
8
46-
9
47-
10
48-
(10 rows)
17+
step s2r2: SELECT COUNT(*) FROM foo;
18+
count
19+
-----
20+
100
21+
(1 row)
4922

5023
step s2c: COMMIT;

‎src/test/isolation/expected/serializable-parallel-3.out

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
Parsed test spec with 4 sessions
2+
3+
starting permutation: s1r s3r s2r1 s4r1 s1c s2r2 s3c s4r2 s4c s2c
4+
step s1r: SELECT * FROM foo;
5+
a
6+
--
7+
1
8+
2
9+
3
10+
4
11+
5
12+
6
13+
7
14+
8
15+
9
16+
10
17+
(10 rows)
18+
19+
step s3r: SELECT * FROM foo;
20+
a
21+
--
22+
1
23+
2
24+
3
25+
4
26+
5
27+
6
28+
7
29+
8
30+
9
31+
10
32+
(10 rows)
33+
34+
step s2r1: SELECT * FROM foo;
35+
a
36+
--
37+
1
38+
2
39+
3
40+
4
41+
5
42+
6
43+
7
44+
8
45+
9
46+
10
47+
(10 rows)
48+
49+
step s4r1: SELECT * FROM foo;
50+
a
51+
--
52+
1
53+
2
54+
3
55+
4
56+
5
57+
6
58+
7
59+
8
60+
9
61+
10
62+
(10 rows)
63+
64+
step s1c: COMMIT;
65+
step s2r2: SELECT * FROM foo;
66+
a
67+
--
68+
1
69+
2
70+
3
71+
4
72+
5
73+
6
74+
7
75+
8
76+
9
77+
10
78+
(10 rows)
79+
80+
step s3c: COMMIT;
81+
step s4r2: SELECT * FROM foo;
82+
a
83+
--
84+
1
85+
2
86+
3
87+
4
88+
5
89+
6
90+
7
91+
8
92+
9
93+
10
94+
(10 rows)
95+
96+
step s4c: COMMIT;
97+
step s2c: COMMIT;

‎src/test/isolation/isolation_schedule

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,4 @@ test: plpgsql-toast
9999
test: truncate-conflict
100100
test: serializable-parallel
101101
test: serializable-parallel-2
102+
test: serializable-parallel-3

‎src/test/isolation/specs/serializable-parallel-2.spec

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33

44
setup
55
{
6-
CREATE TABLE foo AS SELECT generate_series(1, 10)::int a;
6+
CREATE TABLE foo AS SELECT generate_series(1, 100)::int a;
7+
CREATE INDEX ON foo(a);
78
ALTER TABLE foo SET (parallel_workers = 2);
89
}
910

@@ -14,17 +15,20 @@ teardown
1415

1516
session s1
1617
setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; }
17-
step s1r { SELECT * FROM foo; }
18+
step s1r { SELECT COUNT(*) FROM foo; }
1819
step s1c { COMMIT; }
1920

2021
session s2
2122
setup {
2223
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY;
2324
SET parallel_setup_cost = 0;
2425
SET parallel_tuple_cost = 0;
26+
SET min_parallel_index_scan_size = 0;
27+
SET parallel_leader_participation = off;
28+
SET enable_seqscan = off;
2529
}
26-
step s2r1 { SELECT * FROM foo; }
27-
step s2r2 { SELECT * FROM foo; }
30+
step s2r1 { SELECT COUNT(*) FROM foo; }
31+
step s2r2 { SELECT COUNT(*) FROM foo; }
2832
step s2c { COMMIT; }
2933

3034
permutation s1r s2r1 s1c s2r2 s2c

‎src/test/isolation/specs/serializable-parallel-3.spec

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# Exercise the case where a read-only serializable transaction has
2+
# SXACT_FLAG_RO_SAFE set in a parallel query. This variant is like
3+
# two copies of #2 running at the same time, and excercises the case
4+
# where another transaction has the same xmin, and it is the oldest.
5+
6+
setup
7+
{
8+
CREATE TABLE foo AS SELECT generate_series(1, 10)::int a;
9+
ALTER TABLE foo SET (parallel_workers = 2);
10+
}
11+
12+
teardown
13+
{
14+
DROP TABLE foo;
15+
}
16+
17+
session s1
18+
setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; }
19+
step s1r { SELECT * FROM foo; }
20+
step s1c { COMMIT; }
21+
22+
session s2
23+
setup {
24+
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY;
25+
SET parallel_setup_cost = 0;
26+
SET parallel_tuple_cost = 0;
27+
}
28+
step s2r1 { SELECT * FROM foo; }
29+
step s2r2 { SELECT * FROM foo; }
30+
step s2c { COMMIT; }
31+
32+
session s3
33+
setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; }
34+
step s3r { SELECT * FROM foo; }
35+
step s3c { COMMIT; }
36+
37+
session s4
38+
setup {
39+
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY;
40+
SET parallel_setup_cost = 0;
41+
SET parallel_tuple_cost = 0;
42+
}
43+
step s4r1 { SELECT * FROM foo; }
44+
step s4r2 { SELECT * FROM foo; }
45+
step s4c { COMMIT; }
46+
47+
permutation s1r s3r s2r1 s4r1 s1c s2r2 s3c s4r2 s4c s2c

0 commit comments

Comments
 (0)