Re: [COMMITTERS] pgsql: Add GUC variables to control keep-alive

Lists: pgsql-committerspgsql-hackers
From: "Rocco Altier" <RoccoA(at)Routescape(dot)com>
To: "Bruce Momjian" <momjian(at)svr1(dot)postgresql(dot)org>, <pgsql-committers(at)postgresql(dot)org>
Cc: <pg-hackers(at)postgresql(dot)org>
Subject: Re: pgsql: Add GUC variables to control keep-alive times for idle, interval,
Date: 2005-07-30 18:04:26
Message-ID: 6E0907A94904D94B99D7F387E08C4F5730F078@FALCON.INSIGHT
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

This broke the build on AIX.

AIX does not have SOL_TCP as a defined symbol in any of the system
header files.

-rocco

> -----Original Message-----
> From: pgsql-committers-owner(at)postgresql(dot)org
> [mailto:pgsql-committers-owner(at)postgresql(dot)org] On Behalf Of
> Bruce Momjian
> Sent: Saturday, July 30, 2005 11:17 AM
> To: pgsql-committers(at)postgresql(dot)org
> Subject: [COMMITTERS] pgsql: Add GUC variables to control
> keep-alive times for idle, interval,
>
>
> Log Message:
> -----------
> Add GUC variables to control keep-alive times for idle, interval, and
> count.
>
> Oliver Jowett
>
> Modified Files:
> --------------
> pgsql/doc/src/sgml:
> runtime.sgml (r1.339 -> r1.340)
>
> (http://developer.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml
/runtime.sgml.diff?r1=1.339&r2=1.340)
pgsql/src/backend/libpq:
pqcomm.c (r1.176 -> r1.177)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/libpq/pqco
mm.c.diff?r1=1.176&r2=1.177)
pgsql/src/backend/utils/misc:
guc.c (r1.279 -> r1.280)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/misc
/guc.c.diff?r1=1.279&r2=1.280)
postgresql.conf.sample (r1.154 -> r1.155)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/misc
/postgresql.conf.sample.diff?r1=1.154&r2=1.155)
pgsql/src/bin/psql:
tab-complete.c (r1.135 -> r1.136)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/bin/psql/tab-compl
ete.c.diff?r1=1.135&r2=1.136)
pgsql/src/include/libpq:
libpq-be.h (r1.49 -> r1.50)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/include/libpq/libp
q-be.h.diff?r1=1.49&r2=1.50)
pgsql/src/include/utils:
guc.h (r1.61 -> r1.62)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/include/utils/guc.
h.diff?r1=1.61&r2=1.62)

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

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


