Re: Maybe some more low-hanging fruit in the latestCompletedXid patch.

Tom Lane wrote:
> "Florian G. Pflug" <fgp(at)phlo(dot)org> writes:
>> Currently, we do not assume that either the childXids array, nor the xid
>> cache in the proc array are sorted by ascending xid order. I believe that
>> we could simplify the code, further reduce the locking requirements, and
>> enabled a transaction to de-overflow it's xid cache if we assume that those
>> arrays are in ascending xid order.
>
> "de-overflowing" the cache sounds completely unsafe, as other backends need
> that state to determine whether they need to look into pg_subtrans.

We'd only de-overflow if we abort *all* xids that are missing from the
xid cache. And only after marking them as aborted in the clog. If someone
concurrently checks for an overflow, and already sees the new (non-overflowed)
state, than he'll assume the xid is not running if he hasn't found it in
the array. Which is correct - we just aborted it.

Plus, removing the exclusive lock doesn't depend on de-overflowing. It's
just something that seems rather easy to do once the subxid handling is
in a state that allows concurrent removal of entries. If it turns out that
it's not that easy, than I'll just drop the idea again.

> I still don't believe you can avoid taking exclusive lock, either; your
> argument here did not address latestCompletedXid.

Sorry, not addressing latestCompletedXid was an oversight :-(.
My point is the we only *need* to advance latestCompletedXid on COMMITS. We do
so for aborts only to avoid running with unnecessarily low xmins after
a transaction ABORT. That corner case can only happen after a toplevel
ABORT, though - aborting subxacts cannot change the xmin, because the
toplevel xact will have a lower xid than any of it's subtransactions anyway.

We can therefore just remember the largest assigned xid for a given transaction,
and update latestCompletedXid to that on toplevel commit or abort. That
prevents that corner-case too, without updating latestCompletedXid during
subxact abort.

> But the main point remains this: there is no evidence whatsoever that these
> code paths are sufficiently performance-critical to be worth speeding up by
> making the code more fragile.

The gain will be less than that of the locking improvements done so far.
It will be a win for heavy users of plpgsql BEGIN/END/EXCEPTION blocks,
though I think.

We'll also save some cycles in TransactionIdIsInProgress, because we can
use a binary search, but that's just an added bonus.

I'm currently trying to code up a patch, since it's easier to judge the
correctness of actual code than that of a mere proposals. I'll do some
benchmarking when the patch is done to see if it brings measurable benefits.

greetings, Florian Pflug