Alvaro Herrera wrote:
> The fix for the immediate bug is to add some code to HTSU so that it
> checks for locks by other transactions even when the tuple was created
> by us. I haven't looked at the other tqual routines yet, but I imagine
> they will need equivalent fixes.
This POC patch changes the two places in HeapTupleSatisfiesUpdate that
need to be touched for this to work. This is probably too simplistic,
in that I make the involved cases return HeapTupleBeingUpdated without
checking that there actually are remote lockers, which is the case of
concern. I'm not yet sure if this is the final form of the fix, or
instead we should expand the Multi (in the cases where there is a multi)
and verify whether any lockers are transactions other than the current
one. As is, nothing seems to break, but I think that's probably just
chance and should not be relied upon.
Attached are two isolation specs which illustrate both the exact issue
reported by Dan, and a similar one which involves an aborted
subtransaction having updated the second version of the row. (This
takes a slightly different code path.)
As far as I can tell, no other routine in tqual.c needs to change other
than HeapTupleSatisfiesUpdate. The ones that test for visibility
(Dirty, Any, Self) are only concerned with whether the tuple is visible,
and of course that won't be affected by the tuple being locked; and
HeapTupleSatisfiesVacuum is only concerned with the tuple being dead,
which similarly won't.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
setup
{
create extension pageinspect;
create table parent (i int, c char(3));
create unique index parent_idx on parent (i);
insert into parent values (1, 'AAA');
create table child (i int references parent(i));
}
teardown
{
-- drop table child, parent;
-- drop extension pageinspect;
}
session "s1"
step "s1b" { BEGIN; select txid_current(); }
step "s1i" { INSERT INTO child VALUES (1); }
step "s1e" { SELECT lp, t_xmin, t_xmax, to_hex(t_infomask) as mask,
to_hex(t_infomask2) as mask2 FROM heap_page_items(get_raw_page('parent', 0)); }
step "s1e2" { SELECT lp, t_xmax, members.* FROM
heap_page_items(get_raw_page('parent', 0)), pg_get_multixact_members(t_xmax)
members where t_infomask & x'1000'::int <> 0; }
step "s1c" { COMMIT; }
session "s2"
step "s2b" { BEGIN; select txid_current(); }
step "s2u" { UPDATE parent SET c=lower(c); }
# step "s2u2" { UPDATE parent SET i=i; }
step "s2svu" { SAVEPOINT f; UPDATE parent SET c = 'bbb'; ROLLBACK TO f; }
step "s2d" { DELETE FROM parent returning ctid; }
step "s2c" { COMMIT; }
permutation "s1b" "s1i" "s2b" "s2u" "s2svu" "s1e" "s1e2" "s2d" "s1e" "s1e2"
"s1c" "s2c"
#permutation "s1b" "s1i" "s2b" "s2u2" "s2d" "s1c" "s2c"
#permutation "s1b" "s1i" "s2b" "s2d" "s1c" "s2c"
setup
{
create extension pageinspect;
create table parent (i int, c char(3));
create unique index parent_idx on parent (i);
insert into parent values (1, 'AAA');
create table child (i int references parent(i));
}
teardown
{
-- drop table child, parent;
-- drop extension pageinspect;
}
session "s1"
step "s1b" { BEGIN; select txid_current(); }
step "s1i" { INSERT INTO child VALUES (1); }
step "s1e" { SELECT lp, t_xmin, t_xmax, to_hex(t_infomask) as mask,
to_hex(t_infomask2) as mask2 FROM heap_page_items(get_raw_page('parent', 0)); }
step "s1e2" { SELECT lp, t_xmax, members.* FROM
heap_page_items(get_raw_page('parent', 0)), pg_get_multixact_members(t_xmax)
members where t_infomask & x'1000'::int <> 0; }
step "s1c" { COMMIT; }
session "s2"
step "s2b" { BEGIN; select txid_current(); }
step "s2u" { UPDATE parent SET c=lower(c); }
step "s2u2" { UPDATE parent SET i=i; }
step "s2d" { DELETE FROM parent returning ctid; }
step "s2c" { COMMIT; }
permutation "s1b" "s1i" "s2b" "s2u" "s1e" "s1e2" "s2d" "s1e" "s1e2" "s1c" "s2c"
#permutation "s1b" "s1i" "s2b" "s2u2" "s2d" "s1c" "s2c"
#permutation "s1b" "s1i" "s2b" "s2d" "s1c" "s2c"
*** a/src/backend/utils/time/tqual.c
--- b/src/backend/utils/time/tqual.c
***************
*** 687,693 **** HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, CommandId curcid,
--- 687,699 ----
return HeapTupleMayBeUpdated;
if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) /* not deleter */
+ {
+ if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
+ return HeapTupleBeingUpdated;
+ else if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmax(tuple)))
+ return HeapTupleBeingUpdated;
return HeapTupleMayBeUpdated;
+ }
if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
{
***************
*** 700,706 **** HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, CommandId curcid,
/* updating subtransaction must have aborted */
if (!TransactionIdIsCurrentTransactionId(xmax))
! return HeapTupleMayBeUpdated;
else
{
if (HeapTupleHeaderGetCmax(tuple) >= curcid)
--- 706,712 ----
/* updating subtransaction must have aborted */
if (!TransactionIdIsCurrentTransactionId(xmax))
! return HeapTupleBeingUpdated;
else
{
if (HeapTupleHeaderGetCmax(tuple) >= curcid)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
