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

Bruce Momjian wrote:
> Is this a TODO?

It's for from clear that avoing an exclusive ProcArray lock on subxact 
abort will bring a measurable performance benefit, so probably not.

I've actually coded a prototype for this a few months ago, to
check if it would bring any benefit at all, though I ran out of time 
before I had time to benchmark this, and I probably also lack the 
hardware for running high-concurrency tests.

> ---------------------------------------------------------------------------
> Florian G. Pflug wrote:
>> Tom Lane wrote:
>>> "Florian G. Pflug" <fgp@phlo.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.