Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Oliver Jowett]

Tom Lane wrote:
 So the postmaster-log message may be the best we can do ...
 but I don't think we should drop the connection.

Here's a patch to do that; it appears to work as intended on my Linux
system.

-O
Index: src/backend/libpq/pqcomm.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/libpq/pqcomm.c,v
retrieving revision 1.178
diff -c -r1.178 pqcomm.c
*** src/backend/libpq/pqcomm.c  30 Jul 2005 20:28:20 -  1.178
--- src/backend/libpq/pqcomm.c  12 Sep 2005 00:58:15 -
***
*** 595,612 
return STATUS_ERROR;
}
  
!   /* Set default keepalive parameters. This should also catch
!* misconfigurations (non-zero values when socket options aren't
!* supported)
 */
!   if (pq_setkeepalivesidle(tcp_keepalives_idle, port) != 
STATUS_OK)
!   return STATUS_ERROR;
! 
!   if (pq_setkeepalivesinterval(tcp_keepalives_interval, port) != 
STATUS_OK)
!   return STATUS_ERROR;
! 
!   if (pq_setkeepalivescount(tcp_keepalives_count, port) != 
STATUS_OK)
!   return STATUS_ERROR;
}
  
return STATUS_OK;
--- 595,607 
return STATUS_ERROR;
}
  
!   /* Set default keepalive parameters. We ignore misconfigurations
!* that cause errors -- they will be logged, but won't kill the
!* connection.
 */
!   pq_setkeepalivesidle(tcp_keepalives_idle, port);
!   pq_setkeepalivesinterval(tcp_keepalives_interval, port);
!   pq_setkeepalivescount(tcp_keepalives_count, port);
}
  
return STATUS_OK;

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Merlin Moncure]

Tom Lane wrote:
  If the connection gets accepted, I'd expect *something* in the
  postmaster logs -- can you check?
 
 I suspect Merlin's complaint has to do with the fact that the *user*
 doesn't see any error message.  The way you've coded this, setsockopt
 failure during startup is treated as a communications failure and so
 there's no attempt to report the problem to the client.

Correct.  In fact, when I posted I did completely miss the log message
due to initdb resetting log_destination.  Confirmed:
LOG:  setsockopt(TCP_KEEPIDLE) not supported
in server log.

Merlin



---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL (was Re: [HACKERS] Feature freeze date for 8.1)

[Merlin Moncure]

 Here's a patch that adds four new GUCs:
 
   tcp_keepalives (defaults to on, controls SO_KEEPALIVE)
   tcp_keepalives_idle (controls TCP_KEEPIDLE)
   tcp_keepalives_interval (controls TCP_KEEPINTVL)
   tcp_keepalives_count (controls TCP_KEEPCNT)

I just tested this on my windows XP machine running rc1.  A default
configuration reports zeros for the tcp values in a 'show all'.  More
significantly, if you change a tcp parameter from the default, the
server rejects connections without a relevant error message :(.

I did some research and the only way to control these parameters is to
adjust the system registry plus a reboot. (somebody correct me here).
If that is the case IMO it makes the most sense to have the server fail
to start if the default parameters are changed.

Even better would be a stronger test to make sure o/s supports this
feature.

Merlin

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Oliver Jowett]

Merlin Moncure wrote:
Here's a patch that adds four new GUCs:

  tcp_keepalives (defaults to on, controls SO_KEEPALIVE)
  tcp_keepalives_idle (controls TCP_KEEPIDLE)
  tcp_keepalives_interval (controls TCP_KEEPINTVL)
  tcp_keepalives_count (controls TCP_KEEPCNT)
 
 
 I just tested this on my windows XP machine running rc1.  A default
 configuration reports zeros for the tcp values in a 'show all'.

