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" <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
