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

Thanks for the feedback.

---------------------------------------------------------------------------

Florian G. Pflug wrote:
> 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.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +