Re: foreign key locks

Andres Freund wrote:

> - I find using a default: clause in switches with enum types where everything
> is expected to be handled like the following a bad idea, this way the
> compiler won't warn you if youve missed case's which makes changing/extending code harder:
> switch (rc->strength)
> {

I eliminated some of these default clauses, but the compiler is not
happy about not having some of them, so they remain.

> -
> #if 0
> /*
> * The idea here is to remove the IS_MULTI bit, and replace the
> * xmax with the updater's Xid. However, we can't really do it:
> * modifying the Xmax is not allowed under our buffer locking
> * rules, unless we have an exclusive lock; but we don't know that
> * we have it. So the multi needs to remain in place :-(
> */
> ResetMultiHintBit(tuple, buffer, xmax, true);
> #endif
>
> Three things:
> - HeapTupleSatisfiesUpdate is actually always called exclusively locked ;)
> - Extending something like LWLockHeldByMe to also return the current
> lockmode doesn't sound hard
> - we seem to be using #ifdef NOT_YET for such cases

After spending some time trying to make this work usefully, I observed
that it's pointless, at least if we apply it only in
HeapTupleSatisfiesUpdate, because the IS_MULTI bit is going to be
removed by compute_new_xmax_infomask if the multi is gone. Something
like this would be useful if we could do it in other places; say when
we're merely scanning page without the specific intention of modifying
that particular tuple. Maybe in heap_prune_page, for example. ISTM we
can see that as an optimization we can add later.

For the record, the implementation of ResetMultiHintBit looks like this:

/*
* When a tuple is found to be marked by a MultiXactId, all whose members are
* completed transactions, the HEAP_XMAX_IS_MULTI bit can be removed.
* Additionally, at the request of caller, we can set the Xmax to the given
* Xid, and set HEAP_XMAX_COMMITTED.
*
* Note that per buffer access rules, the caller to this function must hold
* exclusive content lock on the affected buffer.
*/
static inline void
ResetMultiHintBit(HeapTupleHeader tuple, Buffer buffer,
TransactionId xid, bool mark_committed)
{
Assert(LWLockModeHeld((&BufferDescriptors[buffer])->content_lock ==
LW_EXCLUSIVE));
Assert(tuple->t_infomask & HEAP_XMAX_IS_MULTI);
Assert(!(tuple->t_infomask & HEAP_XMAX_INVALID));
Assert(!MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)));

tuple->t_infomask &= ~HEAP_XMAX_BITS;
HeapTupleHeaderSetXmax(tuple, xid);
if (!TransactionIdIsValid(xid))
tuple->t_infomask |= HEAP_XMAX_INVALID;
/* note that HEAP_KEYS_UPDATED persists, if set */

if (mark_committed)
SetHintBits(tuple, buffer, HEAP_XMAX_COMMITTED, xid);
else
SetBufferCommitInfoNeedsSave(buffer);
}

(This is removed by commit 52f86f82fb3e01a6ed0b8106bac20f319bb3ad0a in
my github tree, but that commit might be lost in the future, hence this
paste.)

Some of your other observations have been fixed by commits that I have
pushed to my github tree.

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