Horton
Menu

Thread

  1. pgsql: Add support for TCP keepalives on Windows, both for backend and

    Magnus Hagander <mha@postgresql.org> — 2010-07-08T10:20:14Z

    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)

  2. Re: pgsql: Add support for TCP keepalives on Windows, both for backend and

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-07-08T15:04:15Z

    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

  3. Re: pgsql: Add support for TCP keepalives on Windows, both for backend and

    Magnus Hagander <magnus@hagander.net> — 2010-07-08T15:11:50Z

    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/

  4. Re: pgsql: Add support for TCP keepalives on Windows, both for backend and

    Magnus Hagander <magnus@hagander.net> — 2010-07-08T15:33:56Z

    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/

  5. Re: pgsql: Add support for TCP keepalives on Windows, both for backend and

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-07-08T15:39:14Z

    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

  6. Re: pgsql: Add support for TCP keepalives on Windows, both for backend and

    Magnus Hagander <magnus@hagander.net> — 2010-07-08T15:42:57Z

    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/

  7. Re: [HACKERS] pgsql: Add support for TCP keepalives on Windows, both for backend and

    Andrew Dunstan <andrew@dunslane.net> — 2010-07-08T15:45:41Z

    
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

  8. Re: [HACKERS] pgsql: Add support for TCP keepalives on Windows, both for backend and

    Magnus Hagander <magnus@hagander.net> — 2010-07-08T16:06:57Z

    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/

  9. Re: [HACKERS] pgsql: Add support for TCP keepalives on Windows, both for backend and

    Magnus Hagander <magnus@hagander.net> — 2010-07-08T16:20:17Z

    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/

  10. Re: [HACKERS] pgsql: Add support for TCP keepalives on Windows, both for backend and

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-07-08T16:37:43Z

    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

  11. Re: [HACKERS] pgsql: Add support for TCP keepalives on Windows, both for backend and

    Magnus Hagander <magnus@hagander.net> — 2010-07-08T16:42:18Z

    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/

  12. Re: [HACKERS] pgsql: Add support for TCP keepalives on Windows, both for backend and

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-07-08T16:46:42Z

    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

  13. Re: pgsql: Add support for TCP keepalives on Windows, both for backend and

    Bruce Momjian <bruce@momjian.us> — 2010-07-10T20:23:17Z

    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. +

  14. Re: [HACKERS] Re: pgsql: Add support for TCP keepalives on Windows, both for backend and

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-07-10T20:29:08Z

    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

  15. Re: [HACKERS] Re: pgsql: Add support for TCP keepalives on Windows, both for backend and

    Peter Eisentraut <peter_e@gmx.net> — 2010-07-10T22:05:10Z

    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.

  16. Re: [HACKERS] Re: pgsql: Add support for TCP keepalives on Windows, both for backend and

    Magnus Hagander <magnus@hagander.net> — 2010-07-12T10:05:10Z

    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/