That usually means that there is no OS-level support for the TCP_*
values. Does Windows actually provide those #defines?

 More
 significantly, if you change a tcp parameter from the default, the
 server rejects connections without a relevant error message :(.

Could you clarify what you mean by rejects? Does it accept them and
then close the connection, or does it fail to even accept the TCP
connection?

If the connection gets accepted, I'd expect *something* in the
postmaster logs -- can you check?

 I did some research and the only way to control these parameters is to
 adjust the system registry plus a reboot. (somebody correct me here).
 If that is the case IMO it makes the most sense to have the server fail
 to start if the default parameters are changed.

The problem is that the GUC only makes sense when you have an actual TCP
connection present, so it is only set while establishing a new
connection. If setting the value fails (e.g. the OS rejects the value),
then it's just like any other GUC setup failure during backend startup,
which by the sounds of it makes the whole connection fail. I don't know
if we should ignore failures to set a GUC and continue anyway .. that
sounds dangerous.

I'd expect unix-domain-socket connections to continue to work fine in
the face of a misconfiguration currently.

We could check the non-zero configuration on an OS that has no support
case at postmaster boot time, I suppose, but that doesn't help with the
case where there is support but the OS rejects the particular values
you're trying to set.

 Even better would be a stronger test to make sure o/s supports this
 feature.

Well, the code assumes that if the TCP_* constants are present then they
can be used. It seems a bit stupid if Windows defines them but doesn't
support them at all.

-O

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL (was Re: [HACKERS] Feature freeze date for 8.1)

[Tom Lane]

Oliver Jowett [EMAIL PROTECTED] writes:
 Merlin Moncure wrote:
 Even better would be a stronger test to make sure o/s supports this
 feature.

 Well, the code assumes that if the TCP_* constants are present then they
 can be used. It seems a bit stupid if Windows defines them but doesn't
 support them at all.

Mmmm ... we learned the hard way that header files, userland libraries,
and kernel behavior aren't necessarily synchronized.  Certainly you
can't build the code if the header files don't define the TCP_ symbols
for you, but it's a serious mistake to assume that the kernel will take
the values just because there's a header that defines them.  See the
archives from back when we were trying to get the IPv6 code to be
portable.

I'd actually expect these things to be in closer sync on a Windows
machine than on the average Linux machine.  The problem with Windows is
that we're trying to support building an executable on one flavor of
Windows and then running it on other flavors --- so again, what you
saw in the headers need not match what the kernel will do.

In short, if you were assuming that then you'd better fix the code.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Oliver Jowett]

Tom Lane wrote:
 Oliver Jowett [EMAIL PROTECTED] writes:
 
Merlin Moncure wrote:

Even better would be a stronger test to make sure o/s supports this
feature.
 
 
Well, the code assumes that if the TCP_* constants are present then they
can be used. It seems a bit stupid if Windows defines them but doesn't
support them at all.

 In short, if you were assuming that then you'd better fix the code.

Sorry, to clarify: If the TCP_* constants are provided, *and* you
configure the backend to try to use non-default values, then the code
will try the appropriate setsockopt(). If that fails, then the
connection gets dropped.

If you don't change the defaults, it doesn't use setsockopt() at all.

The assumption I'm making is that if the TCP_* values are present at
compile time, then you can make a setsockopt() call and get a sane error
code back if there's no support -- rather than a segfault, or having the
OS spontaneously do weird things to the connection, or anything like
that. Is that a reasonable thing to assume?

...

There doesn't seem any clean way to test if a particular set of values
specified at runtime is going to work or not -- the only way is to try.
I suppose we could set up a dummy TCP connection on postmaster start and
try that, but..

We could potentially do better in the no TCP_* support case, detecting
it on postmaster startup (move the check for NULL port down into the
pqcomm code, and complain about non-zero values even in that case if
there's no support); but that doesn't help the other cases.

If I specify out-of-range values on my Linux box, I get this:

 [EMAIL PROTECTED] ~ $ pg/8.1-beta1/bin/psql -h localhost template1
 psql: server closed the connection unexpectedly
 This probably means the server terminated abnormally
 before or while processing the request.

and in the logs:

 LOG:  setsockopt(TCP_KEEPIDLE) failed: Invalid argument

I'd expect to see something similar in the TCP_* present but no real
support case.

-O

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL (was Re: [HACKERS] Feature freeze date for 8.1)

[Tom Lane]

Oliver Jowett [EMAIL PROTECTED] writes:
 The assumption I'm making is that if the TCP_* values are present at
 compile time, then you can make a setsockopt() call and get a sane error
 code back if there's no support -- rather than a segfault, or having the
 OS spontaneously do weird things to the connection, or anything like
 that. Is that a reasonable thing to assume?

Well, on a sane OS it's reasonable.  I dunno about Windows ;-)

One question to ask is whether we should treat the setsockopt failure
as fatal or not.  It seems to me that aborting the connection could
reasonably be called an overreaction to a bad parameter setting;
couldn't we just set the GUC variable to zero and keep going?

regards, tom lane

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Tom Lane]

Oliver Jowett [EMAIL PROTECTED] writes:
 Merlin Moncure wrote:
 More
 significantly, if you change a tcp parameter from the default, the
 server rejects connections without a relevant error message :(.

 Could you clarify what you mean by rejects? Does it accept them and
 then close the connection, or does it fail to even accept the TCP
 connection?

 If the connection gets accepted, I'd expect *something* in the
 postmaster logs -- can you check?

I suspect Merlin's complaint has to do with the fact that the *user*
doesn't see any error message.  The way you've coded this, setsockopt
failure during startup is treated as a communications failure and so
there's no attempt to report the problem to the client.

regards, tom lane

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Oliver Jowett]

Tom Lane wrote:
 Oliver Jowett [EMAIL PROTECTED] writes:
 
The assumption I'm making is that if the TCP_* values are present at
compile time, then you can make a setsockopt() call and get a sane error
code back if there's no support -- rather than a segfault, or having the
OS spontaneously do weird things to the connection, or anything like
that. Is that a reasonable thing to assume?
 
 
 Well, on a sane OS it's reasonable.  I dunno about Windows ;-)
 
 One question to ask is whether we should treat the setsockopt failure
 as fatal or not.  It seems to me that aborting the connection could
 reasonably be called an overreaction to a bad parameter setting;
 couldn't we just set the GUC variable to zero and keep going?

There's no real reason why not; currently the code looks like this:

   /* Set default keepalive parameters. This should also catch
* misconfigurations (non-zero values when socket options aren't
* supported)
*/
   if (pq_setkeepalivesidle(tcp_keepalives_idle, port) != STATUS_OK)
 return STATUS_ERROR;
 
   if (pq_setkeepalivesinterval(tcp_keepalives_interval, port) != STATUS_OK)
 return STATUS_ERROR;
 
   if (pq_setkeepalivescount(tcp_keepalives_count, port) != STATUS_OK)
 return STATUS_ERROR;

We could just log (already done inside pq_*, IIRC) and continue, instead
of erroring out. It's just the way it is because I personally prefer
misconfigurations to break loudly, so you have to fix them ;-)

