On Friday, June 22, 2012 04:34:33 PM Robert Haas wrote: > 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. Well, before the simple fix Simon applied after my initial complaint you didn't get wakeups *at all* in the synchronous_commit=off case.
Now, with the additional changes, the walsender is woken exactly when data is available to send and not always when a commit happens. I played around with various scenarios and it always was a win. One reason is that the walreceiver often is a bottleneck because it fsyncs the received data immediately so a less blocky transfer pattern is reducing that problem a bit. > 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. True. Greetings, Andres -- Andres Freund 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
