pgsql: Add support for TCP keepalives on Windows, both for backend and
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/
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/
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: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/
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
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/
Attachments:
mingwkeepalives.patchapplication/octet-stream; name=mingwkeepalives.patchDownload
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 987a4a3..5757832 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -83,7 +83,7 @@
#ifdef HAVE_UTIME_H
#include <utime.h>
#endif
-#ifdef WIN32
+#ifdef WIN32_ONLY_COMPILER /* mstcpip.h is missing on mingw */
#include <mstcpip.h>
#endif
@@ -1323,7 +1323,7 @@ pq_endcopyout(bool errorAbort)
* actually set them to zero, not default), therefor we fallback to
* the out-of-the-box default instead.
*/
-#ifdef WIN32
+#if defined(WIN32) && defined(SIO_KEEPALIVE_VALS)
static int
pq_setkeepaliveswin32(Port *port, int idle, int interval)
{
@@ -1412,7 +1412,7 @@ pq_setkeepalivesidle(int idle, Port *port)
if (port == NULL || IS_AF_UNIX(port->laddr.addr.ss_family))
return STATUS_OK;
-#if defined(TCP_KEEPIDLE) || defined(TCP_KEEPALIVE) || defined(WIN32)
+#if defined(TCP_KEEPIDLE) || defined(TCP_KEEPALIVE) || defined(SIO_KEEPALIVE_VALS)
if (idle == port->keepalives_idle)
return STATUS_OK;
@@ -1451,7 +1451,7 @@ pq_setkeepalivesidle(int idle, Port *port)
#else /* WIN32 */
return pq_setkeepaliveswin32(port, idle, port->keepalives_interval);
#endif
-#else /* TCP_KEEPIDLE || WIN32 */
+#else /* TCP_KEEPIDLE || SIO_KEEPALIVE_VALS */
if (idle != 0)
{
elog(LOG, "setting the keepalive idle time is not supported");
@@ -1464,7 +1464,7 @@ pq_setkeepalivesidle(int idle, Port *port)
int
pq_getkeepalivesinterval(Port *port)
{
-#if defined(TCP_KEEPINTVL) || defined(WIN32)
+#if defined(TCP_KEEPINTVL) || defined(SIO_KEEPALIVE_VALS)
if (port == NULL || IS_AF_UNIX(port->laddr.addr.ss_family))
return 0;
@@ -1501,7 +1501,7 @@ pq_setkeepalivesinterval(int interval, Port *port)
if (port == NULL || IS_AF_UNIX(port->laddr.addr.ss_family))
return STATUS_OK;
-#if defined(TCP_KEEPINTVL) || defined (WIN32)
+#if defined(TCP_KEEPINTVL) || defined (SIO_KEEPALIVE_VALS)
if (interval == port->keepalives_interval)
return STATUS_OK;
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 0bd0a95..dc7c57a 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -38,7 +38,9 @@
#endif
#define near
#include <shlobj.h>
+#ifdef WIN32_ONLY_COMPILER /* mstcpip.h is missing on mingw */
#include <mstcpip.h>
+#endif
#else
#include <sys/socket.h>
#include <netdb.h>
@@ -1093,6 +1095,7 @@ setKeepalivesCount(PGconn *conn)
}
#else /* Win32 */
+#ifdef SIO_KEEPALIVE_VALS
/*
* Enable keepalives and set the keepalive values on Win32,
* where they are always set in one batch.
@@ -1137,6 +1140,7 @@ setKeepalivesWin32(PGconn *conn)
}
return 1;
}
+#endif /* SIO_KEEPALIVE_VALS */
#endif /* WIN32 */
/* ----------
@@ -1555,8 +1559,10 @@ keep_going: /* We will come back to here until there is
|| !setKeepalivesCount(conn))
err = 1;
#else /* WIN32 */
+#ifdef SIO_KEEPALIVE_VALS
else if (!setKeepalivesWin32(conn))
err = 1;
+#endif /* SIO_KEEPALIVE_VALS */
#endif /* WIN32 */
if (err)
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
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
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.
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/