-O

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL (was Re: [HACKERS] Feature freeze date for 8.1)

[Tom Lane]

Oliver Jowett [EMAIL PROTECTED] writes:
 We could just log (already done inside pq_*, IIRC) and continue, instead
 of erroring out. It's just the way it is because I personally prefer
 misconfigurations to break loudly, so you have to fix them ;-)

Well, dropping the connection with no message (except in the postmaster
log, which is equivalent to /dev/null for way too many people) isn't
my idea of how to complain loudly.  It just makes the software look
broken.

I'm not sure if we can issue a notice that will be seen on the client
side at this point in the startup cycle.  I seem to recall the protocol
document advising against sending NOTICEs during the authentication
cycle.  So the postmaster-log message may be the best we can do ...
but I don't think we should drop the connection.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Oliver Jowett]

Tom Lane wrote:

 I'm not sure if we can issue a notice that will be seen on the client
 side at this point in the startup cycle.  I seem to recall the protocol
 document advising against sending NOTICEs during the authentication
 cycle.

As currently written the setsockopt() calls are done very early, well
before even ProcessStartupPacket() has run -- so we can't really send
anything at all to the client because we don't even know which protocol
to use.

Delaying the setsockopt() introduces the danger that you could lose the
network at just the wrong time and have a backend sitting around for an
extended period waiting on a startup packet (but not holding locks,
admittedly).

