On 2014-03-05 18:26:13 +0200, Heikki Linnakangas wrote: > On 02/25/2014 06:41 PM, Robert Haas wrote: > >On Tue, Feb 25, 2014 at 11:23 AM, Andres Freund <and...@2ndquadrant.com> > >wrote: > >>Usually that state will be reached very quickly because before > >>that we're writing data to the network as fast as it can be read from > >>disk. > > > >I'm unimpressed. Even if that is in practice true, making the code > >self-consistent is a goal of non-trivial value. The timing of sending > >keep-alives has no business depending on the state of the write queue, > >and right now it doesn't. Your patch would make it depend on that, > >mostly by accident AFAICS.
I still don't see how my proposed patch increases the dependency, rather the contrary, it's less dependant on the flushing behaviour. But I agree that a more sweeping change is a good idea. > The logic was the same before the patch, but I added the XXX comment above. > Why do we sleep in increments of 1/10 of wal_sender_timeout? Originally, the > code calculated when the next wakeup should happen, by adding > wal_sender_timeout (or replication_timeout, as it was called back then) to > the time of the last reply. Why don't we do that? > [ archeology ] It imo makes sense to wakeup after last_reply + wal_sender_timeout/2, so a requested reply actually has time to arrive, but otherwise I agree. I think your patch makes sense. Additionally imo the timeout checking should be moved outside the if (caughtup || pq_is_send_pending()), but that's probably a separate patch. Any chance you could apply your patch soon? I've a patch pending that'll surely conflict with this and it seems better to fix it first. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
