Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 11ac4c7

Browse files
alvherrealvherre
committed
Don't ignore tuple locks propagated by our updates
If a tuple was locked by transaction A, and transaction B updated it, the new version of the tuple created by B would be locked by A, yet visible only to B; due to an oversight in HeapTupleSatisfiesUpdate, the lock held by A wouldn't get checked if transaction B later deleted (or key-updated) the new version of the tuple. This might cause referential integrity checks to give false positives (that is, allow deletes that should have been rejected). This is an easy oversight to have made, because prior to improved tuple locks in commit 0ac5ad5 it wasn't possible to have tuples created by our own transaction that were also locked by remote transactions, and so locks weren't even considered in that code path. It is recommended that foreign keys be rechecked manually in bulk after installing this update, in case some referenced rows are missing with some referencing row remaining. Per bug reported by Daniel Wood in CAPweHKe5QQ1747X2c0tA=5zf4YnS2xcvGf13Opd-1Mq24rF1cQ@mail.gmail.com
1 parent 65d6e4c commit 11ac4c7

File tree

6 files changed

+216
-2
lines changed

6 files changed

+216
-2
lines changed

‎src/backend/access/transam/multixact.c

+33
Original file line numberDiff line numberDiff line change
@@ -1274,6 +1274,39 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
12741274
return truelength;
12751275
}
12761276

1277+
/*
1278+
* MultiXactHasRunningRemoteMembers
1279+
* Does the given multixact have still-live members from
1280+
* transactions other than our own?
1281+
*/
1282+
bool
1283+
MultiXactHasRunningRemoteMembers(MultiXactId multi)
1284+
{
1285+
MultiXactMember *members;
1286+
int nmembers;
1287+
int i;
1288+
1289+
nmembers = GetMultiXactIdMembers(multi, &members, true);
1290+
if (nmembers <= 0)
1291+
return false;
1292+
1293+
for (i = 0; i < nmembers; i++)
1294+
{
1295+
/* not interested in our own members */
1296+
if (TransactionIdIsCurrentTransactionId(members[i].xid))
1297+
continue;
1298+
1299+
if (TransactionIdIsInProgress(members[i].xid))
1300+
{
1301+
pfree(members);
1302+
return true;
1303+
}
1304+
}
1305+
1306+
pfree(members);
1307+
return false;
1308+
}
1309+
12771310
/*
12781311
* mxactMemberComparator
12791312
* qsort comparison function for MultiXactMember

‎src/backend/utils/time/tqual.c

+34-2
Original file line numberDiff line numberDiff line change
@@ -493,8 +493,36 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
493493
if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid */
494494
return HeapTupleMayBeUpdated;
495495

496-
if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) /* not deleter */
497-
return HeapTupleMayBeUpdated;
496+
if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
497+
{
498+
TransactionId xmax;
499+
500+
xmax = HeapTupleHeaderGetRawXmax(tuple);
501+
502+
/*
503+
* Careful here: even though this tuple was created by our own
504+
* transaction, it might be locked by other transactions, if
505+
* the original version was key-share locked when we updated
506+
* it.
507+
*/
508+
509+
if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
510+
{
511+
if (MultiXactHasRunningRemoteMembers(xmax))
512+
return HeapTupleBeingUpdated;
513+
else
514+
return HeapTupleMayBeUpdated;
515+
}
516+
517+
/* if locker is gone, all's well */
518+
if (!TransactionIdIsInProgress(xmax))
519+
return HeapTupleMayBeUpdated;
520+
521+
if (!TransactionIdIsCurrentTransactionId(xmax))
522+
return HeapTupleBeingUpdated;
523+
else
524+
return HeapTupleMayBeUpdated;
525+
}
498526

