Обсуждение: pgsql: Add support for TCP keepalives on Windows, both for backend and
pgsql: Add support for TCP keepalives on Windows, both for backend and
От
mha@postgresql.org (Magnus Hagander)
Дата:
Log Message: ----------- Add support for TCP keepalives on Windows, both for backend and the new libpq support. Modified Files: -------------- pgsql/src/backend/libpq: pqcomm.c (r1.210 -> r1.211) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/libpq/pqcomm.c?r1=1.210&r2=1.211) pgsql/src/interfaces/libpq: fe-connect.c (r1.396 -> r1.397) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-connect.c?r1=1.396&r2=1.397) pgsql/doc/src/sgml: config.sgml (r1.293 -> r1.294) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml/config.sgml?r1=1.293&r2=1.294) libpq.sgml (r1.312 -> r1.313) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml/libpq.sgml?r1=1.312&r2=1.313)
mha@postgresql.org (Magnus Hagander) writes: > Log Message: > ----------- > Add support for TCP keepalives on Windows, both for backend and the new > libpq support. Buildfarm member narwhal doesn't like this patch. You have about six or eight hours to fix or revert it before beta3 wraps. regards, tom lane
On Thu, Jul 8, 2010 at 17:04, Tom Lane <tgl@sss.pgh.pa.us> wrote: > mha@postgresql.org (Magnus Hagander) writes: >> Log Message: >> ----------- >> Add support for TCP keepalives on Windows, both for backend and the new >> libpq support. > > Buildfarm member narwhal doesn't like this patch. You have about six > or eight hours to fix or revert it before beta3 wraps. Gah. Seems mingw is out of date with reality again. I'll go look for a vm with it on and see if I can find how to do it there. (and yes, I even asked Dave to do a special run with that bf member for me, and then forgot to check the result. sorry!) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes: > Seems pretty simple - mingw doesn't have support for this. We have two > ways to deal with that I think: > 1) Disable it on mingw. > 2) Include it in our custom headers. > For #2, what we need to include is the define of SIO_KEEPALIVE_VALS as > well as the definition of struct tcp_keepalive. > We've done #2 before at least once, which worked well until mingw > suddenly caught up and added it a while later. It's not like this is a > new definition in windows, but we need to be ready for them to > eventually do that. Yeah. I'm satisfied with doing #1 and waiting for them to fix it. > I guess there is: > 3) write an autoconf test and provide them only when mingw doesn't have it. > if we're going with #3, I'll respectfully have to ask somebod yelse to > write the autoconf test, that's beyond me I think :-) An easy approximation would be to make the code #ifdef SIO_KEEPALIVE_VALS. That would fail if the mingw guys decide to provide the #define without adding the struct at the same time, but that seems moderately unlikely. regards, tom lane
On Thu, Jul 8, 2010 at 17:11, Magnus Hagander <magnus@hagander.net> wrote: > On Thu, Jul 8, 2010 at 17:04, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> mha@postgresql.org (Magnus Hagander) writes: >>> Log Message: >>> ----------- >>> Add support for TCP keepalives on Windows, both for backend and the new >>> libpq support. >> >> Buildfarm member narwhal doesn't like this patch. You have about six >> or eight hours to fix or revert it before beta3 wraps. > > Gah. Seems mingw is out of date with reality again. I'll go look for a > vm with it on and see if I can find how to do it there. > > (and yes, I even asked Dave to do a special run with that bf member > for me, and then forgot to check the result. sorry!) Seems pretty simple - mingw doesn't have support for this. We have two ways to deal with that I think: 1) Disable it on mingw. 2) Include it in our custom headers. For #2, what we need to include is the define of SIO_KEEPALIVE_VALS as well as the definition of struct tcp_keepalive. We've done #2 before at least once, which worked well until mingw suddenly caught up and added it a while later. It's not like this is a new definition in windows, but we need to be ready for them to eventually do that. I guess there is: 3) write an autoconf test and provide them only when mingw doesn't have it. if we're going with #3, I'll respectfully have to ask somebod yelse to write the autoconf test, that's beyond me I think :-) Opinions on which way to go? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
On Thu, Jul 8, 2010 at 17:39, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >> Seems pretty simple - mingw doesn't have support for this. We have two >> ways to deal with that I think: >> 1) Disable it on mingw. >> 2) Include it in our custom headers. > >> For #2, what we need to include is the define of SIO_KEEPALIVE_VALS as >> well as the definition of struct tcp_keepalive. > >> We've done #2 before at least once, which worked well until mingw >> suddenly caught up and added it a while later. It's not like this is a >> new definition in windows, but we need to be ready for them to >> eventually do that. > > Yeah. I'm satisfied with doing #1 and waiting for them to fix it. > >> I guess there is: >> 3) write an autoconf test and provide them only when mingw doesn't have it. >> if we're going with #3, I'll respectfully have to ask somebod yelse to >> write the autoconf test, that's beyond me I think :-) > > An easy approximation would be to make the code #ifdef SIO_KEEPALIVE_VALS. > That would fail if the mingw guys decide to provide the #define without > adding the struct at the same time, but that seems moderately unlikely. Seems reasonable. I'll go do something along that line and verify that it actually works :-) That laves the questions of docs - right now the docs just say it works on windows. I guess we need to add some kind of disclaimer around that, but the fact is that for 99+% of our windows users it will work - since they use the binaries, and the binaries are built with the full api - so we shouldn't make it *too* prominent.. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] pgsql: Add support for TCP keepalives on Windows, both for backend and
От
Andrew Dunstan
Дата:
Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: > >> Seems pretty simple - mingw doesn't have support for this. We have two >> ways to deal with that I think: >> 1) Disable it on mingw. >> 2) Include it in our custom headers. >> > > >> For #2, what we need to include is the define of SIO_KEEPALIVE_VALS as >> well as the definition of struct tcp_keepalive. >> > > >> We've done #2 before at least once, which worked well until mingw >> suddenly caught up and added it a while later. It's not like this is a >> new definition in windows, but we need to be ready for them to >> eventually do that. >> > > Yeah. I'm satisfied with doing #1 and waiting for them to fix it. > > >> I guess there is: >> 3) write an autoconf test and provide them only when mingw doesn't have it. >> if we're going with #3, I'll respectfully have to ask somebod yelse to >> write the autoconf test, that's beyond me I think :-) >> > > An easy approximation would be to make the code #ifdef SIO_KEEPALIVE_VALS. > That would fail if the mingw guys decide to provide the #define without > adding the struct at the same time, but that seems moderately unlikely. > > > +1 for this course of action. cheers andrew
Re: [HACKERS] pgsql: Add support for TCP keepalives on Windows, both for backend and
От
Magnus Hagander
Дата:
On Thu, Jul 8, 2010 at 17:45, Andrew Dunstan <andrew@dunslane.net> wrote: > > > Tom Lane wrote: >> >> Magnus Hagander <magnus@hagander.net> writes: >> >>> >>> Seems pretty simple - mingw doesn't have support for this. We have two >>> ways to deal with that I think: >>> 1) Disable it on mingw. >>> 2) Include it in our custom headers. >>> >> >> >>> >>> For #2, what we need to include is the define of SIO_KEEPALIVE_VALS as >>> well as the definition of struct tcp_keepalive. >>> >> >> >>> >>> We've done #2 before at least once, which worked well until mingw >>> suddenly caught up and added it a while later. It's not like this is a >>> new definition in windows, but we need to be ready for them to >>> eventually do that. >>> >> >> Yeah. I'm satisfied with doing #1 and waiting for them to fix it. >> >> >>> >>> I guess there is: >>> 3) write an autoconf test and provide them only when mingw doesn't have >>> it. >>> if we're going with #3, I'll respectfully have to ask somebod yelse to >>> write the autoconf test, that's beyond me I think :-) >>> >> >> An easy approximation would be to make the code #ifdef SIO_KEEPALIVE_VALS. >> That would fail if the mingw guys decide to provide the #define without >> adding the struct at the same time, but that seems moderately unlikely. >> >> >> > > +1 for this course of action. Here's what I came up with and will apply as soon as my msvc build completes. (the mingw one works with this) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] pgsql: Add support for TCP keepalives on Windows, both for backend and
От
Magnus Hagander
Дата:
On Thu, Jul 8, 2010 at 18:06, Magnus Hagander <magnus@hagander.net> wrote: > On Thu, Jul 8, 2010 at 17:45, Andrew Dunstan <andrew@dunslane.net> wrote: >> >> >> Tom Lane wrote: >>> >>> Magnus Hagander <magnus@hagander.net> writes: >>> >>>> >>>> Seems pretty simple - mingw doesn't have support for this. We have two >>>> ways to deal with that I think: >>>> 1) Disable it on mingw. >>>> 2) Include it in our custom headers. >>>> >>> >>> >>>> >>>> For #2, what we need to include is the define of SIO_KEEPALIVE_VALS as >>>> well as the definition of struct tcp_keepalive. >>>> >>> >>> >>>> >>>> We've done #2 before at least once, which worked well until mingw >>>> suddenly caught up and added it a while later. It's not like this is a >>>> new definition in windows, but we need to be ready for them to >>>> eventually do that. >>>> >>> >>> Yeah. I'm satisfied with doing #1 and waiting for them to fix it. >>> >>> >>>> >>>> I guess there is: >>>> 3) write an autoconf test and provide them only when mingw doesn't have >>>> it. >>>> if we're going with #3, I'll respectfully have to ask somebod yelse to >>>> write the autoconf test, that's beyond me I think :-) >>>> >>> >>> An easy approximation would be to make the code #ifdef SIO_KEEPALIVE_VALS. >>> That would fail if the mingw guys decide to provide the #define without >>> adding the struct at the same time, but that seems moderately unlikely. >>> >>> >>> >> >> +1 for this course of action. > > Here's what I came up with and will apply as soon as my msvc build > completes. (the mingw one works with this) ... and applied. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes: > Here's what I came up with and will apply as soon as my msvc build > completes. (the mingw one works with this) This is still going to need manual adjustment in the future, since probably when mingw adds the #define, they will put it in <mstcpip.h>, and this code will not see it. So at some point we'll need to add an autoconf test for <mstcpip.h> and replace those #ifdef WIN32_ONLY_COMPILER checks with #ifdef HAVE_MSTCPIP_H. But I see no need to bother until such a version of mingw exists, and I'd just as soon not be making such changes on the day of a wrap. So this is just a note for later. regards, tom lane
Re: [HACKERS] pgsql: Add support for TCP keepalives on Windows, both for backend and
От
Magnus Hagander
Дата:
On Thu, Jul 8, 2010 at 18:37, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >> Here's what I came up with and will apply as soon as my msvc build >> completes. (the mingw one works with this) > > This is still going to need manual adjustment in the future, since > probably when mingw adds the #define, they will put it in <mstcpip.h>, > and this code will not see it. So at some point we'll need to add an > autoconf test for <mstcpip.h> and replace those #ifdef > WIN32_ONLY_COMPILER checks with #ifdef HAVE_MSTCPIP_H. But I see no > need to bother until such a version of mingw exists, and I'd just as > soon not be making such changes on the day of a wrap. So this is just > a note for later. Agreed. And it's far from certain they'll actually add it to mstcpip.h - sometimes they stick it in a different header... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes: > On Thu, Jul 8, 2010 at 18:37, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> This is still going to need manual adjustment in the future, > And it's far from certain they'll actually add it to mstcpip.h - > sometimes they stick it in a different header... Yeah, that's the other reason for waiting for them to fix it before we invest more effort on our end. regards, tom lane
Magnus Hagander wrote: > > An easy approximation would be to make the code #ifdef SIO_KEEPALIVE_VALS. > > That would fail if the mingw guys decide to provide the #define without > > adding the struct at the same time, but that seems moderately unlikely. > > Seems reasonable. I'll go do something along that line and verify that > it actually works :-) > > That laves the questions of docs - right now the docs just say it > works on windows. I guess we need to add some kind of disclaimer > around that, but the fact is that for 99+% of our windows users it > will work - since they use the binaries, and the binaries are built > with the full api - so we shouldn't make it *too* prominent.. Wow, how would they know if the binaries are MinGW compiled? Does it show in version()? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + None of us is going to be here forever. +
Bruce Momjian <bruce@momjian.us> writes: > Magnus Hagander wrote: >> That laves the questions of docs - right now the docs just say it >> works on windows. I guess we need to add some kind of disclaimer >> around that, but the fact is that for 99+% of our windows users it >> will work - since they use the binaries, and the binaries are built >> with the full api - so we shouldn't make it *too* prominent.. > Wow, how would they know if the binaries are MinGW compiled? Does it > show in version()? AFAIK there is nobody distributing mingw-built binaries. If somebody has one, they'd have built it themselves, and they'd know. regards, tom lane
Re: [HACKERS] Re: pgsql: Add support for TCP keepalives on Windows, both for backend and
От
Peter Eisentraut
Дата:
On lör, 2010-07-10 at 16:23 -0400, Bruce Momjian wrote: > Wow, how would they know if the binaries are MinGW compiled? Does it > show in version()? Yes, I think so.
Re: [HACKERS] Re: pgsql: Add support for TCP keepalives on Windows, both for backend and
От
Magnus Hagander
Дата:
On Sun, Jul 11, 2010 at 00:05, Peter Eisentraut <peter_e@gmx.net> wrote: > On lör, 2010-07-10 at 16:23 -0400, Bruce Momjian wrote: >> Wow, how would they know if the binaries are MinGW compiled? Does it >> show in version()? > > Yes, I think so. It definitely does. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/