-O

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Bruce Momjian]

Is this the functionality we agreed we wanted?

---

Oliver Jowett wrote:
 Bruce Momjian wrote:
  Is this patch being worked on?
 
 Here's an updated version. It compiles and appears to work as expected
 under Linux (supports TCP_KEEPIDLE etc) and Solaris 9 (no support).
 
 Main changes:
 
 - removed the tcp_keepalives GUC, SO_KEEPALIVE is now always on (as in
 current CVS)
 - {get,set}sockopt calls are only done when absolutely necessary (no
 extra syscalls during backend startup in a default configuration).
 
 I still haven't had a chance to glue in support for the TCP_KEEPALIVE
 (Solaris-style) option, but that should be fairly painless to add later.
 
 -O

 ? postgresql-8.1devel.tar.gz
 Index: doc/src/sgml/runtime.sgml
 ===
 RCS file: /projects/cvsroot/pgsql/doc/src/sgml/runtime.sgml,v
 retrieving revision 1.335
 diff -u -c -r1.335 runtime.sgml
 *** doc/src/sgml/runtime.sgml 2 Jul 2005 19:16:36 -   1.335
 --- doc/src/sgml/runtime.sgml 4 Jul 2005 10:41:33 -
 ***
 *** 894,899 
 --- 894,946 
 /listitem
/varlistentry

 +  varlistentry id=guc-tcp-keepalives-idle 
 xreflabel=tcp_keepalives_idle
 +   termvarnametcp_keepalives_idle/varname 
 (typeinteger/type)/term
 +   indexterm
 +primaryvarnametcp_keepalives_idle/ configuration 
 parameter/primary
 +   /indexterm
 +   listitem
 +para
 + On systems that support the TCP_KEEPIDLE socket option, specifies 
 the
 + number of seconds between sending keepalives on an otherwise idle
 + connection. A value of 0 uses the system default. If TCP_KEEPIDLE is
 + not supported, this parameter must be 0. This option is ignored for
 + connections made via a Unix-domain socket.
 +/para
 +   /listitem
 +  /varlistentry
 +  
 +  varlistentry id=guc-tcp-keepalives-interval 
 xreflabel=tcp_keepalives_interval
 +   termvarnametcp_keepalives_interval/varname 
 (typeinteger/type)/term
 +   indexterm
 +primaryvarnametcp_keepalives_interval/ configuration 
 parameter/primary
 +   /indexterm
 +   listitem
 +para
 + On systems that support the TCP_KEEPINTVL socket option, specifies 
 how
 + long, in seconds, to wait for a response to a keepalive before
 + retransmitting. A value of 0 uses the system default. If 
 TCP_KEEPINTVL
 + is not supported, this parameter must be 0. This option is ignored
 + for connections made via a Unix-domain socket.
 +/para
 +   /listitem
 +  /varlistentry
 +  
 +  varlistentry id=guc-tcp-keepalives-count 
 xreflabel=tcp_keepalives_count
 +   termvarnametcp_keepalives_count/varname 
 (typeinteger/type)/term
 +   indexterm
 +primaryvarnametcp_keepalives_count/ configuration 
 parameter/primary
 +   /indexterm
 +   listitem
 +para
 + On systems that support the TCP_KEEPCNT socket option, specifies how
 + many keepalives may be lost before the connection is considered 
 dead. 
 + A value of 0 uses the system default. If TCP_KEEPINTVL is not
 + supported, this parameter must be 0.
 +/para
 +   /listitem
 +  /varlistentry
 +  
