Keepalives win32
in [PgSql]
|
From: Bruce Momjian on 29 Jun 2010 20:09 Tom Lane wrote: > Robert Haas <robertmhaas(a)gmail.com> writes: > > On Mon, Jun 28, 2010 at 8:24 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > >> What I was trying to say is I think we could dispense with the > >> setsockopt() code path, and just always use the WSAIoctl() path anytime > >> keepalives are turned on. �I don't know what "system default values" > >> you're speaking of, if they're not the registry entries; and I > >> definitely don't see the point of consulting such values if they aren't > >> user-settable. �We might as well just consult the RFCs and be done. > > > FWIW, I think I prefer Magnus's approach, but I'm not 100% sure I can > > defend that preference... > > Well, basically what I don't like about Magnus' proposal is that setting > one of the two values changes the default that will be used for the > other one. (Or, if it does not change the default, the extra code is > useless anyway.) If we just always go through the WSAIoctl() path then > we can clearly document "the default for this on Windows is so-and-so". > How is the documentation going to explain the behavior of the proposed > code? Let's look at the usage probabilities. 99% of Win32 users will not use any of these settings. I would hate to come up with a solution that changes the default behavior for that 99%. Therefore, I think using hard-coded defaults only for cases where someone sets one but not all settings is appropriate. The documentation text would be: On Windows, if a keepalive settings is set, then defaults will be used for any unset values, specifically, keepalives_idle (200) and keepalives_interval(4). Windows does not allow control of keepalives_count. Seems simple enough. -- Bruce Momjian <bruce(a)momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + None of us is going to be here forever. + -- 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 30 Jun 2010 03:51 2010/6/30 Pavel Golub <pavel(a)microolap.com>: > Hello, Bruce. > > You wrote: > > BM> Tom Lane wrote: >>> Robert Haas <robertmhaas(a)gmail.com> writes: >>> > On Mon, Jun 28, 2010 at 8:24 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >>> >> What I was trying to say is I think we could dispense with the >>> >> setsockopt() code path, and just always use the WSAIoctl() path anytime >>> >> keepalives are turned on. �I don't know what "system default values" >>> >> you're speaking of, if they're not the registry entries; and I >>> >> definitely don't see the point of consulting such values if they aren't >>> >> user-settable. �We might as well just consult the RFCs and be done. >>> >>> > FWIW, I think I prefer Magnus's approach, but I'm not 100% sure I can >>> > defend that preference... >>> >>> Well, basically what I don't like about Magnus' proposal is that setting >>> one of the two values changes the default that will be used for the >>> other one. �(Or, if it does not change the default, the extra code is >>> useless anyway.) �If we just always go through the WSAIoctl() path then >>> we can clearly document "the default for this on Windows is so-and-so". >>> How is the documentation going to explain the behavior of the proposed >>> code? > > BM> Let's look at the usage probabilities. �99% of Win32 users will not use > BM> any of these settings. > > Let me disagree with this statement. As DAC developer I'm faced with > opposite reality. There are a lot of users demanding this > functionality. It's very intersting to hear from somebody who expects to actually use this. But are you aware that we're only talking about *adjusting* the keepalive values, not enabling them? Because we will, as the code stands now, enable keepalive by defaults - just use the system default values for timeout intervals. (Meaning this is how we do it on Unix as of HEAD, irregardless of my patch) Do you have an opinion on the two choices for handling keepalives_idle and keepalives_interval? They basically are: 1) When not configured, use system defaults. When only one of the two parameters configured, use RFC default for the other one (overwrite system default). 2) When not configured, use RFC defaults (overwrite system defaults). When only one of the two parameters configured, use RFC default for the other one (overwrite system default) 3) When not configured, use system defaults. When only one of the two parameters configured, throw error. I can see pros and cons with both. Given that I still think *most* people will not configure the intervals at all, I think #1 is the one that follows "principle of least surprise". Perhaps option *3* is the one that does this for partial configuration? -- 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: Pavel Golub on 30 Jun 2010 02:32 Hello, Bruce. You wrote: BM> Tom Lane wrote: >> Robert Haas <robertmhaas(a)gmail.com> writes: >> > On Mon, Jun 28, 2010 at 8:24 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >> >> What I was trying to say is I think we could dispense with the >> >> setsockopt() code path, and just always use the WSAIoctl() path anytime >> >> keepalives are turned on. �I don't know what "system default values" >> >> you're speaking of, if they're not the registry entries; and I >> >> definitely don't see the point of consulting such values if they aren't >> >> user-settable. �We might as well just consult the RFCs and be done. >> >> > FWIW, I think I prefer Magnus's approach, but I'm not 100% sure I can >> > defend that preference... >> >> Well, basically what I don't like about Magnus' proposal is that setting >> one of the two values changes the default that will be used for the >> other one. (Or, if it does not change the default, the extra code is >> useless anyway.) If we just always go through the WSAIoctl() path then >> we can clearly document "the default for this on Windows is so-and-so". >> How is the documentation going to explain the behavior of the proposed >> code? BM> Let's look at the usage probabilities. 99% of Win32 users will not use BM> any of these settings. Let me disagree with this statement. As DAC developer I'm faced with opposite reality. There are a lot of users demanding this functionality. BM> I would hate to come up with a solution that BM> changes the default behavior for that 99%. BM> Therefore, I think using hard-coded defaults only for cases where BM> someone sets one but not all settings is appropriate. The documentation BM> text would be: BM> On Windows, if a keepalive settings is set, then defaults will be BM> used for any unset values, specifically, keepalives_idle (200) and BM> keepalives_interval(4). Windows does not allow control of BM> keepalives_count. BM> Seems simple enough. BM> -- BM> Bruce Momjian <bruce(a)momjian.us> http://momjian.us BM> EnterpriseDB http://enterprisedb.com BM> + None of us is going to be here forever. + -- With best wishes, Pavel mailto:pavel(a)gf.microolap.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: Pavel Golub on 1 Jul 2010 01:59 Hello, Tom. You wrote: TL> Bruce Momjian <bruce(a)momjian.us> writes: >> Tom Lane wrote: >>> What's your idea of "affecting the fewest people"? There is no previous >>> history to be backward-compatible with, because we never supported >>> keepalive on Windows before. >> Well, starting in 9.0, keepalives in libpq will default to 'on': TL> Yes, which is already a change in behavior. I don't understand why you TL> are worrying about "backwards compatibility" to parameter values that TL> weren't in use before. I think self-consistency of the new version is TL> far more important than that. Absolutely agree. >> even if we use Windows defaults, those defaults might be different for >> different Windows versions. TL> I'm not sure if that's an issue or not, but if it is, that seems to me TL> to argue for #2 not #1. TL> regards, tom lane -- With best wishes, Pavel mailto:pavel(a)gf.microolap.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 7 Jul 2010 09:32
On Wed, Jul 7, 2010 at 9:20 AM, Magnus Hagander <magnus(a)hagander.net> wrote: > On Wed, Jun 30, 2010 at 17:46, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >> "Kevin Grittner" <Kevin.Grittner(a)wicourts.gov> writes: >>> I also think we may want to suggest that for most environments, >>> people may want to change these settings to something more >>> aggressive, like a 30 to 120 second initial delay, with a 10 or 20 >>> second retry interval. �The RFC defaults seem approximately right >>> for a TCP connection to a colony on the surface of the moon, where >>> besides the round trip latency of 2.5 seconds they might have to pay >>> by the byte. >> >> Well, the RFCs were definitely written at a time when bandwidth was a >> lot more expensive than it is today. >> >>> In other words, it is *so* conservative that I have >>> trouble seeing it ever causing a problem compared to not having >>> keepalive enabled, but it will eventually clean things up. >> >> Yes. �This is a large part of the reason why I think it's okay for us to >> turn libpq keepalive on by default in 9.0 --- the default parameters for >> it are so conservative as to be unlikely to cause trouble. �If Windows >> isn't using RFC-equivalent default parameters, that seems like a good >> reason to disregard the system settings and force use of the RFC values >> as defaults. > > Here's an updated version of the patch, which includes server side > functionality. I took out the code that tried to"be smart". It'll now > set them to 2 hours/1 second by default. I looked quickly at the RFC > and didn't find the exact values there, so those values are the > documented out-of-the-box defaults on Windows. I can easily change > them to RFC values if someone can find them for me :) > > It's also merged with roberts macos patch, since they were conflicting. > > Doc changes not included, but I'll get those in before commit. > > Comments? Looks generally OK, though my knowledge of Windows is pretty limited. We'd better get this committed PDQ if it's going into beta3, else there won't be a full buildfarm cycle before we wrap. (BTW, there are two buildfarm machines - wigeon and orangutan - that are consistently failing with rather bizarre error messages. Are these real failures or are those machines just messed up?) -- 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 |