From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Rocco Altier <RoccoA(at)Routescape(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org>
Subject: Re: [COMMITTERS] pgsql: Add GUC variables to control keep-alive times
Date: 2005-07-30 18:29:55
Message-ID: 200507301829.j6UITt003161@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Rocco Altier wrote:
> This broke the build on AIX.
>
> AIX does not have SOL_TCP as a defined symbol in any of the system
> header files.
>

OK, is there any way of setting the keepalive values on AIX?

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


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Rocco Altier" <RoccoA(at)Routescape(dot)com>
Cc: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>, pgsql-committers(at)postgresql(dot)org, pg-hackers(at)postgresql(dot)org
Subject: Re: pgsql: Add GUC variables to control keep-alive times for idle, interval,
Date: 2005-07-30 20:02:18
Message-ID: 5880.1122753738@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

"Rocco Altier" <RoccoA(at)Routescape(dot)com> writes:
> AIX does not have SOL_TCP as a defined symbol in any of the system
> header files.

Seems a tad odd that it would have all three TCP_xxx symbols and not any
way to use them. Perhaps there's something besides SOL_TCP?

regards, tom lane


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Rocco Altier" <RoccoA(at)Routescape(dot)com>
Cc: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>, pgsql-committers(at)postgresql(dot)org, pg-hackers(at)postgresql(dot)org
Subject: Re: pgsql: Add GUC variables to control keep-alive times for idle, interval,
Date: 2005-07-30 20:07:44
Message-ID: 7427.1122754064@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Actually, both the Single Unix Specification and the HPUX man pages are
pretty definite that the correct spelling of the parameter to getsockopt
is "IPPROTO_TCP"; which in fact is what we are using in existing calls.
This SOL_TCP thing is simply wrong.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Rocco Altier <RoccoA(at)Routescape(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [COMMITTERS] pgsql: Add GUC variables to control keep-alive
Date: 2005-07-30 21:44:29
Message-ID: 42EBF4BD.4030904@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Bruce Momjian wrote:

>Rocco Altier wrote:
>
>
>>This broke the build on AIX.
>>
>>AIX does not have SOL_TCP as a defined symbol in any of the system
>>header files.
>>
>>
>>
>
>OK, is there any way of setting the keepalive values on AIX?
>
>
>

Looks like Unixware is broken too,

cheers

andrew


From: "Larry Rosenman" <ler(at)lerctr(dot)org>
To: "'Andrew Dunstan'" <andrew(at)dunslane(dot)net>, "'Bruce Momjian'" <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: "'Rocco Altier'" <RoccoA(at)Routescape(dot)com>, "'PostgreSQL-development'" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [COMMITTERS] pgsql: Add GUC variables to control keep-alive
Date: 2005-07-30 22:07:06
Message-ID: E1DyzUo-0004n1-Mu@lerami.lerctr.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Andrew Dunstan wrote:

>
> Looks like Unixware is broken too,
>
> cheers
>
> andrew
>

I think Tom's fix to use IPPROTO_TCP will fix firefly.

LER

--
Larry Rosenman http://www.lerctr.org/~ler
Phone: +1 972-414-9812 E-Mail: ler(at)lerctr(dot)org
US Mail: 3535 Gaspar Drive, Dallas, TX 75220-3611 US


From: Oliver Jowett <oliver(at)opencloud(dot)com>
To: Larry Rosenman <ler(at)lerctr(dot)org>
Cc: 'Andrew Dunstan' <andrew(at)dunslane(dot)net>, 'Bruce Momjian' <pgman(at)candle(dot)pha(dot)pa(dot)us>, 'Rocco Altier' <RoccoA(at)Routescape(dot)com>, 'PostgreSQL-development' <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [COMMITTERS] pgsql: Add GUC variables to control keep-alive
Date: 2005-07-31 00:41:56
Message-ID: 42EC1E54.5040808@opencloud.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Larry Rosenman wrote:

> I think Tom's fix to use IPPROTO_TCP will fix firefly.

Ah, I forgot about the "we'll just use IP protocol numbers as socket
option levels" behaviour (BSD-derived?). My Linux man page only talks
about SOL_TCP, but I have run into this before and should have
remembered.. my bad.

per my linux/socket.h:

> /* Setsockoptions(2) level. Thanks to BSD these must match IPPROTO_xxx */
> #define SOL_IP 0
> /* #define SOL_ICMP 1 No-no-no! Due to Linux :-) we cannot use SOL_ICMP=1 */
> #define SOL_TCP 6

(I won't get into why using wire-level-protocol constants for syscall
option numbering is a bad idea.. :)

-O


From: "Larry Rosenman" <ler(at)lerctr(dot)org>
To: "'Larry Rosenman'" <ler(at)lerctr(dot)org>, "'Andrew Dunstan'" <andrew(at)dunslane(dot)net>, "'Bruce Momjian'" <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: "'Rocco Altier'" <RoccoA(at)Routescape(dot)com>, "'PostgreSQL-development'" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [COMMITTERS] pgsql: Add GUC variables to control keep-alive
Date: 2005-07-31 02:53:01
Message-ID: E1Dz3ww-0007ct-AV@lerami.lerctr.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Larry Rosenman wrote:

>
> I think Tom's fix to use IPPROTO_TCP will fix firefly.
>
> LER

And based on the last run, it did.

Thanks, Tom!

--
Larry Rosenman http://www.lerctr.org/~ler
Phone: +1 972-414-9812 E-Mail: ler(at)lerctr(dot)org
US Mail: 3535 Gaspar Drive, Dallas, TX 75220-3611 US


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Oliver Jowett <oliver(at)opencloud(dot)com>
Cc: Larry Rosenman <ler(at)lerctr(dot)org>, "'Andrew Dunstan'" <andrew(at)dunslane(dot)net>, "'Bruce Momjian'" <pgman(at)candle(dot)pha(dot)pa(dot)us>, "'Rocco Altier'" <RoccoA(at)Routescape(dot)com>, "'PostgreSQL-development'" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [COMMITTERS] pgsql: Add GUC variables to control keep-alive
Date: 2005-07-31 07:30:47
Message-ID: 25517.1122795047@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Oliver Jowett <oliver(at)opencloud(dot)com> writes:
> per my linux/socket.h:

>> /* Setsockoptions(2) level. Thanks to BSD these must match IPPROTO_xxx */
>> #define SOL_IP 0
>> /* #define SOL_ICMP 1 No-no-no! Due to Linux :-) we cannot use SOL_ICMP=1 */
>> #define SOL_TCP 6

> (I won't get into why using wire-level-protocol constants for syscall
> option numbering is a bad idea.. :)

[ raised eyebrow... ] You think the wire protocol numbers are more
likely to change than the syscall assignments? Consider the range of
systems that each change would affect.

Also, that assignment is not mere BSD-ism, it's specified by the Single
Unix Spec, as well as every other document I could lay my hands on
today. Whoever wrote the comments in linux/socket.h is just
clueless.

regards, tom lane


From: Oliver Jowett <oliver(at)opencloud(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: 'PostgreSQL-development' <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [COMMITTERS] pgsql: Add GUC variables to control keep-alive
Date: 2005-07-31 10:54:25
Message-ID: 42ECADE1.7060108@opencloud.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Tom Lane wrote:
> Oliver Jowett <oliver(at)opencloud(dot)com> writes:
>
>>per my linux/socket.h:
>
>
>>>/* Setsockoptions(2) level. Thanks to BSD these must match IPPROTO_xxx */
>>>#define SOL_IP 0
>>>/* #define SOL_ICMP 1 No-no-no! Due to Linux :-) we cannot use SOL_ICMP=1 */
>>>#define SOL_TCP 6
>
>
>>(I won't get into why using wire-level-protocol constants for syscall
>>option numbering is a bad idea.. :)
>
>
> [ raised eyebrow... ] You think the wire protocol numbers are more
> likely to change than the syscall assignments? Consider the range of
> systems that each change would affect.

I'm not worried about changing values; I think that representing the
"option level" as an IP protocol number, in an interface that
encompasses non-IP protocols, is a bad API design decision.

If the rule for setsockopt is "pass the IP protocol number as the level
argument", then what value should I pass to manipulate, say,
X.25-specific settings on an X.25 socket?

In practice you use a synthetic value which is outside the range of
valid IP protocols. getprotoent() won't know about this value and
there's no IPPROTO_ define for it -- just a SOL_ one. So, er, why were
you using IP protocol constants as levels again? A clear design would
have them as two separate namespaces even if they happened to share values.

> Also, that assignment is not mere BSD-ism, it's specified by the Single
> Unix Spec, as well as every other document I could lay my hands on
> today. Whoever wrote the comments in linux/socket.h is just
> clueless.

That seems a bit unfair, the comment is accurate (the SOL_* constants do
need to match the protocol number assignments for compatibility with
code that expects the BSD behaviour) and might even predate SUS.

From a portability point of view, it certainly seems better to use
IPPROTO_* and I have no problem with doing that. It just grates at a
design level.

Anyway, this doesn't affect the patch one way or the other, which is why
I didn't really want to get into it in the first place..

-O


From: Andrew - Supernews <andrew+nonews(at)supernews(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Add GUC variables to control keep-alive
Date: 2005-08-01 02:39:42
Message-ID: slrnder2re.bj3.andrew+nonews@trinity.supernews.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

On 2005-07-31, Oliver Jowett <oliver(at)opencloud(dot)com> wrote:
> I'm not worried about changing values; I think that representing the
> "option level" as an IP protocol number, in an interface that
> encompasses non-IP protocols, is a bad API design decision.

The interpretation of that parameter, if not equal to SOL_SOCKET, is
clearly protocol-dependent and therefore driven by the protocol family of
the socket.

> If the rule for setsockopt is "pass the IP protocol number as the level
> argument", then what value should I pass to manipulate, say,
> X.25-specific settings on an X.25 socket?

The rule is "pass whatever value the socket's protocol family expects".
For IP that happens to be an IP protocol number. For other families it
is some constant meaningful to that family.

> In practice you use a synthetic value which is outside the range of
> valid IP protocols.

No, you don't. There is no assumption that socket level parameters form
a single namespace (with the exception of SOL_SOCKET which is common to
all protocol families).

> getprotoent() won't know about this value

getprotoent() is specific to the INET address family and makes no attempt
to pretend to support anything else (if it did, it'd need to take an AF_*
parameter to disambiguate between families).

>> Also, that assignment is not mere BSD-ism, it's specified by the Single
>> Unix Spec, as well as every other document I could lay my hands on
>> today. Whoever wrote the comments in linux/socket.h is just
>> clueless.
>
> That seems a bit unfair, the comment is accurate (the SOL_* constants do
> need to match the protocol number assignments for compatibility with
> code that expects the BSD behaviour) and might even predate SUS.

It's based on a fundamental misunderstanding that SOL_* constants other than
SOL_SOCKET should even exist and that they form a namespace (which they do
not).

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services


From: Oliver Jowett <oliver(at)opencloud(dot)com>
To: andrew(at)supernews(dot)com
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Add GUC variables to control keep-alive
Date: 2005-08-01 03:24:09
Message-ID: 42ED95D9.8000203@opencloud.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Andrew - Supernews wrote:
> On 2005-07-31, Oliver Jowett <oliver(at)opencloud(dot)com> wrote:
>
>>I'm not worried about changing values; I think that representing the
>>"option level" as an IP protocol number, in an interface that
>>encompasses non-IP protocols, is a bad API design decision.
>
> The interpretation of that parameter, if not equal to SOL_SOCKET, is
> clearly protocol-dependent and therefore driven by the protocol family of
> the socket.

It's *not* clearly protocol dependent, that's my point about the API not
being clearly designed. SUS just says this:

>> The level argument specifies the protocol level at which the option
>> resides. To set options at the socket level, specify the level argument
>> as SOL_SOCKET. To set options at other levels, supply the appropriate
>> level identifier for the protocol controlling the option. For example,
>> to indicate that an option is interpreted by the TCP (Transport Control
>> Protocol), set level to IPPROTO_TCP as defined in the <netinet/in.h>
>> header.

There's no dependency on socket PF mentioned there, and the obvious
reading of that text is that a "level identifier" uniquely identifies
the "protocol controlling the option" -- so IPPROTO_TCP unambiguously
means "the TCP protocol".

Having multiple socket-PF-dependent namespaces which might overlap is
just asking for hard-to-find bugs (if you accidentally manage to use the
wrong namespace for the socket, you run the risk of getting weird
behaviour rather than an error).

Still counts as badly designed in my book, sorry.

-O


From: Andrew - Supernews <andrew+nonews(at)supernews(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Add GUC variables to control keep-alive
Date: 2005-08-01 04:17:38
Message-ID: slrnder8j1.bj3.andrew+nonews@trinity.supernews.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

On 2005-08-01, Oliver Jowett <oliver(at)opencloud(dot)com> wrote:
> There's no dependency on socket PF mentioned there, and the obvious
> reading of that text is that a "level identifier" uniquely identifies
> the "protocol controlling the option" -- so IPPROTO_TCP unambiguously
> means "the TCP protocol".

You're reading more into the spec than is present. It should be obvious
that the existence of, number of and organization of any layers that might
happen to exist below the socket layer is entirely dependent on the
protocol family. The fact that PF-dependent constants are used for
referring to such layers should reinforce this.

> Having multiple socket-PF-dependent namespaces which might overlap is
> just asking for hard-to-find bugs (if you accidentally manage to use the
> wrong namespace for the socket, you run the risk of getting weird
> behaviour rather than an error).

Think about what happens when you add a brand-new protocol family.

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services