/variablelist
/sect3
sect3 id=runtime-config-connection-security
 Index: src/backend/libpq/pqcomm.c
 ===
 RCS file: /projects/cvsroot/pgsql/src/backend/libpq/pqcomm.c,v
 retrieving revision 1.176
 diff -u -c -r1.176 pqcomm.c
 *** src/backend/libpq/pqcomm.c22 Feb 2005 04:35:57 -  1.176
 --- src/backend/libpq/pqcomm.c4 Jul 2005 10:41:33 -
 ***
 *** 87,93 
   #include libpq/libpq.h
   #include miscadmin.h
   #include storage/ipc.h
 ! 
   
   /*
* Configuration options
 --- 87,93 
   #include libpq/libpq.h
   #include miscadmin.h
   #include storage/ipc.h
 ! #include utils/guc.h
   
   /*
* Configuration options
 ***
 *** 594,599 
 --- 594,612 
   elog(LOG, setsockopt(SO_KEEPALIVE) failed: %m);
   return STATUS_ERROR;
   }
 + 
 + /* Set default keepalive parameters. This should also catch
 +  * misconfigurations (non-zero values when socket options aren't
 +  * supported)
 +  */
 + if (pq_setkeepalivesidle(tcp_keepalives_idle, port) != 
 STATUS_OK)
 + return STATUS_ERROR;
 + 
 + if (pq_setkeepalivesinterval(tcp_keepalives_interval, port) != 
 STATUS_OK)
 + return STATUS_ERROR;
 + 
 + if (pq_setkeepalivescount(tcp_keepalives_count, port) != 
 STATUS_OK)
 + return STATUS_ERROR;
   }
   
   return STATUS_OK;
 

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Oliver Jowett]

Bruce Momjian wrote:
 Is this the functionality we agreed we wanted?

I think it covers Tom's comments on the first version:

 - a GUC to turn off SO_KEEPALIVE isn't particularly useful
 - don't do redundant get/setsockopt calls on backend startup

-O

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Oliver Jowett]

Bruce Momjian wrote:
 Is this patch being worked on?

Here's an updated version. It compiles and appears to work as expected
under Linux (supports TCP_KEEPIDLE etc) and Solaris 9 (no support).

Main changes:

- removed the tcp_keepalives GUC, SO_KEEPALIVE is now always on (as in
current CVS)
- {get,set}sockopt calls are only done when absolutely necessary (no
extra syscalls during backend startup in a default configuration).

I still haven't had a chance to glue in support for the TCP_KEEPALIVE
(Solaris-style) option, but that should be fairly painless to add later.

-O
? postgresql-8.1devel.tar.gz
Index: doc/src/sgml/runtime.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/runtime.sgml,v
retrieving revision 1.335
diff -u -c -r1.335 runtime.sgml
*** doc/src/sgml/runtime.sgml   2 Jul 2005 19:16:36 -   1.335
--- doc/src/sgml/runtime.sgml   4 Jul 2005 10:41:33 -
***
*** 894,899 
--- 894,946 
/listitem
   /varlistentry
   
