On Wed, 2010-06-23 at 21:54 +0000, Robert Haas wrote:
> Log Message:
> -----------
> Add TCP keepalive support to libpq.
I misunderstood the earlier discussion on this and didn't realise you
were considering committing in this way. For me, this is two patches,
not one. I object to one and like the other.
> This adds four additional connection parameters to libpq: keepalives,
> keepalives_idle, keepalives_count, and keepalives_interval.
> keepalives default to on, per discussion, but can be turned off by
> specifying keepalives=0. The remaining parameters, where supported,
> can be used to adjust how often keepalives are sent and how many
> can be lost before the connection is broken.
There isn't any need at at all for this. We can already add options on
the libpq connection line.
options = '-o tcp_keepalives_idle=X
tcp_keepalives_interval=Y
tcp_keepalives_count=Z'
I see zero need to further complicate the libpq interface. If it changes
frequently between releases then supporting PostgreSQL programs becomes
much harder and leads to software not working correctly with Postgres.
Connecting to Postgres is already too complex.
At most it needs an example in the manual to show how to do this the
existing way.
I'm sorry to express this opinion now, again because I misunderstood.
> The immediate motivation for this patch is to make sure that
> walreceiver will eventually notice if the master reboots without
> closing the connection cleanly, but it should be helpful in other
> cases as well.
My understanding is that the motivation for this is that the above
options parameter would not have worked? So I regard making that work as
a bug fix, so agree with some parts of this patch, I think.
-- Simon Riggs www.2ndQuadrant.com
