On Fri, Jun 22, 2012 at 10:19 AM, Andres Freund <and...@2ndquadrant.com> wrote: > On Friday, June 22, 2012 04:09:59 PM Robert Haas wrote: >> >> I am not convinced that it's a good idea to wake up every walsender >> >> every time we do XLogInsert(). XLogInsert() is a super-hot code path, >> >> and adding more overhead there doesn't seem warranted. We need to >> >> replicate commit, commit prepared, etc. quickly, by why do we need to >> >> worry about a short delay in replicating heap_insert/update/delete, >> >> for example? They don't really matter until the commit arrives. 7 >> >> seconds might be a bit long, but that could be fixed by decreasing the >> >> polling interval for walsender to, say, a second. >> > >> > Its not woken up every XLogInsert call. Its only woken up if there was an >> > actual disk write + fsync in there. Thats exactly the point of the patch. >> Sure, but it's still adding cycles to XLogInsert. I'm not sure that >> XLogBackgroundFlush() is the right place to be doing this, but at >> least it's in the background rather than the foreground. > It adds one if() if nothing was fsynced. If something was written and fsynced > inside XLogInsert some kill() calls are surely not the problem. > >> > The wakeup rate is actually lower for synchronous_commit=on than before >> > because then it unconditionally did a wakeup for every commit (and >> > similar) and now only does that if something has been written + fsynced. >> I'm a bit confused by this, because surely if there's been a commit, >> then WAL has been written and fsync'd, but the reverse is not true. > As soon as you have significant concurrency by the time the XLogFlush in > RecordTransactionCommit() is reached another backend or the wal writer may > have already fsynced the wal up to the requested point. In that case no wakeup > will performed by the comitting backend at all. 9.2 improved the likelihood of > that as you know.
Hmm, well, I guess. I'm still not sure I really understand what benefit we're getting out of this. If we lose a few WAL records for an uncommitted transaction, who cares? That transaction is gone anyway. As an implementation detail, I suggest rewriting WalSndWakeupRequest and WalSndWakeupProcess as macros. The old code does an in-line test for max_wal_senders > 0, which suggests that somebody thought the function call overhead might be enough to matter here. Perhaps they were wrong, but it shouldn't hurt anything to keep it that way. -- 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
