TCP keepalive support for libpq
in [PgSql]
|
From: Robert Haas on 22 Jun 2010 09:20 On Mon, Feb 15, 2010 at 8:58 PM, Fujii Masao <masao.fujii(a)gmail.com> wrote: > On Tue, Feb 16, 2010 at 1:18 AM, Robert Haas <robertmhaas(a)gmail.com> wrote: >>> I'm all for this as a 9.1 submission, but let's not commit to trying to >>> debug it now. �I would like a green buildfarm for awhile before we wrap >>> alpha4, and this sort of untested "it can't hurt" patch is exactly what >>> is likely to make things not green. >> >> Mmm. �OK, fair enough. > > Okay. I added the patch to the first CF for v9.1. > Let's discuss about it later. There is talk of applying this patch, or something like it, for 9.0, so I guess now is the time for discussion. The overriding issue is that we need walreceiver to notice if the master goes away. Rereading this thread, the major argument against applying this patch is that it changes the default behavior: it ALWAYS enables keepalives, and then additionally provides libpq parameters to change some related parameters (number of seconds between sending keepalives, number of seconds after which to retransmit a keepalive, number of lost keepalives after which a connection is declared dead). Although the consensus seems to be that keepalives are a good idea much more often than not, I am wary of unconditionally turning on a behavior that has, in previous releases, been unconditionally turned off. I don't want to do this in 9.0, and I don't think I want to do it in 9.1, either. What I think would make sense is to add an option to control whether keepalives get turned on. If you say keepalives=1, you get on = 1; setsockopt(conn->sock, SOL_SOCKET, SO_KEEPALIVE, (char *) &on, sizeof(on); if you say keepalives=0, we do nothing special. If you say neither, you get the default behavior, which I'm inclined to make keepalives=1. That way, everyone gets the benefit of this patch (keepalives turned on) by default, but if for some reason someone is using libpq over the deep-space network or a connection for which they pay by the byte, they can easily shut it off. We can note the behavior change under "observe the following incompatibilities". I am inclined to punt the keepalives_interval, keepalives_idle, and keepalives_count parameters to 9.1. If these are needed for walreciever to work reliably, this whole approach is a dead-end, because those parameters are not portable. I will post a patch later today along these lines. -- 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: Magnus Hagander on 22 Jun 2010 09:27 On Tue, Jun 22, 2010 at 15:20, Robert Haas <robertmhaas(a)gmail.com> wrote: > On Mon, Feb 15, 2010 at 8:58 PM, Fujii Masao <masao.fujii(a)gmail.com> wrote: >> On Tue, Feb 16, 2010 at 1:18 AM, Robert Haas <robertmhaas(a)gmail.com> wrote: >>>> I'm all for this as a 9.1 submission, but let's not commit to trying to >>>> debug it now. �I would like a green buildfarm for awhile before we wrap >>>> alpha4, and this sort of untested "it can't hurt" patch is exactly what >>>> is likely to make things not green. >>> >>> Mmm. �OK, fair enough. >> >> Okay. I added the patch to the first CF for v9.1. >> Let's discuss about it later. > > There is talk of applying this patch, or something like it, for 9.0, > so I guess now is the time for discussion. �The overriding issue is > that we need walreceiver to notice if the master goes away. �Rereading > this thread, the major argument against applying this patch is that it > changes the default behavior: it ALWAYS enables keepalives, and then > additionally provides libpq parameters to change some related > parameters (number of seconds between sending keepalives, number of > seconds after which to retransmit a keepalive, number of lost > keepalives after which a connection is declared dead). �Although the > consensus seems to be that keepalives are a good idea much more often > than not, I am wary of unconditionally turning on a behavior that has, > in previous releases, been unconditionally turned off. �I don't want > to do this in 9.0, and I don't think I want to do it in 9.1, either. > > What I think would make sense is to add an option to control whether > keepalives get turned on. � If you say keepalives=1, you get on = 1; > setsockopt(conn->sock, SOL_SOCKET, SO_KEEPALIVE, > (char *) &on, sizeof(on); if you say keepalives=0, we do nothing > special. �If you say neither, you get the default behavior, which I'm > inclined to make keepalives=1. �That way, everyone gets the benefit of > this patch (keepalives turned on) by default, but if for some reason > someone is using libpq over the deep-space network or a connection for > which they pay by the byte, they can easily shut it off. �We can note > the behavior change under "observe the following incompatibilities". +1 on enabling it by default, but providing a switch to turn it off. > I am inclined to punt the keepalives_interval, keepalives_idle, and > keepalives_count parameters to 9.1. �If these are needed for > walreciever to work reliably, this whole approach is a dead-end, > because those parameters are not portable. �I will post a patch later > today along these lines. Do we know how unportable? If it still helps the majority, it might be worth doing. But I agree, if it's not really needed for walreceiver, then it should be punted to 9.1. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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: Robert Haas on 22 Jun 2010 12:16 On Tue, Jun 22, 2010 at 9:27 AM, Magnus Hagander <magnus(a)hagander.net> wrote: >> I am inclined to punt the keepalives_interval, keepalives_idle, and >> keepalives_count parameters to 9.1. �If these are needed for >> walreciever to work reliably, this whole approach is a dead-end, >> because those parameters are not portable. �I will post a patch later >> today along these lines. > > Do we know how unportable? If it still helps the majority, it might be > worth doing. But I agree, if it's not really needed for walreceiver, > then it should be punted to 9.1. This might not be such a good idea as I had thought. It looks like the default parameters on Linux (Fedora 12) are: tcp_keepalive_intvl:75 tcp_keepalive_probes:9 tcp_keepalive_time:7200 [ See also http://tldp.org/HOWTO/TCP-Keepalive-HOWTO/usingkeepalive.html ] That's clearly better than no keepalives, but I venture to say it's not going to be anything close to the behavior people want for walreceiver... I think we're going to need to either vastly reduce the keepalive time and interval, or abandon the strategy of using TCP keepalives completely. Which brings us to the question of portability. A quick search around the Internet suggests that this is supported on recent versions of Linux, Free/OpenBSD, AIX, and HP/UX, and it appears to work on my Mac also. I'm not clear how long it's been implemented on each of these platforms, though. With respect to Windows, it looks like there are registry settings for all of these parameters, but I'm unclear whether they can be set on a per-connection basis and what's required to make this happen. -- 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: Josh Berkus on 22 Jun 2010 14:02 All, If we *don't* rely on tcp-keepalive for terminating SR connections where the master is dead, what is the alternative? That issue, IMHO, is a blocker for 9.0. If tcp-keepalives are the only idea we have, then we need to work around the limitations and implement them. I'll also point out that keepalives are already a supported feature for production PostgreSQL on the server side, so I don't see that adding them for libpq is a big deal. We might not want to enable them by default, though. -- -- Josh Berkus PostgreSQL Experts Inc. http://www.pgexperts.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: Josh Berkus on 22 Jun 2010 14:24 > In what environment do you see that causing a problem (compared to > no keepalive)? If it were Alpha3 right now, I'd have no issue with it, and if we're talking about it for 9.1 I'd have no issue with it. I am, however, extremely reluctant to introduce a default behavior change for Beta3. -- -- Josh Berkus PostgreSQL Experts Inc. http://www.pgexperts.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
|
Next
|
Last
Pages: 1 2 3 4 Prev: what exactly is a PlaceHolderVar? Next: [HACKERS] Parallel pg_restore versus old dump files |