+  varlistentry id=guc-tcp-keepalives-idle 
xreflabel=tcp_keepalives_idle
+   termvarnametcp_keepalives_idle/varname 
(typeinteger/type)/term
+   indexterm
+primaryvarnametcp_keepalives_idle/ configuration 
parameter/primary
+   /indexterm
+   listitem
+para
+ On systems that support the TCP_KEEPIDLE socket option, specifies the
+ number of seconds between sending keepalives on an otherwise idle
+ connection. A value of 0 uses the system default. If TCP_KEEPIDLE is
+ not supported, this parameter must be 0. This option is ignored for
+ connections made via a Unix-domain socket.
+/para
+   /listitem
+  /varlistentry
+  
+  varlistentry id=guc-tcp-keepalives-interval 
xreflabel=tcp_keepalives_interval
+   termvarnametcp_keepalives_interval/varname 
(typeinteger/type)/term
+   indexterm
+primaryvarnametcp_keepalives_interval/ configuration 
parameter/primary
+   /indexterm
+   listitem
+para
+ On systems that support the TCP_KEEPINTVL socket option, specifies how
+ long, in seconds, to wait for a response to a keepalive before
+ retransmitting. A value of 0 uses the system default. If TCP_KEEPINTVL
+ is not supported, this parameter must be 0. This option is ignored
+ for connections made via a Unix-domain socket.
+/para
+   /listitem
+  /varlistentry
+  
+  varlistentry id=guc-tcp-keepalives-count 
xreflabel=tcp_keepalives_count
+   termvarnametcp_keepalives_count/varname 
(typeinteger/type)/term
+   indexterm
+primaryvarnametcp_keepalives_count/ configuration 
parameter/primary
+   /indexterm
+   listitem
+para
+ On systems that support the TCP_KEEPCNT socket option, specifies how
+ many keepalives may be lost before the connection is considered dead. 
+ A value of 0 uses the system default. If TCP_KEEPINTVL is not
+ supported, this parameter must be 0.
+/para
+   /listitem
+  /varlistentry
+  
   /variablelist
   /sect3
   sect3 id=runtime-config-connection-security
Index: src/backend/libpq/pqcomm.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/libpq/pqcomm.c,v
retrieving revision 1.176
diff -u -c -r1.176 pqcomm.c
*** src/backend/libpq/pqcomm.c  22 Feb 2005 04:35:57 -  1.176
--- src/backend/libpq/pqcomm.c  4 Jul 2005 10:41:33 -
***
*** 87,93 
  #include libpq/libpq.h
  #include miscadmin.h
  #include storage/ipc.h
! 
  
  /*
   * Configuration options
--- 87,93 
  #include libpq/libpq.h
  #include miscadmin.h
  #include storage/ipc.h
! #include utils/guc.h
  
  /*
   * Configuration options
***
*** 594,599 
--- 594,612 
elog(LOG, setsockopt(SO_KEEPALIVE) failed: %m);
return STATUS_ERROR;
}
+ 
+   /* Set default keepalive parameters. This should also catch
+* misconfigurations (non-zero values when socket options aren't
+* supported)
+*/
+   if (pq_setkeepalivesidle(tcp_keepalives_idle, port) != 
STATUS_OK)
+   return STATUS_ERROR;
+ 
+   if (pq_setkeepalivesinterval(tcp_keepalives_interval, port) != 
STATUS_OK)
+   return STATUS_ERROR;
+ 
+   if (pq_setkeepalivescount(tcp_keepalives_count, port) != 
STATUS_OK)
+   return STATUS_ERROR;
}
  
return STATUS_OK;
***
*** 1158,1160 
--- 1171,1369 
/* in non-error case, copy.c will have emitted the terminator line */
DoingCopyOut = false;
  }
+ 
+ int
+ pq_getkeepalivesidle(Port *port)
+ {
+ #ifdef TCP_KEEPIDLE
+   if (IS_AF_UNIX(port-laddr.addr.ss_family))

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Bruce Momjian]

Oliver Jowett wrote:
 Bruce Momjian wrote:
  Is this patch being worked on?
 
 I have shelved it for the moment, waiting on any further comments from 
 Tom before reworking it.

I thought he wanted the patch modified so keepalive could never be
turned off, but that the OS default  parameters could be changed.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send unregister YourEmailAddressHere to [EMAIL PROTECTED])

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL (was Re: [HACKERS] Feature freeze date for 8.1)

[Tom Lane]

Oliver Jowett [EMAIL PROTECTED] writes:
 Here's a patch that adds four new GUCs:

   tcp_keepalives (defaults to on, controls SO_KEEPALIVE)
   tcp_keepalives_idle (controls TCP_KEEPIDLE)
   tcp_keepalives_interval (controls TCP_KEEPINTVL)
   tcp_keepalives_count (controls TCP_KEEPCNT)

