On Wed, May 30, 2012 at 4:36 AM, Simon Riggs <si...@2ndquadrant.com> wrote: > When I read this the first time, I was in full agreement. > > On closer inspection neither point is valid, though both points were > worth considering. > >> Well, consider the one in the background writer, for example. That's >> just a periodic flush, so I see no benefit in having it acquire the >> lock and then wait some more. It already did wait. > > We use XLogBackgroundFlush() not XLogFlush() from background processes. > >> And what about >> the case where we're flushing while holding WALInsertLock because the >> buffer's full? Clearly waiting is useless in that case - nobody can >> join the group commit for exactly the same reason that we're doing the >> flush in the first place: no buffer space. > > We don't flush WAL in that case, we just write it.
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. 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. 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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