499527
if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
500528
{
@@ -507,7 +535,11 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
507535

508536
/* updating subtransaction must have aborted */
509537
if (!TransactionIdIsCurrentTransactionId(xmax))
538+
{
539+
if (MultiXactHasRunningRemoteMembers(HeapTupleHeaderGetRawXmax(tuple)))
540+
return HeapTupleBeingUpdated;
510541
return HeapTupleMayBeUpdated;
542+
}
511543
else
512544
{
513545
if (HeapTupleHeaderGetCmax(tuple) >= curcid)

‎src/include/access/multixact.h

+1
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ extern bool MultiXactIdIsRunning(MultiXactId multi);
8989
extern void MultiXactIdSetOldestMember(void);
9090
extern int GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **xids,
9191
bool allow_old);
92+
extern bool MultiXactHasRunningRemoteMembers(MultiXactId multi);
9293
extern bool MultiXactIdPrecedes(MultiXactId multi1, MultiXactId multi2);
9394
extern bool MultiXactIdPrecedesOrEquals(MultiXactId multi1,
9495
MultiXactId multi2);

‎src/test/isolation/expected/propagate-lock-delete.out

+105
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
Parsed test spec with 3 sessions
2+
3+
starting permutation: s1b s1l s2b s2l s3b s3u s3d s1c s2c s3c
4+
step s1b: BEGIN;
5+
step s1l: INSERT INTO child VALUES (1);
6+
step s2b: BEGIN;
7+
step s2l: INSERT INTO child VALUES (1);
8+
step s3b: BEGIN;
9+
step s3u: UPDATE parent SET c=lower(c);
10+
step s3d: DELETE FROM parent; <waiting ...>
11+
step s1c: COMMIT;
12+
step s2c: COMMIT;
13+
step s3d: <... completed>
14+
error in steps s2c s3d: ERROR: update or delete on table "parent" violates foreign key constraint "child_i_fkey" on table "child"
15+
step s3c: COMMIT;
16+
17+
starting permutation: s1b s1l s2b s2l s3b s3u s3svu s3d s1c s2c s3c
18+
step s1b: BEGIN;
19+
step s1l: INSERT INTO child VALUES (1);
20+
step s2b: BEGIN;
21+
step s2l: INSERT INTO child VALUES (1);
22+
step s3b: BEGIN;
23+
step s3u: UPDATE parent SET c=lower(c);
24+
step s3svu: SAVEPOINT f; UPDATE parent SET c = 'bbb'; ROLLBACK TO f;
25+
step s3d: DELETE FROM parent; <waiting ...>
26+
step s1c: COMMIT;
27+
step s2c: COMMIT;
28+
step s3d: <... completed>
29+
error in steps s2c s3d: ERROR: update or delete on table "parent" violates foreign key constraint "child_i_fkey" on table "child"
30+
step s3c: COMMIT;
31+
32+
starting permutation: s1b s1l s2b s2l s3b s3u2 s3d s1c s2c s3c
33+
step s1b: BEGIN;
34+
step s1l: INSERT INTO child VALUES (1);
35+
step s2b: BEGIN;
36+
step s2l: INSERT INTO child VALUES (1);
37+
step s3b: BEGIN;
38+
step s3u2: UPDATE parent SET i = i;
39+
step s3d: DELETE FROM parent; <waiting ...>
40+
step s1c: COMMIT;
41+
step s2c: COMMIT;
42+
step s3d: <... completed>
43+
error in steps s2c s3d: ERROR: update or delete on table "parent" violates foreign key constraint "child_i_fkey" on table "child"
44+
step s3c: COMMIT;
45+
46+
starting permutation: s1b s1l s2b s2l s3b s3u2 s3svu s3d s1c s2c s3c
47+
step s1b: BEGIN;
48+
step s1l: INSERT INTO child VALUES (1);
49+
step s2b: BEGIN;
50+
step s2l: INSERT INTO child VALUES (1);
51+
step s3b: BEGIN;
52+
step s3u2: UPDATE parent SET i = i;
53+
step s3svu: SAVEPOINT f; UPDATE parent SET c = 'bbb'; ROLLBACK TO f;
54+
step s3d: DELETE FROM parent; <waiting ...>
55+
step s1c: COMMIT;
56+
step s2c: COMMIT;
57+
step s3d: <... completed>
58+
error in steps s2c s3d: ERROR: update or delete on table "parent" violates foreign key constraint "child_i_fkey" on table "child"
59+
step s3c: COMMIT;
60+
61+
starting permutation: s1b s1l s3b s3u s3d s1c s3c
62+
step s1b: BEGIN;
63+
step s1l: INSERT INTO child VALUES (1);
64+
step s3b: BEGIN;
65+
step s3u: UPDATE parent SET c=lower(c);
66+
step s3d: DELETE FROM parent; <waiting ...>
67+
step s1c: COMMIT;
68+
step s3d: <... completed>
69+
error in steps s1c s3d: ERROR: update or delete on table "parent" violates foreign key constraint "child_i_fkey" on table "child"
70+
step s3c: COMMIT;
71+
72+
starting permutation: s1b s1l s3b s3u s3svu s3d s1c s3c
73+
step s1b: BEGIN;
74+
step s1l: INSERT INTO child VALUES (1);
75+
step s3b: BEGIN;
76+
step s3u: UPDATE parent SET c=lower(c);
77+
step s3svu: SAVEPOINT f; UPDATE parent SET c = 'bbb'; ROLLBACK TO f;
78+
step s3d: DELETE FROM parent; <waiting ...>
79+
step s1c: COMMIT;
80+
step s3d: <... completed>
81+
error in steps s1c s3d: ERROR: update or delete on table "parent" violates foreign key constraint "child_i_fkey" on table "child"
82+
step s3c: COMMIT;
83+
84+
starting permutation: s1b s1l s3b s3u2 s3d s1c s3c
85+
step s1b: BEGIN;
86+
step s1l: INSERT INTO child VALUES (1);
87+
step s3b: BEGIN;
88+
step s3u2: UPDATE parent SET i = i;
89+
step s3d: DELETE FROM parent; <waiting ...>
90+
step s1c: COMMIT;
91+
step s3d: <... completed>
92+
error in steps s1c s3d: ERROR: update or delete on table "parent" violates foreign key constraint "child_i_fkey" on table "child"
93+
step s3c: COMMIT;
94+
95+
starting permutation: s1b s1l s3b s3u2 s3svu s3d s1c s3c
96+
step s1b: BEGIN;
97+
step s1l: INSERT INTO child VALUES (1);
98+
step s3b: BEGIN;
99+
step s3u2: UPDATE parent SET i = i;
100+
step s3svu: SAVEPOINT f; UPDATE parent SET c = 'bbb'; ROLLBACK TO f;
101+
step s3d: DELETE FROM parent; <waiting ...>
102+
step s1c: COMMIT;
103+
step s3d: <... completed>
104+
error in steps s1c s3d: ERROR: update or delete on table "parent" violates foreign key constraint "child_i_fkey" on table "child"
105+
step s3c: COMMIT;

‎src/test/isolation/isolation_schedule

+1
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,6 @@ test: delete-abort-savept-2
2121
test: aborted-keyrevoke
2222
test: multixact-no-deadlock
2323
test: multixact-no-forget
24+
test: propagate-lock-delete
2425
test: drop-index-concurrently-1
2526
test: timeouts

‎src/test/isolation/specs/propagate-lock-delete.spec

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# When an update propagates a preexisting lock on the updated tuple, make sure
2+
# we don't ignore the lock in subsequent operations of the new version. (The
3+
# version with the aborted savepoint uses a slightly different code path).
4+
setup
5+
{
6+
create table parent (i int, c char(3));
7+
create unique index parent_idx on parent (i);
8+
insert into parent values (1, 'AAA');
9+
create table child (i int references parent(i));
10+
}
11+
12+
teardown
13+
{
14+
drop table child, parent;
15+
}
16+
17+
session "s1"
18+
step "s1b" { BEGIN; }
19+
step "s1l" { INSERT INTO child VALUES (1); }
20+
step "s1c" { COMMIT; }
21+
22+
session "s2"
23+
step "s2b" { BEGIN; }
24+
step "s2l" { INSERT INTO child VALUES (1); }
25+
step "s2c" { COMMIT; }
26+
27+
session "s3"
28+
step "s3b" { BEGIN; }
29+
step "s3u" { UPDATE parent SET c=lower(c); } # no key update
30+
step "s3u2" { UPDATE parent SET i = i; } # key update
31+
step "s3svu" { SAVEPOINT f; UPDATE parent SET c = 'bbb'; ROLLBACK TO f; }
32+
step "s3d" { DELETE FROM parent; }
33+
step "s3c" { COMMIT; }
34+
35+
permutation "s1b" "s1l" "s2b" "s2l" "s3b" "s3u" "s3d" "s1c" "s2c" "s3c"
36+
permutation "s1b" "s1l" "s2b" "s2l" "s3b" "s3u" "s3svu" "s3d" "s1c" "s2c" "s3c"
37+
permutation "s1b" "s1l" "s2b" "s2l" "s3b" "s3u2" "s3d" "s1c" "s2c" "s3c"
38+
permutation "s1b" "s1l" "s2b" "s2l" "s3b" "s3u2" "s3svu" "s3d" "s1c" "s2c" "s3c"
39+
permutation "s1b" "s1l" "s3b" "s3u" "s3d" "s1c" "s3c"
40+
permutation "s1b" "s1l" "s3b" "s3u" "s3svu" "s3d" "s1c" "s3c"
41+
permutation "s1b" "s1l" "s3b" "s3u2" "s3d" "s1c" "s3c"
42+
permutation "s1b" "s1l" "s3b" "s3u2" "s3svu" "s3d" "s1c" "s3c"

0 commit comments

Comments
 (0)