[COMMITTERS] pgsql: Add TCP keepalive support to libpq.
in [PgSql]
|
From: Simon Riggs on 24 Jun 2010 03:10 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 24 Jun 2010 10:13 Simon Riggs <simon(a)2ndQuadrant.com> writes: > On Wed, 2010-06-23 at 21:54 +0000, Robert Haas wrote: >> 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' Huh? The above is 100% fanciful; there was no code in libpq or anywhere else that would have processed such a thing. The bigger picture is that this patch is needed, not only for walreceiver but for many other purposes --- the feature has been requested repeatedly over the years and was already in the 9.1 commitfest queue. We moved it up because it seemed fairly important for walreceiver's purposes, but it would have gotten done in the very near future anyway. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 24 Jun 2010 10:30 On Thu, Jun 24, 2010 at 10:13 AM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > Simon Riggs <simon(a)2ndQuadrant.com> writes: >> On Wed, 2010-06-23 at 21:54 +0000, Robert Haas wrote: >>> 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' > > Huh? �The above is 100% fanciful; there was no code in libpq or anywhere > else that would have processed such a thing. You can do this: psql "host=127.0.0.1 options='-c tcp_keepalives_idle=1'" ....but it doesn't do the same thing as this patch. It lets you set the TCP keepalive parameters on the server side, whereas what this patch does is let you set them on the client side. Only setting them on the client side will allow the client to notice when the server has gone away. There is still an open question in my mind as to whether this is really an adequate solution to the walrecevier problem, but as you say, if it turns out not to be, it's got other value. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Florian Pflug on 24 Jun 2010 10:40 On Jun 24, 2010, at 16:30 , Robert Haas wrote: > On Thu, Jun 24, 2010 at 10:13 AM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >> Simon Riggs <simon(a)2ndQuadrant.com> writes: >>> On Wed, 2010-06-23 at 21:54 +0000, Robert Haas wrote: >>>> 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' >> >> Huh? The above is 100% fanciful; there was no code in libpq or anywhere >> else that would have processed such a thing. > > You can do this: > > psql "host=127.0.0.1 options='-c tcp_keepalives_idle=1'" Hm, seems a bit error-prone though. The difference between the above psql "host=127.0.0.1 keepalives=1" isn't immediately obvious I'd say. Should we maybe rename the libpq-side parameters to tcp_client_keepalives, tcp_client_keepalives_idle, tcp_client_keepalives_count and tcp_client_keepalives_interval? Or do we expect people who fiddle with those parameters to understand the subtle difference? best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 24 Jun 2010 10:45 On Thu, Jun 24, 2010 at 10:40 AM, Florian Pflug <fgp(a)phlo.org> wrote: > On Jun 24, 2010, at 16:30 , Robert Haas wrote: >> On Thu, Jun 24, 2010 at 10:13 AM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >>> Simon Riggs <simon(a)2ndQuadrant.com> writes: >>>> 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' >>> >>> Huh? �The above is 100% fanciful; there was no code in libpq or anywhere >>> else that would have processed such a thing. >> >> You can do this: >> >> psql "host=127.0.0.1 options='-c tcp_keepalives_idle=1'" > > Hm, seems a bit error-prone though. The difference between the above > > psql "host=127.0.0.1 keepalives=1" > > isn't immediately obvious I'd say. > > Should we maybe rename the libpq-side parameters to tcp_client_keepalives, tcp_client_keepalives_idle, tcp_client_keepalives_count and tcp_client_keepalives_interval? Or do we expect people who fiddle with those parameters to understand the subtle difference? I think the existing names are fine - people should understand that "options" means "server-side options" and that anything else is a client-side option. However, if there's a strong consensus the other way and someone feels like working up a patch, that's fine too. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
|
Next
|
Last
Pages: 1 2 3 Prev: ECPG FETCH readahead Next: [HACKERS] TOAST issue on custom index access method |