On Tue, Dec 03, 2013 at 04:37:58PM +0100, Andres Freund wrote:
> On 2013-12-03 10:29:54 -0500, Noah Misch wrote:
> > Sorry, my original report was rather terse. I speak of the scenario
> > exercised
> > by the second permutation in that isolationtester spec. The multixact is
> > later than VACUUM's cutoff_multi, so 9.3.1 does not freeze it at all. 9.3.2
> > does freeze it to InvalidTransactionId per the code I cited in my first
> > response on this thread, which wrongly removes a key lock.
>
> That one is clear, I was only confused about the Assert() you
> reported. But I think I've explained that elsewhere.
>
> I currently don't see fixing the errorneous freezing of lockers (not the
> updater though) without changing the wal format or synchronously waiting
> for all lockers to end. Which both see like a no-go?
Not fixing it at all is the real no-go. We'd take both of those undesirables
before just tolerating the lost locks in 9.3.
The attached patch illustrates the approach I was describing earlier. It
fixes the test case discussed above. I haven't verified that everything else
in the system is ready for it, so this is just for illustration purposes.
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
*** a/src/backend/access/heap/heapam.c
--- b/src/backend/access/heap/heapam.c
***************
*** 5384,5412 **** heap_freeze_tuple(HeapTupleHeader tuple, TransactionId
cutoff_xid,
TransactionId update_xid;
/*
! * This is a multixact which is not marked LOCK_ONLY,
but which
! * is newer than the cutoff_multi. If the update_xid
is below the
! * cutoff_xid point, then we can just freeze the Xmax
in the
! * tuple, removing it altogether. This seems simple,
but there
! * are several underlying assumptions:
! *
! * 1. A tuple marked by an multixact containing a very
old
! * committed update Xid would have been pruned away by
vacuum; we
! * wouldn't be freezing this tuple at all.
! *
! * 2. There cannot possibly be any live locking members
remaining
! * in the multixact. This is because if they were
alive, the
! * update's Xid would had been considered, via the
lockers'
! * snapshot's Xmin, as part the cutoff_xid.
! *
! * 3. We don't create new MultiXacts via
MultiXactIdExpand() that
! * include a very old aborted update Xid: in that
function we only
! * include update Xids corresponding to transactions
that are
! * committed or in-progress.
*/
update_xid = HeapTupleGetUpdateXid(tuple);
if (TransactionIdPrecedes(update_xid, cutoff_xid))
! freeze_xmax = true;
}
}
else if (TransactionIdIsNormal(xid) &&
--- 5384,5404 ----
TransactionId update_xid;
/*
! * This is a multixact which is not marked LOCK_ONLY,
but which is
! * newer than the cutoff_multi. To advance
relfrozenxid, we must
! * eliminate any updater XID that falls below
cutoff_xid.
! * Affected multixacts may also contain outstanding
lockers, which
! * we must preserve. Forming a new multixact is
tempting, but
! * solving it that way requires a WAL format change.
Instead,
! * mark the tuple LOCK_ONLY. The updater XID is still
present,
! * but consuming code will notice LOCK_ONLY and assume
it's gone.
*/
update_xid = HeapTupleGetUpdateXid(tuple);
if (TransactionIdPrecedes(update_xid, cutoff_xid))
! {
! tuple->t_infomask |= HEAP_XMAX_LOCK_ONLY;
! changed = true;
! }
}
}
else if (TransactionIdIsNormal(xid) &&
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
