On 30 May 2012 17:19, Peter Geoghegan <pe...@2ndquadrant.com> wrote: > On 30 May 2012 15:25, Simon Riggs <si...@2ndquadrant.com> wrote: >>> 1. It seems wrong to do it in xact_redo_commit_internal(). It won't >>> matter if commit_siblings>0 since there won't be any other backends >>> with transaction IDs anyway, but if commit_siblings==0 then we'll >>> sleep for no possible benefit. >> >> Agreed
I've looked at this more closely now and I can see that the call to XLogFlush() that is made from xact_redo_commit_internal() doesn't ever actually flush WAL, so whether we delay or not is completely irrelevant. So un-agreed. No change required to patch there. >>> 2. Doing it in FlushBuffer() seems slightly iffy since we might be >>> sitting on a buffer lock. But maybe it's a win anyway, or just not >>> worth worrying about. >> >> Agreed. > > As I've pointed out, we cannot meaningfully skip the wait for > auxiliary processes alone (except perhaps by having commit_siblings > set sufficiently high). > >> The remaining cases aren't worth worrying about, apart from >> SlruPhysicalWritePage() which happens during visibility checks and >> needs to happen as quickly as possible also. > > I'm not so sure. It says in that function: > > /* > * We must determine the largest async-commit LSN for the > page. This > * is a bit tedious, but since this entire function is a slow > path > * anyway, it seems better to do this here than to maintain a > per-page > * LSN variable (which'd need an extra comparison in the > * transaction-commit path). > */ > >> I would say the additional contention from waiting outweighs the >> benefit of the wait in those 3 places, so skipping the wait is wise. > > MinimumActiveBackends() reports the "count backends (other than > myself) that are in active transactions", so unnecessary calls will > have to occur when we have active transactions >= CommitSiblings, not > connections >= CommitSiblings as was previously the case. > > What if we were to skip the wait during recovery only, by specially > setting CommitDelay to 0 in the start-up process? Would that satisfy > everyone's concerns about unhelpful delays? I'm not sure how this > might interact with hot standby. Hmm, that was a good idea, but as of my comments above, that isn't required or useful. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
