Re: BUG #8273: Assertion failure in 9.3 beta2 with serializable and savepoints

levertond(at)googlemail(dot)com wrote:

> START TRANSACTION ISOLATION LEVEL SERIALIZABLE;
> CREATE TABLE testing(
>   x INTEGER PRIMARY KEY
> );
> INSERT INTO testing VALUES(1);
> SELECT * FROM testing WHERE x = 1 FOR UPDATE;
> SAVEPOINT test;
> UPDATE testing SET x = 2 WHERE x = 1;
> ROLLBACK TO test;
> UPDATE testing SET x = 3 WHERE x = 1;
> ROLLBACK;

Thanks for the test case. This one-liner patch fixes it:

diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
index 55563ea..fa9c9d7 100644
--- a/src/backend/utils/time/tqual.c
+++ b/src/backend/utils/time/tqual.c
@@ -1287,7 +1287,7 @@ HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin,
{
if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid */
return HEAPTUPLE_INSERT_IN_PROGRESS;
- if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
+ if (HeapTupleHeaderIsOnlyLocked(tuple))
return HEAPTUPLE_INSERT_IN_PROGRESS;
/* inserted and then deleted by same xact */
return HEAPTUPLE_DELETE_IN_PROGRESS;

That is, I was taking a shortcut here by checking only the infomask bits
about locking, and not resolving the MultiXact to see if the updater
(sub)transaction was aborted; but this was wrong, because a tuple which
was created here and updated by an aborted subxact needs to be seen as
in-progress insertion, not an in-progress delete. This causes trouble
to the predicate.c code because it tries to obtain the update XID for
the tuple, but since the update has aborted, that returned zero, causing
the assert failure.

Sadly, this has performance implications, because what previously was
just an in-place check of bit flags has now become a function call.
Perhaps it'd be smart to optimize it a bit so that we first check the
flags, and only call the function if that fails.

Before pushing this patch, I'm going to examine the other users of
HEAP_XMAX_IS_LOCKED_ONLY to ensure they don't also need the same change.
I think some of them were prepared for the possibility that there were
aborted updaters; but this was a late addition so perhaps I missed
others that aren't.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services