On Tue, Feb 25, 2014 at 11:23 AM, Andres Freund <and...@2ndquadrant.com> wrote: > On 2014-02-25 11:15:46 -0500, Robert Haas wrote: >> On Tue, Feb 25, 2014 at 10:54 AM, Andres Freund <and...@2ndquadrant.com> >> wrote: >> > I am not sure I can follow. Why doesn't it make sense to send out the >> > keepalive (with replyRequested = true) when we're busy sending stuff >> > (which will be the case most of the time on a busy server)? >> >> It may very well make sense, but your patch won't generally have that >> effect, because with the patch you proposed, the keep-alive can only >> be sent when the server is busy if the write queue is also full. > > Well, it either needs to be caughtup *or*/and have a busy write queue, > right?
Right. > 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. > Also, there's no timeout checks outside that if (caughtup || > send_pending()) block, so there's not much of a window to hit problems when > that loop isn't entered. That might be true, but waiting until the write queue is full to send a ping and then checking whether we've timed out before the queue has drained isn't a sane design pattern. -- 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