Do you think the system defaults are really going to be port-specific?
I'm slightly annoyed by the number of syscalls this adds to every
connection startup ... getting rid of redundant getsockopt calls would
help.  Getting rid of redundant setsockopt calls (ie, a call to
establish the value that we already know is in force) would help more.

Alternatively, we could lose the frammish that PostgreSQL can tell you
what the system defaults are: 0 in the GUC just means do nothing,
not find out what the current setting is on the off chance that the
user might want to know.

Or, if you really think that's important, it could be left to the SHOW
routines to extract the value on-demand.  That is:
GUC = 0: do nothing at connection start
GUC != 0: setsockopt at connection start
SHOW: do getsockopt and report result

regards, tom lane

---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
  joining column's datatypes do not match

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Oliver Jowett]

Tom Lane wrote:

Oliver Jowett [EMAIL PROTECTED] writes:


Here's a patch that adds four new GUCs:




 tcp_keepalives (defaults to on, controls SO_KEEPALIVE)
 tcp_keepalives_idle (controls TCP_KEEPIDLE)
 tcp_keepalives_interval (controls TCP_KEEPINTVL)
 tcp_keepalives_count (controls TCP_KEEPCNT)


Do you think the system defaults are really going to be port-specific?


I don't understand what you mean. TCP_* override the system defaults on 
a per-connection basis, if that's what you mean.



I'm slightly annoyed by the number of syscalls this adds to every
connection startup ... getting rid of redundant getsockopt calls would
help.  Getting rid of redundant setsockopt calls (ie, a call to
establish the value that we already know is in force) would help more.


I originally did this but went in favor of simpler code. Are the extra 
syscalls an issue? I didn't think they were that expensive..



Alternatively, we could lose the frammish that PostgreSQL can tell you
what the system defaults are: 0 in the GUC just means do nothing,
not find out what the current setting is on the off chance that the
user might want to know.


I didn't do this as it meant that using SET tcp_whatever = 0 does *not* 
reset the setting to what you'd get with a value of 0 in 
postgresql.conf, which seemed confusing to me.


This could all get simpler if we didn't allow per-connection changing of 
that GUC (i.e. set it once at startup and forget about it), which 
probably isn't unreasonable. I wonder if those socket options get 
inherited from the listening socket? Will check.


-O

---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
 joining column's datatypes do not match

Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

[Oliver Jowett]

Oliver Jowett wrote:

 Here's a patch that adds four new GUCs:

 I haven't had a chance to check it builds on other systems or to test
 that this handles actual network failures nicely yet.

The patch builds OK on Solaris 9, which doesn't have the required socket
options.

Solaris (and some other OSes, apparently) supports TCP_KEEPALIVE instead
of TCP_KEEPIDLE/TCP_KEEPCNT/TCP_KEEPINTVL. I will look at adding support
for that next.

-O

---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]

[PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL (was Re: [HACKERS] Feature freeze date for 8.1)

[Oliver Jowett]

Tom Lane wrote:
 Oliver Jowett [EMAIL PROTECTED] writes:
 
Tom Lane wrote:

I'm not convinced that Postgres ought to provide
a way to second-guess the TCP stack ...
 
 
Would you be ok with a patch that allowed configuration of the 
TCP_KEEPCNT / TCP_KEEPIDLE / TCP_KEEPINTVL socket options on backend 
sockets?
 
 
 [ shrug... ]  As long as it doesn't fail to build on platforms that
 don't offer those options, I couldn't complain too hard.  But do we
 really need all that?

Here's a patch that adds four new GUCs:

  tcp_keepalives (defaults to on, controls SO_KEEPALIVE)
  tcp_keepalives_idle (controls TCP_KEEPIDLE)
  tcp_keepalives_interval (controls TCP_KEEPINTVL)
  tcp_keepalives_count (controls TCP_KEEPCNT)

They're ignored for AF_UNIX connections and when running standalone.

tcp_keepalives_* treat 0 as use system default. If the underlying OS
doesn't provide the TCP_KEEP* socket options, the GUCs are present but
reject any value other than 0.

SHOW reflects the currently-in-use value or 0 if not applicable or not
known. i.e. if you set it to 0 and you have the socket options
available, SHOW will show the result of getsockopt() which is non-zero.

A default install on my Linux system produces:

template1=# show all;
  name  |   setting

+--
[...]
 tcp_keepalives | on
 tcp_keepalives_count   | 9
 tcp_keepalives_idle| 7200
 tcp_keepalives_interval| 75
[...]

I haven't had a chance to check it builds on other systems or to test
that this handles actual network failures nicely yet.

-O
Index: doc/src/sgml/runtime.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/runtime.sgml,v
retrieving revision 1.315
diff -c -r1.315 runtime.sgml
*** doc/src/sgml/runtime.sgml   23 Apr 2005 03:27:40 -  1.315
--- doc/src/sgml/runtime.sgml   3 May 2005 01:44:02 -
***
*** 889,894 
--- 889,961 
/listitem
   /varlistentry
   
+  varlistentry id=guc-tcp-keepalives xreflabel=tcp_keepalives
+   termvarnametcp_keepalives/varname (typeboolean/type)/term
+   indexterm
+primaryvarnametcp_keepalives/ configuration parameter/primary
+   /indexterm
+   listitem
+para
+ Controls the use of TCP keepalives on client connections. When 
enabled,
+ idle connections will be periodically probed to check that the client
+ is still present. If sufficient probes are lost, the connection will
+ be broken. This option is ignored for connections made via a
+ Unix-domain socket.
+/para
+   /listitem
+  /varlistentry
+  
+  varlistentry id=guc-tcp-keepalives-idle 
xreflabel=tcp_keepalives_idle
+   termvarnametcp_keepalives_idle/varname 
(typeinteger/type)/term
+   indexterm
+primaryvarnametcp_keepalives_idle/ configuration 
parameter/primary
+   /indexterm
+   listitem
+para
+ On systems that support the TCP_KEEPIDLE socket option, specifies the
+ number of seconds between sending keepalives on an otherwise idle
+ connection. A value of 0 uses the system default. If TCP_KEEPIDLE is
+ not supported, this parameter must be 0. This option is ignored for
+ connections made via a Unix-domain socket, and will have no effect
+ unless the varnametcp_keepalives/varname option is enabled.
+/para
+   /listitem
+  /varlistentry
+  
+  varlistentry id=guc-tcp-keepalives-interval 
xreflabel=tcp_keepalives_interval
+   termvarnametcp_keepalives_interval/varname 
(typeinteger/type)/term
+   indexterm
+primaryvarnametcp_keepalives_interval/ configuration 
parameter/primary
+   /indexterm
+   listitem
+para
+ On systems that support the TCP_KEEPINTVL socket option, specifies how
+ long, in seconds, to wait for a response to a keepalive before
+ retransmitting. A value of 0 uses the system default. If TCP_KEEPINTVL
+ is not supported, this parameter must be 0. This option is ignored
+ for connections made via a Unix-domain socket, and will have no effect
+ unless the varnametcp_keepalives/varname option is enabled.
+/para
+   /listitem
+  /varlistentry
+  
+  varlistentry id=guc-tcp-keepalives-count 
xreflabel=tcp_keepalives_count
+   termvarnametcp_keepalives_count/varname 
(typeinteger/type)/term
+   indexterm
+primaryvarnametcp_keepalives_count/ configuration 
parameter/primary
+   /indexterm
+   listitem
+para
+ On systems that support the TCP_KEEPCNT socket option, specifies how
+ many keepalives may be lost before the connection is considered dead. 
+ A value of 0 uses the system default. If TCP_KEEPINTVL is not
+ supported,