On 30 May 2012 13:24, Robert Haas <robertmh...@gmail.com> wrote: > OK, but there are a lot of places where we call XLogFlush(), and it's > far from obvious that it's a win to do this in all of those cases. At > least, nobody's done that analysis. XLogFlush is called from: > > - WriteTruncateXlogRec(), which is called from TruncateCLOG() > - SlruPhysicalWritePage() > - EndPrepare() > - RecordTransactionCommitPrepared() > - RecordTransactionCommit() > - xact_redo_commit_internal() > - CreateCheckPoint() > - RelationTruncate() > - FlushBuffer() > - write_relmap_file() > > Most of those actually do look like reasonable places to try to get > grouped flushing behavior, but: > > 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 > 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. 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 would say the additional contention from waiting outweighs the benefit of the wait in those 3 places, so skipping the wait is wise. Should be implemented as #define XLogFlush(lsn) XLogFlushInternal(lsn, true) #define XLogFlushNoWait(lsn) XLogFlushInternal(lsn, false) so we can drop the no wait version in without touching the other call points > Another thing to think about is that if we do this across the board > rather than just for commits, then commit_delay and commit_siblings > will really be totally misnamed - they really ought to be flush_delay > and flush_siblings at that point. I think if we were to rename the parameters, then they should be called group_commit_delay group_commit_siblings Since "Group Commit" is the accepted term for this behaviour, even though, as you point out that the behaviour isn't just about commit. -- 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
