Re: Network write errors (was: Re: Feature freeze date for

You might remember that when we released 8.0, the plan was to have a
12-month development cycle for 8.1, unless there were Win32 problems
that required complex fixes, in which case we would have a shorter 8.1
cycle.

Well the good news is that there have been almost no Win32 problems, but
the other good news is that we are getting a lot of powerful features
for 8.1 already:

o two-phase (Heikki Linnakangas, almost done)
o multiple out function paramters (Tom, done)
o bitmappted indexes (Tom, almost done)
o shared row locks (Alvaro, almost done)
o integrated auto-vacuum (Bruce)
o buffer cache fixes for SMP (Tom, done)

It is possible all these items will be done by sometime in June. Now,
if that happens, do we want to continue with the 12-month plan or
shorten the 8.1 release cycle, perhaps targeting a release in the
September/October timeframe?

The current core proposal is to do feature freeze on July 1, with the
understanding that we will be done most of the items above by then and
have the outstanding patches applied.

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

On Thu, Apr 28, 2005 at 09:02:40 -0400,
Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> wrote:
> Well the good news is that there have been almost no Win32 problems, but
> the other good news is that we are getting a lot of powerful features
> for 8.1 already:

You forgot to list the indexed aggregate feature for max and min. While
this isn't that important for experienced postgres users, it is a gotcha
for new users. Between this, integrated autovacuum and the cross type
index changes in 8.0 we have covered almost all of the newbie gotchas.
This should make Postgres effectively equivalent in difficulty with
getting started for new users as MySQL. That could significantly
boost usage for low end use once word gets out.

Bruce Momjian wrote:
> You might remember that when we released 8.0, the plan was to have a
> 12-month development cycle for 8.1, unless there were Win32 problems
> that required complex fixes, in which case we would have a shorter 8.1
> cycle.
>
> Well the good news is that there have been almost no Win32 problems, but
> the other good news is that we are getting a lot of powerful features
> for 8.1 already:
>
> o two-phase (Heikki Linnakangas, almost done)
> o multiple out function paramters (Tom, done)
> o bitmappted indexes (Tom, almost done)
> o shared row locks (Alvaro, almost done)
> o integrated auto-vacuum (Bruce)
> o buffer cache fixes for SMP (Tom, done)
>
> It is possible all these items will be done by sometime in June. Now,
> if that happens, do we want to continue with the 12-month plan or
> shorten the 8.1 release cycle, perhaps targeting a release in the
> September/October timeframe?
>
> The current core proposal is to do feature freeze on July 1, with the
> understanding that we will be done most of the items above by then and
> have the outstanding patches applied.

It seems to be a good idea to take the chance now to make a release.
Delaying the release would mean preventing the wide usage of features
although they appear production grade. OTOH, if feature freeze is
delayed some seemingly essential features for 8.1 which might arise in
the meantime might delay the release further, or induce late feature
exclusion when some last minute issues are discovered. Integrated
autovacuum seems good enough a reason to release early.

Regards,
Andreas

In the last exciting episode, pgman(at)candle(dot)pha(dot)pa(dot)us (Bruce Momjian) wrote:
> o integrated auto-vacuum (Bruce)

If this can kick off a vacuum of a Very Large Table at an unfortunate
time, this can turn out to be a prety painful misfeature.

What I'd _really_ love to see (and alas, it's beyond my ken) is some
parallel to the FSM, namely a "Recently Updated Blocks Map," which
would enable a vacuuming approach that would not go through entire
tables, but which would rather go through only those blocks known to
be recently updated.

There continues to be trouble if you have a table that grows to 50
million rows where there are 100K rows that are being heavily
updated. In effect, only the 100K rows need facuuming.
--
(reverse (concatenate 'string "moc.liamg" "@" "enworbbc"))
http://linuxfinances.info/info/emacs.html
Group Dynamics
"Following Minsky and Schelling, consider a person as a society of
agents. A group is then a larger society of such agents. Understand
groups by examining interactions of coalitions of agents that
cross-cut their grouping into people."
-- Mark Miller

Christopher Browne <cbbrowne(at)acm(dot)org> writes:
> In the last exciting episode, pgman(at)candle(dot)pha(dot)pa(dot)us (Bruce Momjian) wrote:
>> o integrated auto-vacuum (Bruce)

> If this can kick off a vacuum of a Very Large Table at an unfortunate
> time, this can turn out to be a prety painful misfeature.

[ shrug... ] You'll always be able to turn it off if you don't want it.
I'm not sure that we'll be ready to turn it on by default even in 8.1.

regards, tom lane

Tom Lane wrote:
> Christopher Browne <cbbrowne(at)acm(dot)org> writes:
> > In the last exciting episode, pgman(at)candle(dot)pha(dot)pa(dot)us (Bruce Momjian) wrote:
> >> o integrated auto-vacuum (Bruce)
>
> > If this can kick off a vacuum of a Very Large Table at an unfortunate
> > time, this can turn out to be a prety painful misfeature.
>
> [ shrug... ] You'll always be able to turn it off if you don't want it.
> I'm not sure that we'll be ready to turn it on by default even in 8.1.

Agreed. It will just be there to turn on from postgresql.conf if you
want it, and we do have TODO information about keeping such an FSM for
recently expired pages.

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

Tom Lane wrote:

>Christopher Browne <cbbrowne(at)acm(dot)org> writes:
>
>
>>If this can kick off a vacuum of a Very Large Table at an unfortunate
>>time, this can turn out to be a prety painful misfeature.
>>
>>
>
>[ shrug... ] You'll always be able to turn it off if you don't want it.
>I'm not sure that we'll be ready to turn it on by default even in 8.1.
>
>

What to people think about having an optional "maintenance window" so
that autovac only takes action during an approved time. But perhaps
just using the vacuum delay settings will be enough.

On Fri, 29 Apr 2005, Bruno Wolff III wrote:

> On Fri, Apr 29, 2005 at 10:09:43 -0400,
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> wrote:
>> Tom Lane wrote:
>>> Christopher Browne <cbbrowne(at)acm(dot)org> writes:
>>>> In the last exciting episode, pgman(at)candle(dot)pha(dot)pa(dot)us (Bruce Momjian) wrote:
>>>>> o integrated auto-vacuum (Bruce)
>>>
>>>> If this can kick off a vacuum of a Very Large Table at an unfortunate
>>>> time, this can turn out to be a prety painful misfeature.
>>>
>>> [ shrug... ] You'll always be able to turn it off if you don't want it.
>>> I'm not sure that we'll be ready to turn it on by default even in 8.1.
>>
>> Agreed. It will just be there to turn on from postgresql.conf if you
>> want it, and we do have TODO information about keeping such an FSM for
>> recently expired pages.
>
> I think if we aren't finding problems in testing that it would be better
> to turn pg_autovacuum on by default. Vacuum is something the burns new
> users and having it one by default is going to cut down on surprises.

Except for the surprise of peridically having the system go unresponsive
because it hit a large table, and that new user wondering what is wrong
with postgresql that it just stalls seemingly randomly :(

----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email: scrappy(at)hub(dot)org Yahoo!: yscrappy ICQ: 7615664

On Fri, Apr 29, 2005 at 10:09:43 -0400,
Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> wrote:
> Tom Lane wrote:
> > Christopher Browne <cbbrowne(at)acm(dot)org> writes:
> > > In the last exciting episode, pgman(at)candle(dot)pha(dot)pa(dot)us (Bruce Momjian) wrote:
> > >> o integrated auto-vacuum (Bruce)
> >
> > > If this can kick off a vacuum of a Very Large Table at an unfortunate
> > > time, this can turn out to be a prety painful misfeature.
> >
> > [ shrug... ] You'll always be able to turn it off if you don't want it.
> > I'm not sure that we'll be ready to turn it on by default even in 8.1.
>
> Agreed. It will just be there to turn on from postgresql.conf if you
> want it, and we do have TODO information about keeping such an FSM for
> recently expired pages.

I think if we aren't finding problems in testing that it would be better
to turn pg_autovacuum on by default. Vacuum is something the burns new
users and having it one by default is going to cut down on surprises.
Experienced users know about vacuum and will probably read the release
notes when upgrading and do something that is appropiate for them.

"Matthew T. O'Connor" <matthew(at)zeut(dot)net> writes:
> What to people think about having an optional "maintenance window" so
> that autovac only takes action during an approved time. But perhaps
> just using the vacuum delay settings will be enough.

I'm not sure autovac should go completely catatonic during the day;
what if someone does an unusual mass deletion, or something? But
it does seem pretty reasonable to have a notion of a maintenance
window where it should be more active than it is at other times.

Maybe what you want is two complete sets of autovac parameters.
Definitely at least two sets of the vacuum-delay values.

regards, tom lane

On Fri, Apr 29, 2005 at 12:43:37 -0300,
"Marc G. Fournier" <scrappy(at)postgresql(dot)org> wrote:
> On Fri, 29 Apr 2005, Bruno Wolff III wrote:
>
> Except for the surprise of peridically having the system go unresponsive
> because it hit a large table, and that new user wondering what is wrong
> with postgresql that it just stalls seemingly randomly :(

I think most users running systems with that large of tables will know
what they are doing. And will be more careful with vacuum.

Vacuum running for a long time on large tables with few pages that need
updating is really a separate problem that could use its own solution.

On Fri, 29 Apr 2005, Tom Lane wrote:

> "Matthew T. O'Connor" <matthew(at)zeut(dot)net> writes:
>> What to people think about having an optional "maintenance window" so
>> that autovac only takes action during an approved time. But perhaps
>> just using the vacuum delay settings will be enough.
>
> I'm not sure autovac should go completely catatonic during the day;
> what if someone does an unusual mass deletion, or something? But
> it does seem pretty reasonable to have a notion of a maintenance
> window where it should be more active than it is at other times.
>
> Maybe what you want is two complete sets of autovac parameters.
> Definitely at least two sets of the vacuum-delay values.

With the introduction of the stats collector, is there not some way of
extending it so that autovac has more information to work off of? For
instance, in my environment, we have clients in every timezone hitting the
database ... our Japanese clients will be busy at a totally different time
of day then our East Coast/NA clients, so a 'maintenance window' is near
impossible to state ...

I know one person was talking about being able to target only those that
pages that have changes, instead of the whole table ... but some sort of
"load monitoring" that checks # of active connections and tries to find
'lulls'?

Basically, everything right now is being keyed to updates to the tables
themselves, but isn't looking at what the system itself is doing ... if
I'm doing a massive import of data into a table, the last time I want is a
VACUUM to cut in and slow down the loading ...

----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email: scrappy(at)hub(dot)org Yahoo!: yscrappy ICQ: 7615664

Martha Stewart called it a Good Thing when scrappy(at)postgresql(dot)org ("Marc G. Fournier") wrote:
> I know one person was talking about being able to target only those
> that pages that have changes, instead of the whole table ... but some
> sort of "load monitoring" that checks # of active connections and
> tries to find 'lulls'?

I have some "log table purging" processes I'd like to put in place; it
would be really slick to be able to get some statistics from the
system as to how busy the DB has been in the last little while.

The nice, adaptive algorithm:

- Loop forever

- Once a minute, evaluate how busy things seem, giving some metric X

-> If X is "high" then purge 10 elderly tuples from table log_table
-> If X is "moderate" then purge 100 elderly tuples from table
log_table
-> If X is "low" then purge 1000 elderly tuples from table
log_table

The trouble is in measuring some form of "X."

Some reasonable approximations might include:
- How much disk I/O was recorded in the last 60 seconds?
- How many application transactions (e.g. - invoices or such) were
issued in the last 60 seconds (monitoring a sequence could be
good enough).
--
output = reverse("gro.mca" "@" "enworbbc")
http://linuxfinances.info/info/slony.html
?OM ERROR

I think what you're suggesting is that vacuum settings (most likely
delay) take into consideration the load on the database, which I think
is a great idea. One possibility is if vacuum tracks how many blocks
it's read/written, it can see how many blocks the database has done
overall; subtract the two and you know how much other disk IO is going
on in the system. You can then use that number to decide how long you'll
sleep before the next vacuum cycle.

On Fri, Apr 29, 2005 at 01:34:56PM -0300, Marc G. Fournier wrote:
> On Fri, 29 Apr 2005, Tom Lane wrote:
>
> >"Matthew T. O'Connor" <matthew(at)zeut(dot)net> writes:
> >>What to people think about having an optional "maintenance window" so
> >>that autovac only takes action during an approved time. But perhaps
> >>just using the vacuum delay settings will be enough.
> >
> >I'm not sure autovac should go completely catatonic during the day;
> >what if someone does an unusual mass deletion, or something? But
> >it does seem pretty reasonable to have a notion of a maintenance
> >window where it should be more active than it is at other times.
> >
> >Maybe what you want is two complete sets of autovac parameters.
> >Definitely at least two sets of the vacuum-delay values.
>
> With the introduction of the stats collector, is there not some way of
> extending it so that autovac has more information to work off of? For
> instance, in my environment, we have clients in every timezone hitting the
> database ... our Japanese clients will be busy at a totally different time
> of day then our East Coast/NA clients, so a 'maintenance window' is near
> impossible to state ...
>
> I know one person was talking about being able to target only those that
> pages that have changes, instead of the whole table ... but some sort of
> "load monitoring" that checks # of active connections and tries to find
> 'lulls'?
>
> Basically, everything right now is being keyed to updates to the tables
> themselves, but isn't looking at what the system itself is doing ... if
> I'm doing a massive import of data into a table, the last time I want is a
> VACUUM to cut in and slow down the loading ...
>
> ----
> Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
> Email: scrappy(at)hub(dot)org Yahoo!: yscrappy ICQ: 7615664
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>

--
Jim C. Nasby, Database Consultant decibel(at)decibel(dot)org
Give your computer some brain candy! www.distributed.net Team #1828

Windows: "Where do you want to go today?"
Linux: "Where do you want to go tomorrow?"
FreeBSD: "Are you guys coming, or what?"

On Fri, 29 Apr 2005, Christopher Browne wrote:

> Martha Stewart called it a Good Thing when scrappy(at)postgresql(dot)org ("Marc G. Fournier") wrote:
>> I know one person was talking about being able to target only those
>> that pages that have changes, instead of the whole table ... but some
>> sort of "load monitoring" that checks # of active connections and
>> tries to find 'lulls'?
>
> I have some "log table purging" processes I'd like to put in place; it
> would be really slick to be able to get some statistics from the
> system as to how busy the DB has been in the last little while.
>
> The nice, adaptive algorithm:
>
> - Loop forever
>
> - Once a minute, evaluate how busy things seem, giving some metric X
>
> -> If X is "high" then purge 10 elderly tuples from table log_table
> -> If X is "moderate" then purge 100 elderly tuples from table
> log_table
> -> If X is "low" then purge 1000 elderly tuples from table
> log_table
>
> The trouble is in measuring some form of "X."
>
> Some reasonable approximations might include:
> - How much disk I/O was recorded in the last 60 seconds?
> - How many application transactions (e.g. - invoices or such) were
> issued in the last 60 seconds (monitoring a sequence could be
> good enough).

Some way of doing a 'partial vacuum' would be nice ... where a VACUUM
could stop after it processed those '10 elderly tuples' and on the next
pass, resume from that point instead of starting from the beginning again
...

----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email: scrappy(at)hub(dot)org Yahoo!: yscrappy ICQ: 7615664

Marc G. Fournier wrote:

> On Fri, 29 Apr 2005, Christopher Browne wrote:
>
>> Some reasonable approximations might include:
>> - How much disk I/O was recorded in the last 60 seconds?
>> - How many application transactions (e.g. - invoices or such) were
>> issued in the last 60 seconds (monitoring a sequence could be
>> good enough).
>
>
> Some way of doing a 'partial vacuum' would be nice ... where a VACUUM
> could stop after it processed those '10 elderly tuples' and on the
> next pass, resume from that point instead of starting from the
> beginning again ...

That is sorta what the vacuum delay settings accomplish.

The world rejoiced as matthew(at)zeut(dot)net ("Matthew T. O'Connor") wrote:
> Marc G. Fournier wrote:
>
>> On Fri, 29 Apr 2005, Christopher Browne wrote:
>>
>>> Some reasonable approximations might include:
>>> - How much disk I/O was recorded in the last 60 seconds?
>>> - How many application transactions (e.g. - invoices or such) were
>>> issued in the last 60 seconds (monitoring a sequence could be
>>> good enough).
>>
>>
>> Some way of doing a 'partial vacuum' would be nice ... where a
>> VACUUM could stop after it processed those '10 elderly tuples' and
>> on the next pass, resume from that point instead of starting from
>> the beginning again ...
>
> That is sorta what the vacuum delay settings accomplish.

What they do is orthogonal to that.

"Vacuum delay" prevents vacuum I/O from taking over the I/O bus.

Unfortunately, if you have a table with a very large number of _live_
tuples, there is no way to skip over those and only concentrate on the
dead ones.

In that scenario "vacuum delay" leads to the vacuum on the table
running for a Very, Very Long Time, because it sits there delaying a
lot as it walks thru pages it never modifies. The one good news is
that, for any pages where no tuples are touched, the indices are also
left untouched.
--
wm(X,Y):-write(X),write('@'),write(Y). wm('cbbrowne','gmail.com').
http://linuxdatabases.info/info/slony.html
"The Board views the endemic use of PowerPoint briefing slides instead
of technical papers as an illustration of the problematic methods of
technical communication at NASA." -- Official report on the Columbia
shuttle disaster.

Hi,

> What to people think about having an optional "maintenance window" so
> that autovac only takes action during an approved time.

This sounds like a realy good idea to me!
Sander.

We have talked about performance and some new features
before freeze of 8.1. Like ;

· Bitmap indexes
· Autovacuum
· GIS features
· Object-Oriented features
· PITR
· Table Partition

But there is a feature that is too important for a
database. It is availability.Now PostgreSQL doesn't have
high availability.We must discuss it here. Imagine a
database that has a lots of features that others don’t
have. I tested the PostgreSQL for that feature, i couldn't
find it enough. Here :

Process A start to update / insert some rows in a table
and then the connection of process A is lost to PostgreSQL
before it sends commit or rollback. Other processes want to
update the same rows or SELECT …..FOR UPDATE for the same
rows.Now these processes are providing SELECT WAITING… or
CANCEL QUERY if statement_timeout was set. Imagine these
processes is getting grower. What will we do now ?
Restarting backend or finding process A and kill it ?

Now, do you think that the PostgreSQL database is a high
available database ? A feature must be added to solve that
problem or PostgreSQL databases would never get a good
place among huge databases.

Best Regards

Adnan DURSUN
ASRIN Bili&#351;im Ltd.
Ankara /TURKEY

On Sun, May 01, 2005 at 03:09:37PM +0300, adnandursun(at)asrinbilisim(dot)com(dot)tr wrote:

> Process A start to update / insert some rows in a table
> and then the connection of process A is lost to PostgreSQL
> before it sends commit or rollback. Other processes want to
> update the same rows or SELECT …..FOR UPDATE for the same
> rows.Now these processes are providing SELECT WAITING… or
> CANCEL QUERY if statement_timeout was set. Imagine these
> processes is getting grower. What will we do now ?
> Restarting backend or finding process A and kill it ?

Well, if process A loses the connection to the client, then the
transaction will be rolled back and other processes will be able to
continue.

Another thing to keep in mind is that if process A is inserting a tuple,
other processes will not see it because it isn't committed. So MVCC
rules protect them from blocking. (Unless there is a unique restriction
and some other process wants to insert the same value to it.)

Now, we do have an "availability" problem in 8.0 and earlier, which is
that you could block trying to check a foreign key that other process is
also checking. I am happy to say that it doesn't happen anymore so
that's one less barrier.

--
Alvaro Herrera (<alvherre[(at)]dcc(dot)uchile(dot)cl>)
"In fact, the basic problem with Perl 5's subroutines is that they're not
crufty enough, so the cruft leaks out into user-defined code instead, by
the Conservation of Cruft Principle." (Larry Wall, Apocalypse 6)

Alvaro Herrera wrote:
> On Sun, May 01, 2005 at 03:09:37PM +0300,
adnandursun(at)asrinbilisim(dot)com(dot)tr wrote:
> > Process A start to update / insert some rows in a table
> > and then the connection of process A is lost to PostgreSQL
> > before it sends commit or rollback. Other processes want to
> > update the same rows or SELECT …..FOR UPDATE for the same
> > rows.Now these processes are providing SELECT WAITING… or
> > CANCEL QUERY if statement_timeout was set. Imagine these
> > processes is getting grower. What will we do now ?
> > Restarting backend or finding process A and kill it ?
>
> Well, if process A loses the connection to the client, then the
> transaction will be rolled back and other processes will be able to
> continue.

The problem, as I understand it, is that if you have a long-running
query and the client process disappears, the query keeps running and
holds whatever resources it may have until it finishes. In fact, it
keeps sending data to the client and keeps ignoring the SIGPIPE it gets
(in case of a Unix-domain socket connection).

Now of course this has nothing to do with "high availability" and does
not warrant hijacking a thread about the release schedule, but it may
be worth investigating.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

On Sun, 1 May 2005, Alvaro Herrera wrote:

> Well, if process A loses the connection to the client, then the
> transaction will be rolled back and other processes will be able to
> continue.

If the other end of a tcp/ip connection just disapears, for example if the
network cable is cut off then in linux it can take up to 2 hours as
default for it to close the connection. Normally if a client application
dies then the client OS cleans up and closes the socket so that the server
knows about it.

There are some settings that one can alter to change the time it waits
before probing and killing the connection, ie tcp_keepalive_time in
/proc/sys/net/ipv4/.

It's documented in "man tcp" that say that it will take 2h11m as default
to kill of such a connection.

Pg could of course also implement some pinging protocl that should be done
every now and then by the client so that the server knows that it is
alive. For now you just have to lower the global settings as the one above
if you want it to handle it better.

--
/Dennis Björklund

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> The problem, as I understand it, is that if you have a long-running
> query and the client process disappears, the query keeps running and
> holds whatever resources it may have until it finishes.

There is a trivial solution for this: it's called statement_timeout.

If the concern is that a process may block other processes for a long
time, what does it matter whether the client is still connected or not?
It's the long-running command in itself that is the problem. So you
limit the time the command can run.

It might be interesting to think about a transaction_timeout as well,
to bound the time locks can be held. But none of this has anything
to do with "high availability" as I understand the term. It looks
more like a forcing function to make your users fix poorly-written
client software ;-)

regards, tom lane

On 2005-05-01, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> The problem, as I understand it, is that if you have a long-running
> query and the client process disappears, the query keeps running and
> holds whatever resources it may have until it finishes. In fact, it
> keeps sending data to the client and keeps ignoring the SIGPIPE it gets
> (in case of a Unix-domain socket connection).

Ignoring the SIGPIPE is exactly the right thing to do.

What's _not_ a good idea is ignoring the EPIPE error from write(), which
seems to currently be reported via ereport(COMMERROR) which doesn't try
and abort the query as far as I can tell.

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

On Sun, 01 May 2005 11:37:47 -0400
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
>> The problem, as I understand it, is that if you have a
>long-running
>> query and the client process disappears, the query keeps
>running and
>> holds whatever resources it may have until it finishes.
>
>There is a trivial solution for this: it's called
>statement_timeout.

statement_timeout is not a solution if many processes are
waiting the resource. statement_timeout is providing a
escape mechanism. Imagine tens of processes are waiting,
statement_timeout refuses them but this processes must do
their works.

>If the concern is that a process may block other processes
>for a long
>time, what does it matter whether the client is still
>connected or not?
>It's the long-running command in itself that is the
>problem. So you
>limit the time the command can run.
>
>It might be interesting to think about a
>transaction_timeout as well,
>to bound the time locks can be held. But none of this has
>anything
>to do with "high availability" as I understand the term.
> It looks
>more like a forcing function to make your users fix
>poorly-written
>client software ;-)

Listen Tom, write a client software that releases the
resources / locks that was hold before client power is down
or client connection was lost.

Do we must suggest this solution to ppl that wants to use
PostgreSQL or do we must implement a pinging mechanism to
check whether client is dead or live ?

Best Regards

Adnan DURSUN
ASRIN Bilişim Hiz.Ltd.
Ankara / TURKEY

On P, 2005-05-01 at 11:37 -0400, Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > The problem, as I understand it, is that if you have a long-running
> > query and the client process disappears, the query keeps running and
> > holds whatever resources it may have until it finishes.
>
> There is a trivial solution for this: it's called statement_timeout.

statement timeout does not solve the fundamental problem of server not
seeing when a cleint connection has disappeared.

I had another variant of the same problem - i had 300 client connections
to the same postmaster, postmaster was configured to handle 500
simultaneous connections. Then there was a network outage for a few
minutes, during which clients saw that the postmaster was not there and
closed their conections and tried to make new ones. After the network
came back, only 200 of them were able to reconnect, as the server never
saw them leaving and just kept the 300 zombie connections.

> It might be interesting to think about a transaction_timeout as well,
> to bound the time locks can be held. But none of this has anything
> to do with "high availability" as I understand the term. It looks
> more like a forcing function to make your users fix poorly-written
> client software ;-)

Im my case all ttransactions were implicit one command functon calls
("select * from dbfunc()"), so transaction timeout would not help.

probably the only way for server to detect stale connections would be
sending/receiving some kind of keepalives.

--
Hannu Krosing <hannu(at)skype(dot)net>

On Sun, May 01, 2005 at 19:57:37 +0300,
adnandursun(at)asrinbilisim(dot)com(dot)tr wrote:
>
> Listen Tom, write a client software that releases the
> resources / locks that was hold before client power is down
> or client connection was lost.

If Postgres can tell the connection has been lost then it should roll
back the connection. The problem is that you can't always tell if
a connection has been lost. All you can do is timeout, either when TCP
times out or some other timeout (such as a statment timeout) that you
set.

On Sun, 1 May 2005 14:35:37 -0500
Bruno Wolff III <bruno(at)wolff(dot)to> wrote:
>On Sun, May 01, 2005 at 19:57:37 +0300,
> adnandursun(at)asrinbilisim(dot)com(dot)tr wrote:
>>
>> Listen Tom, write a client software that releases the
>> resources / locks that was hold before client power is
>down
>> or client connection was lost.
>
>If Postgres can tell the connection has been lost then it
>should roll back the connection.

Yes, but, Can PostgreSQL know which connection is lost or
live or dead ?

>The problem is that you can't always
>tell if a connection has been lost. All you can do is
timeout, either when TCP
>times out or some other timeout (such as a statment
timeout) that you set.

You are right, a timeout parameter must be used for that
on the backend. a client application never find the
previous instance before it crashed. However more than one
connection was able to be established to PostgreSQL
backend..

Statement_timeout is just a escape mechanism for active
transaction. Imagine; you've started a process to update
the rows in a table then your PC power was down but you
have not sent commit or rollback yet..What will happen now
? Example Codes ;

-- Client Side of Codes

1. send statement_timeout = 10;
2. start a transaction;
3. start to update table;
** connection is lost here
4. commit;

Best Regards,

Adnan DURSUN
ASRIN Bilişim Hiz.Ltd.
Ankara / TURKEY

After takin a swig o' Arrakan spice grog, adnandursun(at)asrinbilisim(dot)com(dot)tr belched out:
> On Sun, 1 May 2005 14:35:37 -0500
> Bruno Wolff III <bruno(at)wolff(dot)to> wrote:
>>On Sun, May 01, 2005 at 19:57:37 +0300,
>> adnandursun(at)asrinbilisim(dot)com(dot)tr wrote:
>>>
>>> Listen Tom, write a client software that releases the
>>> resources / locks that was hold before client power is
>>down
>>> or client connection was lost.
>>
>>If Postgres can tell the connection has been lost then it
>>should roll back the connection.
>
> Yes, but, Can PostgreSQL know which connection is lost or
> live or dead ?

Certainly, with the "magic connection analysis protocol."

Or not...

In order to that sort of analysis, you need to have some form of
heartbeat monitor on the connection, thereby requiring extra
connections.

It could make sense to attach that kind of extra apparatus to a
connection pool manager like pgpool, but probably not directly to
PostgreSQL itself.

You might want to look into pgpool; that is something that many people
interested in "enterprise usage" of PostgreSQL are looking into...
--
(reverse (concatenate 'string "moc.liamg" "@" "enworbbc"))
http://linuxdatabases.info/info/slony.html
"A army's effectiveness depends on its size, training, experience and
morale, and morale is worth more than all the other factors combined."
-- Napoleon Bonaparte

adnandursun(at)asrinbilisim(dot)com(dot)tr wrote:
> statement_timeout is not a solution if many processes are
> waiting the resource.

Why not?

I think the only problem with using statement_timeout for this purpose
is that the client connection might die during a long-running
transaction at a point when no statement is currently executing. Tom's
suggested transaction_timeout would be a reasonable way to fix this.
Adnan, if you think this is such a significant problem (I can't say that
I agree), I'd encourage you to submit a patch.

-Neil

Neil Conway wrote:
> adnandursun(at)asrinbilisim(dot)com(dot)tr wrote:
>
>> statement_timeout is not a solution if many processes are
>> waiting the resource.
>
>
> Why not?
>
> I think the only problem with using statement_timeout for this purpose
> is that the client connection might die during a long-running
> transaction at a point when no statement is currently executing. Tom's
> suggested transaction_timeout would be a reasonable way to fix this.
> Adnan, if you think this is such a significant problem (I can't say that
> I agree), I'd encourage you to submit a patch.

I raised this a while back on -hackers:

http://archives.postgresql.org/pgsql-hackers/2005-02/msg00397.php

but did not get much feedback.

Does anyone have comments on that email?

It's a problem that is unlikely to happen in normal operation, but you
do need to deal with it to cover the network failure cases if you have
an otherwise failure-tolerant cluster..

-O

On 5/1/05, adnandursun(at)asrinbilisim(dot)com(dot)tr
<adnandursun(at)asrinbilisim(dot)com(dot)tr> wrote:
> On Sun, 1 May 2005 14:35:37 -0500
> Bruno Wolff III <bruno(at)wolff(dot)to> wrote:
> >On Sun, May 01, 2005 at 19:57:37 +0300,
> > adnandursun(at)asrinbilisim(dot)com(dot)tr wrote:
> >>
> >> Listen Tom, write a client software that releases the
> >> resources / locks that was hold before client power is
> >down
> >> or client connection was lost.
> >
> >If Postgres can tell the connection has been lost then it
> >should roll back the connection.
>
> Yes, but, Can PostgreSQL know which connection is lost or
> live or dead ?
>
> >The problem is that you can't always
> >tell if a connection has been lost. All you can do is
> timeout, either when TCP
> >times out or some other timeout (such as a statment
> timeout) that you set.
>
> You are right, a timeout parameter must be used for that
> on the backend. a client application never find the
> previous instance before it crashed. However more than one
> connection was able to be established to PostgreSQL
> backend..
>
> Statement_timeout is just a escape mechanism for active
> transaction. Imagine; you've started a process to update
> the rows in a table then your PC power was down but you
> have not sent commit or rollback yet..What will happen now
>
If you send the update outside a transaction and...

Option 1) ...the client crashes then the update will commit, i think.
If you don't want that send the update inside a begin/commit block.

Option 2) ...the server crashes the update will rollback.

If you send the update inside a transaction and...

Option 1) ...the client crashes then the update will rollback.
Option 2) ...the server crashes the update will rollback.

Actually, i can't see what's the problem. :)

--
Atentamente,
Jaime Casanova
(DBA: DataBase Aniquilator ;)

Oliver Jowett wrote:
> I raised this a while back on -hackers:
>
> http://archives.postgresql.org/pgsql-hackers/2005-02/msg00397.php
>
> but did not get much feedback.

Perhaps you can interpret silence as consent? :)

> Does anyone have comments on that email?

I wouldn't be opposed to it. It would be different than
statement_timeout, in that we'd be measuring transaction *idle* time,
not total transaction runtime, so perhaps "transaction_idle_timeout" is
a better name than "transaction_timeout". Also, presumably when the
transaction idle timeout fires, we should just rollback the current
transaction, not close the client connection -- so you could potentially
have idle backends sticking around for the full TCP timeout period.
Since they shouldn't be holding any locks I don't see that as a big problem.

-Neil

Jaime Casanova <systemguards(at)gmail(dot)com> writes:
> Actually, i can't see what's the problem. :)

I think the issue is "how long does it take for the rollback to happen?"

While that isn't an unreasonable issue on its face, I think it really
boils down to this: the OP is complaining because he thinks the
connection-loss timeout mandated by the TCP RFCs is too long. Perhaps
the OP knows network engineering far better than the authors of those
RFCs, or perhaps not. I'm not convinced that Postgres ought to provide
a way to second-guess the TCP stack ... this looks to me like "I can't
convince the network software people to provide me an easy way to
override their decisions, so I'll beat up on the database people to
override 'em instead. Perhaps the database people don't know the issues
and can be browbeaten more easily."

regards, tom lane

Neil Conway <neilc(at)samurai(dot)com> writes:
>> Does anyone have comments on that email?

> I wouldn't be opposed to it. It would be different than
> statement_timeout, in that we'd be measuring transaction *idle* time,

We would? Why? Please provide a motivation that justifies the
considerably higher cost to make it count that way, as opposed to
time-since-BEGIN. If the point is to limit the time for which locks
are held, I should think this would actually be *less* desirable than
constraining time-since-BEGIN.

> Also, presumably when the
> transaction idle timeout fires, we should just rollback the current
> transaction, not close the client connection

Certainly ...

> -- so you could potentially
> have idle backends sticking around for the full TCP timeout period.

... but that doesn't necessarily follow. Once we've been motivated to
try to send an error message to the client, the relevant timeouts are
way shorter than they are under connection-idle conditions.

> Since they shouldn't be holding any locks I don't see that as a big problem.

Right, once we've released the transaction the pain grows greatly less.
We are still occupying a backend slot though, so failing sooner has some
value, if there is no doubt the connection is unrecoverable. (But see
my upthread doubts about whether we know that better than the TCP stack
does.)

regards, tom lane

Tom Lane wrote:
> We would? Why? Please provide a motivation that justifies the
> considerably higher cost to make it count that way, as opposed to
> time-since-BEGIN.

The specific scenario this feature is intended to resolve is
idle-in-transaction backends holding on to resources while the network
connection times out; it isn't intended to implement "I never want to
run a transaction that takes more than X seconds to execute." While
long-running transactions aren't usually a great idea, I can certainly
imagine installations in which some transactions might take, say, 30
minutes to execute but the admin would like to timeout idle connections
in less than that amount of time.

As for cost, this feature has zero cost until it is enabled. I would
also guess that setitimer() is reasonably cheap on most kernels,
although let me know if I'm mistaken. If it's sufficiently expensive
that a setitimer() per query is noticeable, then I agree that
setitimer() at BEGIN-time is probably sufficient for most people.

> Once we've been motivated to try to send an error message to the
> client, the relevant timeouts are way shorter than they are under
> connection-idle conditions.

Sorry, yes, I should have been more clear.

-Neil

Tom Lane wrote:

> I'm not convinced that Postgres ought to provide
> a way to second-guess the TCP stack ... this looks to me like "I can't
> convince the network software people to provide me an easy way to
> override their decisions, so I'll beat up on the database people to
> override 'em instead. Perhaps the database people don't know the issues
> and can be browbeaten more easily."

Would you be ok with a patch that allowed configuration of the
TCP_KEEPCNT / TCP_KEEPIDLE / TCP_KEEPINTVL socket options on backend
sockets?

-O

On Mon, 2 May 2005 03:05 pm, Neil Conway wrote:
> Tom Lane wrote:
> > We would? Why? Please provide a motivation that justifies the
> > considerably higher cost to make it count that way, as opposed to
> > time-since-BEGIN.
>
> The specific scenario this feature is intended to resolve is
> idle-in-transaction backends holding on to resources while the network
> connection times out; it isn't intended to implement "I never want to
> run a transaction that takes more than X seconds to execute." While
> long-running transactions aren't usually a great idea, I can certainly
> imagine installations in which some transactions might take, say, 30
> minutes to execute but the admin would like to timeout idle connections
> in less than that amount of time.
>
The two big long running transactions I can think of are VACUUM on a large db,
and there is no way to shorten that time, since to stop wraparound you must vacuum
the whole db.

Backups with pg_dump can run for quite a long time.

I would prefer an idle timeout if it's not costly. Because otherwise estimates need to be
made about how long VACUUM and backup could take, and set the timeout longer. Which
in some senses defeats the purpose of being able to cleanup idle connection quickly.

The VACUUM issue may not be a problem, as if BEGIN is not issued, then the transaction
timeout would probably not be used. But the issues would remain for backups.

Just some thoughts

Regards

Russell Smith

Neil Conway <neilc(at)samurai(dot)com> writes:
> Tom Lane wrote:
>> We would? Why? Please provide a motivation that justifies the
>> considerably higher cost to make it count that way, as opposed to
>> time-since-BEGIN.

> The specific scenario this feature is intended to resolve is
> idle-in-transaction backends holding on to resources while the network
> connection times out; it isn't intended to implement "I never want to
> run a transaction that takes more than X seconds to execute."

[ itch... ] This seems to me to be conflating several distinct issues.
AFAIR the points that have been raised in the thread are:

#1 Defend against loss of connectivity to client
#2 Defend against client sitting idle while holding locks (or just
holding an open transaction and thereby preventing VACUUM cleanup)
#3 Defend against client holding locks unreasonably long, even though
not idle (obviously any such constraint will cause clients to
fail midstream, but perhaps some DBAs will see this as the lesser
of two evils)

I claim that if you have a problem with #1 you ought to go discuss it
with some TCP hackers: you basically want to second-guess the TCP
stack's ideas about appropriate timeouts. Maybe you know what you
are doing or maybe not, but it's not a database-level issue.

#2 is a fair point if you need to cope with poorly-programmed clients,
but I'm not seeing exactly why it has to be measured by "idle time"
rather than "total time". The known cases of this involve client
code that issues a BEGIN and then just sits, so there's no difference.

For point #3, I claim you have to measure total time not idle time
or you'll fail to perceive the problem at all.

> While long-running transactions aren't usually a great idea, I can
> certainly imagine installations in which some transactions might take,
> say, 30 minutes to execute but the admin would like to timeout idle
> connections in less than that amount of time.

The fallacy in that argument is that if you don't like a transaction
that sits idle for 30 minutes, you aren't likely to like ones that hog
the CPU for 30 minutes either. The idle xact is certainly not chewing
more resources than the busy xact. If you have a problem with it, it
has to be along the lines of holding-locks-too-long, and that would
apply just as much to the busy guy.

regards, tom lane

Oliver Jowett <oliver(at)opencloud(dot)com> 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?

regards, tom lane

Russell Smith <mr-russ(at)pws(dot)com(dot)au> writes:
> I would prefer an idle timeout if it's not costly. Because otherwise
> estimates need to be made about how long VACUUM and backup could take,
> and set the timeout longer.

Why? No one has suggested that the same timeout must be applied to
every connection. Clients that are going to do maintenance stuff like
VACUUM could just disable the timeout.

This does bring up thoughts of whether the timeout needs to be a
protected variable (SUSET or higher). I'd argue not, since a
noncooperative client can certainly cause performance issues aplenty
no matter what you try to impose with timeouts.

regards, tom lane

On Mon, 2 May 2005, Tom Lane wrote:

> #1 Defend against loss of connectivity to client
>
> I claim that if you have a problem with #1 you ought to go discuss it
> with some TCP hackers: you basically want to second-guess the TCP
> stack's ideas about appropriate timeouts. Maybe you know what you
> are doing or maybe not, but it's not a database-level issue.

Different applications can have different needs here. For some it's okay
to wait a long time, for others it is not.

The tcp hackers have provided an api for clients to set these values per
socket (setsockopt with TCP_KEEPIDLE and similar (in linux at least)).

My problem with the above setting is that some operations can be in
progress for a long time on the server without generating any tcp/ip
traffic to the client (a non verbose vacuum I guess is such a case). Such
an operation would look like it's idle.

There is an overlap with session and transaction timeouts, most
applications work fine with any of these.

--
/Dennis Björklund

Tom Lane wrote:
> #3 Defend against client holding locks unreasonably long, even though
> not idle

I can't get too excited about this case. If the client is malicious,
this feature is surely insufficient to stop them from consuming a lot of
resources (for example, they could easily drop and then reacquire the
locks every (timeout * 0.9) seconds). And how many DBAs are really going
to want to abort non-malicious clients doing useful work if they happen
to exceed a certain total runtime? Perhaps a motivating example would
help...

> I claim that if you have a problem with #1 you ought to go discuss it
> with some TCP hackers: you basically want to second-guess the TCP
> stack's ideas about appropriate timeouts.

Well, no -- you might want to set a different timeout for PostgreSQL
connections than for other connections. Is there a way to change the
socket timeout for some subset of the processes on the machine without
hacking the client or server source? You might also want to set this
timeout on a more granular basis (e.g. per user, per database, etc.)
Implementing this via setting a socket option (provided it can be done
portably) would be fine with me.

-Neil

Tom Lane wrote:
> Oliver Jowett <oliver(at)opencloud(dot)com> 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?

I can't see how you'd aggregate or discard any of those options without
losing useful tuning knobs.. if you're going to have one, you might as
well have them all.

-O

Neil Conway wrote:

> Is there a way to change the
> socket timeout for some subset of the processes on the machine without
> hacking the client or server source?

The only ways I can see of tuning the TCP idle parameters on Linux are
globally via sysfs, or per-socket via setsockopt().

You could LD_PRELOAD something to wrap accept(), I suppose, but that
seems needlessly ugly..

-O

Neil Conway wrote:
> The specific scenario this feature is intended to resolve is
> idle-in-transaction backends holding on to resources while the
> network connection times out;

I was under the impression that the specific scenario is
busy-in-transaction backends continuing to produce and send data while
the client has disappeared. Why does the backend ignore network errors
and keep sending data?

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut wrote:
> Neil Conway wrote:
>
>>The specific scenario this feature is intended to resolve is
>>idle-in-transaction backends holding on to resources while the
>>network connection times out;
>
>
> I was under the impression that the specific scenario is
> busy-in-transaction backends continuing to produce and send data while
> the client has disappeared. Why does the backend ignore network errors
> and keep sending data?

The scenario I need to deal with is this:

There are multiple nodes, network-separated, participating in a cluster.
One node is selected to talk to a particular postgresql instance (call
this node A).

A starts a transaction and grabs some locks in the course of that
transaction. Then A falls off the network before committing because of a
hardware or network failure. A's connection might be completely idle
when this happens.

The cluster liveness machinery notices that A is dead and selects a new
node to talk to postgresql (call this node B). B resumes the work that A
was doing prior to failure.

B has to wait for any locks held by A to be released before it can make
any progress.

Without some sort of tunable timeout, it could take a very long time (2+
hours by default on Linux) before A's connection finally times out and
releases the locks.

-O

On Mon, 02 May 2005 12:05:45 +1000
Neil Conway <neilc(at)samurai(dot)com> wrote:
>adnandursun(at)asrinbilisim(dot)com(dot)tr wrote:
>> statement_timeout is not a solution if many processes
>are
>> waiting the resource.
>
>Why not?

Imagine a process locked some rows to update and process
codes like that ;

-- Sample Client Codes here :

1. Start a Transaction
2. Set statement_timeout to 10000 or any value..
3. Update the rows
* after update is completed the connection was lost
and now commit keyword couldnt be sent
4. send commit to postgresql

Above, because "update" is completed the
statement_timeout is not effected anymore to cancel
query..And others processes that waits same resources /
rows are waiting now...

>I think the only problem with using statement_timeout for
>this purpose is that the client connection might die
>during a long-running transaction at a point when no
>statement is currently executing. Tom's suggested
>transaction_timeout would be a reasonable way to fix this.
>Adnan, if you think this is such a significant problem (I
>can't say that I agree), I'd encourage you to submit a
>patch.

Ok Neil, a transaction_timeout parameters solve this, but
this is worst case.. some ppl uses MSADO conneciton
component and ADO conneciton has an attributes that send
"start transaction" after a commit or sends "start
transaction" after a rollback so, evertime has a
transaction on conneciton / session..

Adnan DURSUN
ASRIN Bilişim Hiz.Ltd.

On Sun, 1 May 2005 23:08:39 -0500
Jaime Casanova <systemguards(at)gmail(dot)com> wrote:
>On 5/1/05, adnandursun(at)asrinbilisim(dot)com(dot)tr
><adnandursun(at)asrinbilisim(dot)com(dot)tr> wrote:
>> On Sun, 1 May 2005 14:35:37 -0500
>> Bruno Wolff III <bruno(at)wolff(dot)to> wrote:
>> >On Sun, May 01, 2005 at 19:57:37 +0300,
>> > adnandursun(at)asrinbilisim(dot)com(dot)tr wrote:
>> >>
>> >> Listen Tom, write a client software that releases the
>> >> resources / locks that was hold before client power
>is
>> >down
>> >> or client connection was lost.
>> >
>> >If Postgres can tell the connection has been lost then
>it
>> >should roll back the connection.
>>
>> Yes, but, Can PostgreSQL know which connection is lost
>or
>> live or dead ?
>>
>> >The problem is that you can't always
>> >tell if a connection has been lost. All you can do is
>> timeout, either when TCP
>> >times out or some other timeout (such as a statment
>> timeout) that you set.
>>
>> You are right, a timeout parameter must be used for
>that
>> on the backend. a client application never find the
>> previous instance before it crashed. However more than
>one
>> connection was able to be established to PostgreSQL
>> backend..
>>
>> Statement_timeout is just a escape mechanism for
>active
>> transaction. Imagine; you've started a process to update
>> the rows in a table then your PC power was down but you
>> have not sent commit or rollback yet..What will happen
>now
>>
>If you send the update outside a transaction and...
>
>Option 1) ...the client crashes then the update will
>commit, i think.
>If you don't want that send the update inside a
>begin/commit block.
>
>Option 2) ...the server crashes the update will rollback.
>
>
>If you send the update inside a transaction and...
>
>Option 1) ...the client crashes then the update will
>rollback.
>Option 2) ...the server crashes the update will rollback.
>
>Actually, i can't see what's the problem. :)

No, process waits until it is completed..

Adnan DURSUN
ASRIN Bili?im Hiz.Ltd.

On Mon, 02 May 2005 00:25:33 -0400
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>Jaime Casanova <systemguards(at)gmail(dot)com> writes:
>> Actually, i can't see what's the problem. :)
>
>I think the issue is "how long does it take for the
>rollback to happen?"
>
> ....so I'll beat up on the database people to override
'em instead. Perhaps the database people don't know the
issues and can be browbeaten more easily."

Never, How can ppl change TCP timeouts because PostgreSQL
can run on different OS. I dont want to browbeat you. All
thing i want is to provide a good mechanism to slove this
problem on PostgreSQL like others databases etc. Oracle,
MSSQL, PervasiveSQL and so...

Adnan DURSUN
ASRIN Bilişim Hiz.Ltd.

On Mon, 02 May 2005 01:35:14 -0400
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

>[ itch... ] This seems to me to be conflating several
>distinct issues.
>AFAIR the points that have been raised in the thread are:
>
>#1 Defend against loss of connectivity to client

>#2 Defend against client sitting idle while holding locks
(or just holding an open transaction and thereby
preventing VACUUM cleanup)

>#3 Defend against client holding locks unreasonably long,
>even though not idle (obviously any such constraint will
cause clients to
> fail midstream, but perhaps some DBAs will see this as
the lesser of two evils)

>I claim that if you have a problem with
>#1 you ought to go discuss it with some TCP hackers: you
basically want to second-guess
>the TCP stack's ideas about appropriate timeouts. Maybe
you know what you
>are doing or maybe not, but it's not a database-level
issue.

>#2 is a fair point if you need to cope with
poorly-programmed clients,
>but I'm not seeing exactly why it has to be measured by
"idle time"
>rather than "total time". The known cases of this involve
>client code that issues a BEGIN and then just sits, so
there's no
>difference.

Ok, the client sent BEGIN and then connection was lost.
Does it means that the client sits ?

Adnan DURSUN
ASRIN Bilişim Hiz.Ltd.

On Mon, 02 May 2005 16:07:07 +1000
Neil Conway <neilc(at)samurai(dot)com> wrote:

>> I claim that if you have a problem with #1 you ought to
>go discuss it with some TCP hackers: you basically want to
second-guess the TCP
>> stack's ideas about appropriate timeouts.
>
>Well, no -- you might want to set a different timeout for
>PostgreSQL connections than for other connections. Is
>there a way to change the socket timeout for some subset
>of the processes on the machine without hacking the client
>or server source? You might also want to set this timeout
>on a more granular basis (e.g. per user, per database,
>etc.) Implementing this via setting a socket option
>(provided it can be done portably) would be fine with me.

You are right Neil.. A machine can make more than one
connection to same server. Some of them are database
connection and some only to OS conneciton. For Example :
Oracle has an parameter to find and kill dead sessions..

PostgreSQL can run on different OS.I think this is solved
database level rather than OS level.

Best Regards,

Adnan DURSUN
ASRIN Bili?im Hiz.Ltd.

On Mon, 2 May 2005 10:11:40 +0200
Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:

>I was under the impression that the specific scenario is
>busy-in-transaction backends continuing to produce and
>send data while the client has disappeared. Why does the
backend ignore network errors
>and keep sending data?

Yes, I think PostgreSQL doesnt know whether client is dead
or live ? Or It knows it but, it keeps to send data..

Best Regards,

Adnan DURSUN
ASRIN Bili?im Hiz.Ltd.

On E, 2005-05-02 at 01:35 -0400, Tom Lane wrote:
> Neil Conway <neilc(at)samurai(dot)com> writes:
> > Tom Lane wrote:
> >> We would? Why? Please provide a motivation that justifies the
> >> considerably higher cost to make it count that way, as opposed to
> >> time-since-BEGIN.
>
> > The specific scenario this feature is intended to resolve is
> > idle-in-transaction backends holding on to resources while the network
> > connection times out; it isn't intended to implement "I never want to
> > run a transaction that takes more than X seconds to execute."
>
> [ itch... ] This seems to me to be conflating several distinct issues.
> AFAIR the points that have been raised in the thread are:
>
> #1 Defend against loss of connectivity to client
> #2 Defend against client sitting idle while holding locks (or just
> holding an open transaction and thereby preventing VACUUM cleanup)
> #3 Defend against client holding locks unreasonably long, even though
> not idle (obviously any such constraint will cause clients to
> fail midstream, but perhaps some DBAs will see this as the lesser
> of two evils)
>
> I claim that if you have a problem with #1 you ought to go discuss it
> with some TCP hackers: you basically want to second-guess the TCP
> stack's ideas about appropriate timeouts. Maybe you know what you
> are doing or maybe not, but it's not a database-level issue.

Well, I've had problems with clients which resolve DB timeouts by
closing the current connection and establish a new one.

If it is actual DB timeout, then it all is ok, the server soon notices
that the client connection is closed and kills itself.

Problems happen when the timeout is caused by actual network problems -
when i have 300 clients (server's max_connections=500) which try to
reconnect after network outage, only 200 of them can do so as the server
is holding to 300 old connections.

In my case this has nothing to do with locks or transactions.

It would be nice if I coud st up some timeut using keepalives (like ssh-
s ProtocoKeepalives") and use similar timeouts on client and server.

--
Hannu Krosing <hannu(at)skype(dot)net>

On Mon, 02 May 2005 13:59:21 +0300
Hannu Krosing <hannu(at)skype(dot)net> wrote:
>On E, 2005-05-02 at 01:35 -0400, Tom Lane wrote:

>Well, I've had problems with clients which resolve DB
timeouts by
>closing the current connection and establish a new one.
>
>If it is actual DB timeout, then it all is ok, the server
>soon notices that the client connection is closed and
kills itself.
>
>Problems happen when the timeout is caused by actual
>network problems - when i have 300 clients (server's
max_connections=500)
>which try to reconnect after network outage, only 200 of
them can do so
>as the server is holding to 300 old connections.
>
>In my case this has nothing to do with locks or
transactions.

Yes, but if you would have locks then same problem
occours..

>It would be nice if I coud st up some timeut using
keepalives (like ssh-s ProtocoKeepalives") and >use similar
timeouts on client and server.

Diffrent ppl use diffrent OS.

Adnan DURSUN
ASRIN Bilişim Hiz.Ltd.

On Sun, 01 May 2005 22:23:19 +0300
Hannu Krosing <hannu(at)skype(dot)net> wrote:
>On P, 2005-05-01 at 11:37 -0400, Tom Lane wrote:
>Im my case all ttransactions were implicit one command
functon calls
>("select * from dbfunc()"), so transaction timeout would
not help.
>
>probably the only way for server to detect stale
connections would be
>sending/receiving some kind of keepalives.

...and then clear stale connection. This would be nice
feature for PostgreSQL

Best Regards,

Adnan DURSUN
ASRIN Bilişim Hiz.Ltd.

Hi,

-- Dennis Bjorklund <db(at)zigo(dot)dhs(dot)org> wrote:

> The tcp hackers have provided an api for clients to set these values per
> socket (setsockopt with TCP_KEEPIDLE and similar (in linux at least)).

you can use SO_KEEPALIVE:

[...] SO_KEEPALIVE enables
the periodic transmission of messages on a connected socket. Should the
connected party fail to respond to these messages, the connection is con-
sidered broken and processes using the socket are notified via a SIGPIPE
signal when attempting to send data.

For me it seems to be a good idea to support this. For the applications
(server, client) it is transparent, but notices a cutted or broken network
connection faster then before...

But even with this you only realise that the connection is gone when sending
something, AFAIK.

Ciao
Alvar

--
Alvar C.H. Freude -- http://alvar.a-blast.org/
http://odem.org/
http://www.assoziations-blaster.de/info/Hommingberger-Gepardenforelle.html
http://www.assoziations-blaster.de/

On Mon, 02 May 2005 13:32:18 +0200
Alvar Freude <alvar(at)a-blast(dot)org> wrote:
>Hi,
>
>-- Dennis Bjorklund <db(at)zigo(dot)dhs(dot)org> wrote:
>
>> The tcp hackers have provided an api for clients to set
>these values per
>> socket (setsockopt with TCP_KEEPIDLE and similar (in
>linux at least)).
>
>you can use SO_KEEPALIVE:
>
> [...] SO_KEEPALIVE enables
> the periodic transmission of messages on a connected
socket. Should the
> connected party fail to respond to these messages, the
connection is con-
> sidered broken and processes using the socket are
notified via a SIGPIPE
> signal when attempting to send data.
>
>
>For me it seems to be a good idea to support this. For the
applications
>(server, client) it is transparent, but notices a cutted
or broken network
>connection faster then before...
>
>But even with this you only realise that the connection is
>gone when sending something, AFAIK.

So this means, If client does never try to send data the
resources would be going to be held.
I think it is not a good solution to find zombie / dead
connection and clear them..

Best Regards,

Adnan DURSUN
ASRIN Bilişim Hiz.Ltd.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> I was under the impression that the specific scenario is
> busy-in-transaction backends continuing to produce and send data while
> the client has disappeared. Why does the backend ignore network errors
> and keep sending data?

There are a couple of reasons behind that:

1. In terms of resources held against the rest of the database,
a query doing SELECT is hardly likely to be your worst problem.
Queries doing UPDATE, VACUUM, etc are holding stronger locks and
probably chewing at least as many resources; but they aren't doing
any client I/O (until the command tag at the very end) and so are
not going to detect client connection loss significantly earlier
than they do now anyway.

2. Because of the lack of output, the behavior of non-SELECT queries
is that they complete before the backend notices client connection
loss. This is nice because it makes it simpler to reason about what
will happen. Currently the backend guarantees the same for SELECT
queries, which is also nice, since we have plenty of SELECT queries
with side-effects (consider user function invocation). Do we really
want to give that up?

3. If we error out on send() failure then we have turned a probable
failure into certain failure, because we will have lost message-boundary
sync with the client --- in other words the error might as well be a
FATAL one. This seems like it might be overkill.

4. Erroring out in the low-level routines is trickier than it looks;
in particular, after you elog() your complaint, elog.c is going to
come right back to you with an error message to send to the client.
Not having this turn into an infinite recursion is a bit ticklish.
Making sure it stays working is even trickier, considering what a
seldom-tested code path it's going to be.

regards, tom lane

Oliver Jowett <oliver(at)opencloud(dot)com> writes:
> The scenario I need to deal with is this:

> There are multiple nodes, network-separated, participating in a cluster.
> One node is selected to talk to a particular postgresql instance (call
> this node A).

> A starts a transaction and grabs some locks in the course of that
> transaction. Then A falls off the network before committing because of a
> hardware or network failure. A's connection might be completely idle
> when this happens.

> The cluster liveness machinery notices that A is dead and selects a new
> node to talk to postgresql (call this node B). B resumes the work that A
> was doing prior to failure.

> B has to wait for any locks held by A to be released before it can make
> any progress.

> Without some sort of tunable timeout, it could take a very long time (2+
> hours by default on Linux) before A's connection finally times out and
> releases the locks.

Wouldn't it be reasonable to expect the "cluster liveness machinery" to
notify the database server's kernel that connections to A are now dead?
I find it really unconvincing to suppose that the above problem should
be solved at the database level.

regards, tom lane

On Mon, 2 May 2005, Hannu Krosing wrote:

> Well, I've had problems with clients which resolve DB timeouts by
> closing the current connection and establish a new one.
>
> If it is actual DB timeout, then it all is ok, the server soon notices
> that the client connection is closed and kills itself.
>
> Problems happen when the timeout is caused by actual network problems -
> when i have 300 clients (server's max_connections=500) which try to
> reconnect after network outage, only 200 of them can do so as the server
> is holding to 300 old connections.
>
> In my case this has nothing to do with locks or transactions.
>
> It would be nice if I coud st up some timeut using keepalives (like ssh-
> s ProtocoKeepalives") and use similar timeouts on client and server.

FWIW, I've been bitten by this problem twice with other applications.

1. We had a DB2 database with clients running in other computers in the
network. A faulty switch caused random network outages. If the connection
timed out and the client was unable to send it's request to the server,
the client would notice that the connection was down, and open a new one.
But the server never noticed that the connection was dead. Eventually,
the maximum number of connections was reached, and the administrator had
to kill all the connections manually.

2. We had a custom client-server application using TCP across a network.
There was stateful firewall between the server and the clients that
dropped the connection at night when there was no activity. After a
couple of days, the server reached the maximum number of threads on the
platform and stopped accepting new connections.

In case 1, the switch was fixed. If another switch fails, the same will
happen again. In case 2, we added an application-level heartbeat that
sends a dummy message from server to client every 10 minutes.

TCP keep-alive with a small interval would have saved the day in both
cases. Unfortunately the default interval must be >= 2 hours, according
to RFC1122.

On most platforms, including Windows and Linux, the TCP keep-alive
interval can't be set on a per-connection basis. The ideal solution would
be to modify the operating system to support it.

What we can do in PostgreSQL is to introduce an application-level
heartbeat. A simple "Hello world" message sent from server to client that
the client would ignore would do the trick.

- Heikki

On Mon, 2 May 2005 18:47:14 +0300 (EEST)
Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:

>FWIW, I've been bitten by this problem twice with other
>applications.
>
>1. We had a DB2 database with clients running in other
>computers in the network. A faulty switch caused random
>network outages. If the connection timed out and the
>client was unable to send it's request to the server, the
>client would notice that the connection was down, and open
>a new one. But the server never noticed that the
>connection was dead. Eventually, the maximum number of
>connections was reached, and the administrator had to kill
>all the connections manually.

Are you pleased from this feature on DB2 ? I think you
will say no :-)

>2. We had a custom client-server application using TCP
>across a network. There was stateful firewall between the
>server and the clients that dropped the connection at
>night when there was no activity. After a couple of days,
>the server reached the maximum number of threads on the
>platform and stopped accepting new connections.

Yes, because your firewall drops only connectiona between
clients and firewall, not database.

>In case 1, the switch was fixed. If another switch fails,
>the same will happen again. In case 2, we added an
>application-level heartbeat that sends a dummy message
>from server to client every 10 minutes.
>
>TCP keep-alive with a small interval would have saved the
>day in both cases. Unfortunately the default interval must
>be >= 2 hours, according to RFC1122.

Yes..

>On most platforms, including Windows and Linux, the TCP
>keep-alive interval can't be set on a per-connection
>basis. The ideal solution would be to modify the operating
>system to support it.

How will we do this ?

>What we can do in PostgreSQL is to introduce an
>application-level heartbeat. A simple "Hello world"
>message sent from server to client that the client would
>ignore would do the trick.

This couldnt be not forgetten that a clients can have more
than one connection to database and one of them is lost..

Best Regards,

Adnan DURSUN
ASRIN Bilişim Hiz.Ltd.

On 2005-05-02, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> While that isn't an unreasonable issue on its face, I think it really
> boils down to this: the OP is complaining because he thinks the
> connection-loss timeout mandated by the TCP RFCs is too long. Perhaps
> the OP knows network engineering far better than the authors of those
> RFCs, or perhaps not. I'm not convinced that Postgres ought to provide
> a way to second-guess the TCP stack ...

Speaking as someone who _does_ know network engineering, I would say that
yes, Postgres absolutely should do so.

The TCP keepalive timeout _is not intended_ to do this job; virtually
every application-level protocol manages its own timeouts independently
of TCP. (The few exceptions, such as telnet, tend to be purely interactive
protocols that rely on the user to figure out that something got stuck.)

One way to handle this is to have an option, set by the client, that
causes the server to send some ignorable message after a given period
of time idle while waiting for the client. If the idleness was due to
network partitioning or similar failure, then this ensures that the
connection breaks within a known time. This is safer than simply having
the backend abort after a given idle period.

If you want comparisons from other protocols, just look around - SMTP,
ssh, IRC, BGP, NNTP, FTP, and many, many more protocols all use timeouts
(or in some cases keepalive messages) with intervals much shorter than the
TCP keepalive timeout itself.

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

On E, 2005-05-02 at 18:47 +0300, Heikki Linnakangas wrote:
> On Mon, 2 May 2005, Hannu Krosing wrote:

> > It would be nice if I coud st up some timeut using keepalives (like ssh-
> > s ProtocoKeepalives") and use similar timeouts on client and server.
>
> FWIW, I've been bitten by this problem twice with other applications.
>
> 1. We had a DB2 database with clients running in other computers in the
> network. A faulty switch caused random network outages. If the connection
> timed out and the client was unable to send it's request to the server,
> the client would notice that the connection was down, and open a new one.
> But the server never noticed that the connection was dead. Eventually,
> the maximum number of connections was reached, and the administrator had
> to kill all the connections manually.
>
> 2. We had a custom client-server application using TCP across a network.
> There was stateful firewall between the server and the clients that
> dropped the connection at night when there was no activity. After a
> couple of days, the server reached the maximum number of threads on the
> platform and stopped accepting new connections.
>
> In case 1, the switch was fixed. If another switch fails, the same will
> happen again. In case 2, we added an application-level heartbeat that
> sends a dummy message from server to client every 10 minutes.
>
> TCP keep-alive with a small interval would have saved the day in both
> cases. Unfortunately the default interval must be >= 2 hours, according
> to RFC1122.
>
> On most platforms, including Windows and Linux, the TCP keep-alive
> interval can't be set on a per-connection basis. The ideal solution would
> be to modify the operating system to support it.

Yep. I think this could be done for (our instance of) linux, but getting
it into mainstream kernel, and then into all popular distros is a lot of
effort.

Going the ssh way (protocol level keepalives) might be way simpler.

> What we can do in PostgreSQL is to introduce an application-level
> heartbeat. A simple "Hello world" message sent from server to client that
> the client would ignore would do the trick.

Actually we would need a round-trip indicator (some there-and-back
message: A: do you copy 42 --> B: yes I copy 42), and not just send. The
difficult part is what to do when one side happens to send the keepalive
in the middle of actual data transfer ?

move to packet oriented connections (UDP) and make different packet
types independant of each other?

--
Hannu Krosing <hannu(at)skype(dot)net>

Tom Lane wrote:

> Wouldn't it be reasonable to expect the "cluster liveness machinery" to
> notify the database server's kernel that connections to A are now dead?

No, because it's a node-level liveness test, not a machine-level
liveness. It's possible that all that happened is the node's VM crashed.
The clustering is all done in userspace.

-O

Tom Lane wrote:

> Wouldn't it be reasonable to expect the "cluster liveness machinery" to
> notify the database server's kernel that connections to A are now dead?
> I find it really unconvincing to suppose that the above problem should
> be solved at the database level.

Actually, if you were to implement this as you suggest, you either put
full-blown group communication in the kernel (ow, no thanks!) or you
implement a system where the DB server's kernel has a heartbeat to each
peer (e.g. A) and if that heartbeat stops, it kills the corresponding
connections.

But that functionality already exists: it is SO_KEEPALIVE.

(I think we're arguing in circles here..)

-O

Tom Lane wrote:
> Oliver Jowett <oliver(at)opencloud(dot)com> 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

On 5/2/05, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> What we can do in PostgreSQL is to introduce an application-level
> heartbeat. A simple "Hello world" message sent from server to client that
> the client would ignore would do the trick.

Hmm, a quick-and-dirty implementation could be that a client issues
"LISTEN heartbeat;" command, and there would be other client issuing
"NOTIFY heartbeat;" every few minutes. I am not sure but this would
probably make server send out these messages to the client, regardless
of whether the client is doing something or not. Again, I am not sure.
Ah, and probably so many NOTIFY messages wouldn't be very nice for
system tables.

Regards,
Dawid

Hannu Krosing <hannu(at)skype(dot)net> writes:
>> What we can do in PostgreSQL is to introduce an application-level
>> heartbeat. A simple "Hello world" message sent from server to client that
>> the client would ignore would do the trick.

> Actually we would need a round-trip indicator (some there-and-back
> message: A: do you copy 42 --> B: yes I copy 42), and not just send.

No, a one-way message is sufficient. The reason is that once we've
asked the TCP stack to send something, the customary timeouts before
declaring the connection dead are far shorter than they are for
keepalives. Also see the point that we must not assume that the
client-side library can get control on short notice (or indeed any
notice).

I am a tad worried about the possibility that if the client does nothing
for long enough, the TCP output buffer will fill causing the backend to
block at send(). A permanently blocked backend is bad news from a
performance point of view (it degrades the sinval protocol for everyone
else).

regards, tom lane

On Tue, 3 May 2005, Tom Lane wrote:

> I am a tad worried about the possibility that if the client does nothing
> for long enough, the TCP output buffer will fill causing the backend to
> block at send(). A permanently blocked backend is bad news from a
> performance point of view (it degrades the sinval protocol for everyone
> else).

Do you mean this scenario:

1. client application doesn't empty its receive buffer (doesn't call
read)
2. server keeps sending data
3. client receive buffer fills
4. server send buffer fills
5. server send blocks.

Unfortunately there's no way to tell if the client is misbehaving or the
network connection is slow or the client is too busy to handle the data
fast enough.

I guess we could increase the send buffer (can it be set per-connection?),
but that only delays the problem.

Does statement_timeout fire on that scenario? How about the new
transaction_timeout option discussed in other threads?

- Heikki

Heikki Linnakangas <hlinnaka(at)iki(dot)fi> writes:
> Does statement_timeout fire on that scenario? How about the new
> transaction_timeout option discussed in other threads?

Probably neither, since very likely you aren't in a transaction at all.
I'd not expect the server to send these messages except when it's been
idle for awhile, so statement_timeout is certainly irrelevant.

BTW, the upthread proposal of just dropping the message (which is what
O_NONBLOCK would do) doesn't work; it will lose encryption sync on SSL
connections.

regards, tom lane

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

Andrew - Supernews wrote:
> On 2005-05-01, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> > The problem, as I understand it, is that if you have a long-running
> > query and the client process disappears, the query keeps running and
> > holds whatever resources it may have until it finishes. In fact, it
> > keeps sending data to the client and keeps ignoring the SIGPIPE it gets
> > (in case of a Unix-domain socket connection).
>
> Ignoring the SIGPIPE is exactly the right thing to do.
>
> What's _not_ a good idea is ignoring the EPIPE error from write(), which
> seems to currently be reported via ereport(COMMERROR) which doesn't try
> and abort the query as far as I can tell.

Where are you seeing this? I looked from PostgresMain() to
ReadCommand() to SocketBackend() to pq_getbyte() which returns EOF, and
PostgresMain checks that and does a proc_exit(0).

I think the main problem is that a long-running query never tries to
interact with the client during the query.

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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Andrew - Supernews wrote:
>> What's _not_ a good idea is ignoring the EPIPE error from write(), which
>> seems to currently be reported via ereport(COMMERROR) which doesn't try
>> and abort the query as far as I can tell.

> Where are you seeing this? I looked from PostgresMain() to
> ReadCommand() to SocketBackend() to pq_getbyte() which returns EOF, and
> PostgresMain checks that and does a proc_exit(0).

It sounds like you were following the input-from-client logic. Andrew
is complaining about the output-to-client side.

We deliberately don't abort on write-to-client failure. There have
been periodic discussions about changing that, but I'm not convinced
that the advocates for a change have made a good case. Right now,
it's predictable that the backend only fails due to loss of connection
when it waits for a new command. The behavior would become much less
predictable if we allowed write failure to abort the query. As an
example: whether an UPDATE command completes might depend on whether
any invoked triggers try to issue NOTICEs.

regards, tom lane

Where are we on this? I do think it solves a problem that some are
having, and it seems it would detect a disconnected client and abort a
long running query.

I am not very excited about adding four more GUC variables, and I am
thinking we could just have it use the OS defaults and see if we need
more later, so that would add only one GUC.

It compiles/tests fine on my BSD system.

---------------------------------------------------------------------------

Oliver Jowett wrote:
> Tom Lane wrote:
> > Oliver Jowett <oliver(at)opencloud(dot)com> 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 -0000 1.315
> --- doc/src/sgml/runtime.sgml 3 May 2005 01:44:02 -0000
> ***************
> *** 889,894 ****
> --- 889,961 ----
> </listitem>
> </varlistentry>
>
> + <varlistentry id="guc-tcp-keepalives" xreflabel="tcp_keepalives">
> + <term><varname>tcp_keepalives</varname> (<type>boolean</type>)</term>
> + <indexterm>
> + <primary><varname>tcp_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">
> + <term><varname>tcp_keepalives_idle</varname> (<type>integer</type>)</term>
> + <indexterm>
> + <primary><varname>tcp_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 <varname>tcp_keepalives</varname> option is enabled.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry id="guc-tcp-keepalives-interval" xreflabel="tcp_keepalives_interval">
> + <term><varname>tcp_keepalives_interval</varname> (<type>integer</type>)</term>
> + <indexterm>
> + <primary><varname>tcp_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 <varname>tcp_keepalives</varname> option is enabled.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry id="guc-tcp-keepalives-count" xreflabel="tcp_keepalives_count">
> + <term><varname>tcp_keepalives_count</varname> (<type>integer</type>)</term>
> + <indexterm>
> + <primary><varname>tcp_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. This option is ignored for
> + connections made via a Unix-domain socket, and will have no effect
> + unless the <varname>tcp_keepalives</varname> option is enabled.
> + </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 -c -r1.176 pqcomm.c
> *** src/backend/libpq/pqcomm.c 22 Feb 2005 04:35:57 -0000 1.176
> --- src/backend/libpq/pqcomm.c 3 May 2005 01:44:03 -0000
> ***************
> *** 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
> ***************
> *** 577,582 ****
> --- 577,583 ----
> if (!IS_AF_UNIX(port->laddr.addr.ss_family))
> {
> int on;
> + socklen_t size;
>
> #ifdef TCP_NODELAY
> on = 1;
> ***************
> *** 587,599 ****
> return STATUS_ERROR;
> }
> #endif
> ! on = 1;
> ! if (setsockopt(port->sock, SOL_SOCKET, SO_KEEPALIVE,
> ! (char *) &on, sizeof(on)) < 0)
> {
> ! elog(LOG, "setsockopt(SO_KEEPALIVE) failed: %m");
> return STATUS_ERROR;
> }
> }
>
> return STATUS_OK;
> --- 588,645 ----
> return STATUS_ERROR;
> }
> #endif
> !
> ! if (pq_setkeepalives(tcp_keepalives, port) != STATUS_OK)
> ! return STATUS_ERROR;
> !
> ! /* Grab default keepalive values, then apply
> ! * our GUC settings.
> ! */
> ! #ifdef TCP_KEEPIDLE
> ! if (getsockopt(port->sock, SOL_TCP, TCP_KEEPIDLE,
> ! (char *) &port->default_keepalives_idle, &size) < 0)
> ! {
> ! elog(LOG, "getsockopt(TCP_KEEPIDLE) failed: %m");
> ! return STATUS_ERROR;
> ! }
> ! #else
> ! port->default_keepalives_idle = 0;
> ! #endif
> !
> ! #ifdef TCP_KEEPINTVL
> ! if (getsockopt(port->sock, SOL_TCP, TCP_KEEPINTVL,
> ! (char *) &port->default_keepalives_interval, &size) < 0)
> {
> ! elog(LOG, "getsockopt(TCP_KEEPINTVL) failed: %m");
> return STATUS_ERROR;
> }
> + #else
> + port->default_keepalives_idle = 0;
> + #endif
> +
> + #ifdef TCP_KEEPCNT
> + if (getsockopt(port->sock, SOL_TCP, TCP_KEEPCNT,
> + (char *) &port->default_keepalives_count, &size) < 0)
> + {
> + elog(LOG, "getsockopt(TCP_KEEPCNT) failed: %m");
> + return STATUS_ERROR;
> + }
> + #else
> + port->default_keepalives_idle = 0;
> + #endif
> +
> + /* 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 ****
> --- 1204,1305 ----
> /* in non-error case, copy.c will have emitted the terminator line */
> DoingCopyOut = false;
> }
> +
> + int
> + pq_setkeepalives(bool onoff, Port *port)
> + {
> + int on = (onoff ? 1 : 0);
> +
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return STATUS_OK;
> +
> + if (setsockopt(port->sock, SOL_SOCKET, SO_KEEPALIVE,
> + (char *) &on, sizeof(on)) < 0)
> + {
> + elog(LOG, "setsockopt(SO_KEEPALIVE) failed: %m");
> + return STATUS_ERROR;
> + }
> +
> + return STATUS_OK;
> + }
> +
> + int
> + pq_setkeepalivesidle(int idle, Port *port)
> + {
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return STATUS_OK;
> +
> + #ifdef TCP_KEEPIDLE
> + if (idle == 0)
> + idle = port->default_keepalives_idle;
> +
> + if (setsockopt(port->sock, SOL_TCP, TCP_KEEPIDLE,
> + (char *) &idle, sizeof(idle)) < 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPIDLE) failed: %m");
> + return STATUS_ERROR;
> + }
> + #else
> + if (idle != 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPIDLE) not supported");
> + return STATUS_ERROR;
> + }
> + #endif
> +
> + return STATUS_OK;
> + }
> +
> + int
> + pq_setkeepalivesinterval(int interval, Port *port)
> + {
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return STATUS_OK;
> +
> + #ifdef TCP_KEEPINTVL
> + if (interval == 0)
> + interval = port->default_keepalives_interval;
> +
> + if (setsockopt(port->sock, SOL_TCP, TCP_KEEPINTVL,
> + (char *) &interval, sizeof(interval)) < 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPINTVL) failed: %m");
> + return STATUS_ERROR;
> + }
> + #else
> + if (interval != 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPINTVL) not supported");
> + return STATUS_ERROR;
> + }
> + #endif
> +
> + return STATUS_OK;
> + }
> +
> + int
> + pq_setkeepalivescount(int count, Port *port)
> + {
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return STATUS_OK;
> +
> + #ifdef TCP_KEEPCNT
> + if (count == 0)
> + count = port->default_keepalives_count;
> +
> + if (setsockopt(port->sock, SOL_TCP, TCP_KEEPCNT,
> + (char *) &count, sizeof(count)) < 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPCNT) failed: %m");
> + return STATUS_ERROR;
> + }
> + #else
> + if (count != 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPCNT) not supported");
> + return STATUS_ERROR;
> + }
> + #endif
> +
> + return STATUS_OK;
> + }
> Index: src/backend/utils/misc/guc.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
> retrieving revision 1.261
> diff -c -r1.261 guc.c
> *** src/backend/utils/misc/guc.c 1 May 2005 18:56:19 -0000 1.261
> --- src/backend/utils/misc/guc.c 3 May 2005 01:44:08 -0000
> ***************
> *** 113,118 ****
> --- 113,125 ----
> static bool assign_transaction_read_only(bool newval, bool doit, GucSource source);
> static const char *assign_canonical_path(const char *newval, bool doit, GucSource source);
>
> + static bool assign_tcp_keepalives(bool newval, bool doit, GucSource source);
> + static bool assign_tcp_keepalives_idle(int newval, bool doit, GucSource source);
> + static bool assign_tcp_keepalives_interval(int newval, bool doit, GucSource source);
> + static bool assign_tcp_keepalives_count(int newval, bool doit, GucSource source);
> + static const char *show_tcp_keepalives_idle(void);
> + static const char *show_tcp_keepalives_interval(void);
> + static const char *show_tcp_keepalives_count(void);
>
> /*
> * GUC option variables that are exported from this module
> ***************
> *** 154,159 ****
> --- 161,170 ----
> char *IdentFileName;
> char *external_pid_file;
>
> + bool tcp_keepalives = true;
> + int tcp_keepalives_idle;
> + int tcp_keepalives_interval;
> + int tcp_keepalives_count;
>
> /*
> * These variables are all dummies that don't do anything, except in some
> ***************
> *** 860,865 ****
> --- 871,885 ----
> #endif
> },
>
> + {
> + {"tcp_keepalives", PGC_USERSET, CLIENT_CONN_OTHER,
> + gettext_noop("Use keepalives on client TCP connections."),
> + NULL,
> + },
> + &tcp_keepalives,
> + true, assign_tcp_keepalives, NULL
> + },
> +
> /* End-of-list marker */
> {
> {NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL
> ***************
> *** 1333,1338 ****
> --- 1353,1387 ----
> BLCKSZ, BLCKSZ, BLCKSZ, NULL, NULL
> },
>
> + {
> + {"tcp_keepalives_idle", PGC_USERSET, CLIENT_CONN_OTHER,
> + gettext_noop("Seconds between issuing TCP keepalives."),
> + gettext_noop("A value of 0 uses the system default."),
> + },
> + &tcp_keepalives_idle,
> + 0, 0, INT_MAX, assign_tcp_keepalives_idle, show_tcp_keepalives_idle
> + },
> +
> + {
> + {"tcp_keepalives_interval", PGC_USERSET, CLIENT_CONN_OTHER,
> + gettext_noop("Seconds between TCP keepalive retransmits."),
> + gettext_noop("A value of 0 uses the system default."),
> + },
> + &tcp_keepalives_interval,
> + 0, 0, INT_MAX, assign_tcp_keepalives_interval, show_tcp_keepalives_interval
> + },
> +
> + {
> + {"tcp_keepalives_count", PGC_USERSET, CLIENT_CONN_OTHER,
> + gettext_noop("Maximum number of TCP keepalive retransmits."),
> + gettext_noop("This controls the number of consecutive keepalive retransmits that can be "
> + "lost before a connection is considered dead. A value of 0 uses the "
> + "system default."),
> + },
> + &tcp_keepalives_count,
> + 0, 0, INT_MAX, assign_tcp_keepalives_count, show_tcp_keepalives_count
> + },
> +
> /* End-of-list marker */
> {
> {NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL
> ***************
> *** 5677,5681 ****
> --- 5726,5802 ----
> return newval;
> }
>
> + static bool
> + assign_tcp_keepalives(bool newval, bool doit, GucSource source)
> + {
> + if (doit && MyProcPort != NULL)
> + {
> + pq_setkeepalives(newval, MyProcPort);
> + }
> + return true;
> + }
> +
> + static bool
> + assign_tcp_keepalives_idle(int newval, bool doit, GucSource source)
> + {
> + if (doit && MyProcPort != NULL)
> + {
> + return (pq_setkeepalivesidle(newval, MyProcPort) == STATUS_OK);
> + }
> +
> + return true;
> + }
> +
> + static const char *
> + show_tcp_keepalives_idle(void)
> + {
> + static char nbuf[32];
> + snprintf(nbuf, sizeof(nbuf), "%d",
> + (tcp_keepalives_idle != 0 || MyProcPort == NULL)
> + ? tcp_keepalives_idle : MyProcPort->default_keepalives_idle);
> + return nbuf;
> + }
> +
> + static bool
> + assign_tcp_keepalives_interval(int newval, bool doit, GucSource source)
> + {
> + if (doit && MyProcPort != NULL)
> + {
> + return (pq_setkeepalivesinterval(newval, MyProcPort) == STATUS_OK);
> + }
> +
> + return true;
> + }
> +
> + static const char *
> + show_tcp_keepalives_interval(void)
> + {
> + static char nbuf[32];
> + snprintf(nbuf, sizeof(nbuf), "%d",
> + (tcp_keepalives_interval != 0 || MyProcPort == NULL)
> + ? tcp_keepalives_interval : MyProcPort->default_keepalives_interval);
> + return nbuf;
> + }
> +
> + static bool
> + assign_tcp_keepalives_count(int newval, bool doit, GucSource source)
> + {
> + if (doit && MyProcPort != NULL)
> + {
> + return (pq_setkeepalivescount(newval, MyProcPort) == STATUS_OK);
> + }
> +
> + return true;
> + }
> +
> + static const char *
> + show_tcp_keepalives_count(void)
> + {
> + static char nbuf[32];
> + snprintf(nbuf, sizeof(nbuf), "%d",
> + (tcp_keepalives_count != 0 || MyProcPort == NULL)
> + ? tcp_keepalives_count : MyProcPort->default_keepalives_count);
> + return nbuf;
> + }
>
> #include "guc-file.c"
> Index: src/backend/utils/misc/postgresql.conf.sample
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v
> retrieving revision 1.140
> diff -c -r1.140 postgresql.conf.sample
> *** src/backend/utils/misc/postgresql.conf.sample 21 Apr 2005 19:18:13 -0000 1.140
> --- src/backend/utils/misc/postgresql.conf.sample 3 May 2005 01:44:08 -0000
> ***************
> *** 66,71 ****
> --- 66,77 ----
> #krb_server_keyfile = ''
> #db_user_namespace = false
>
> + # - TCP Keepalives -
> + # see 'man 7 tcp' for details
> + #tcp_keepalives = on # enables use of TCP keepalives
> + #tcp_keepalives_idle = 0 # TCP_KEEPIDLE, in seconds; 0 uses the system default.
> + #tcp_keepalives_interval = 0 # TCP_KEEPINTVL, in seconds; 0 uses the system default.
> + #tcp_keepalives_count = 0 # TCP_KEEPCNT, in seconds; 0 uses the system default.
>
> #---------------------------------------------------------------------------
> # RESOURCE USAGE (except WAL)
> Index: src/bin/psql/tab-complete.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/bin/psql/tab-complete.c,v
> retrieving revision 1.125
> diff -c -r1.125 tab-complete.c
> *** src/bin/psql/tab-complete.c 21 Apr 2005 19:18:13 -0000 1.125
> --- src/bin/psql/tab-complete.c 3 May 2005 01:44:10 -0000
> ***************
> *** 595,600 ****
> --- 595,604 ----
> "superuser_reserved_connections",
> "syslog_facility",
> "syslog_ident",
> + "tcp_keepalives",
> + "tcp_keepalives_idle",
> + "tcp_keepalives_interval",
> + "tcp_keepalives_count",
> "temp_buffers",
> "TimeZone",
> "trace_notify",
> Index: src/include/libpq/libpq-be.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/include/libpq/libpq-be.h,v
> retrieving revision 1.49
> diff -c -r1.49 libpq-be.h
> *** src/include/libpq/libpq-be.h 31 Dec 2004 22:03:32 -0000 1.49
> --- src/include/libpq/libpq-be.h 3 May 2005 01:44:10 -0000
> ***************
> *** 92,100 ****
> --- 92,113 ----
> char peer_cn[SM_USER + 1];
> unsigned long count;
> #endif
> +
> + /*
> + * Default TCP keepalive values found after accept(); 0 if unsupported or AF_UNIX.
> + */
> + int default_keepalives_idle;
> + int default_keepalives_interval;
> + int default_keepalives_count;
> } Port;
>
>
> extern ProtocolVersion FrontendProtocol;
>
> + /* TCP keepalives configuration. These are no-ops on an AF_UNIX socket. */
> + extern int pq_setkeepalives(bool onoff, Port *port);
> + extern int pq_setkeepalivesidle(int idle, Port *port);
> + extern int pq_setkeepalivesinterval(int interval, Port *port);
> + extern int pq_setkeepalivescount(int count, Port *port);
> +
> #endif /* LIBPQ_BE_H */
> Index: src/include/utils/guc.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/include/utils/guc.h,v
> retrieving revision 1.60
> diff -c -r1.60 guc.h
> *** src/include/utils/guc.h 25 Mar 2005 16:17:28 -0000 1.60
> --- src/include/utils/guc.h 3 May 2005 01:44:10 -0000
> ***************
> *** 133,138 ****
> --- 133,142 ----
> extern char *IdentFileName;
> extern char *external_pid_file;
>
> + extern bool tcp_keepalives;
> + extern int tcp_keepalives_idle;
> + extern int tcp_keepalives_interval;
> + extern int tcp_keepalives_count;
>
> extern void SetConfigOption(const char *name, const char *value,
> GucContext context, GucSource source);

>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq

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

Bruce Momjian wrote:
> Where are we on this? I do think it solves a problem that some are
> having, and it seems it would detect a disconnected client and abort a
> long running query.

I haven't had a chance to do any work to support the Solaris stuff, but
otherwise I'm just waiting on review.

Note that the patch doesn't do anything about long-running queries, it's
really about long-running connections/transactions. The server will only
notice the disconnected client once it tries to read/write something.

> I am not very excited about adding four more GUC variables, and I am
> thinking we could just have it use the OS defaults and see if we need
> more later, so that would add only one GUC.

The whole point of the patch is that the system-wide defaults are
usually not useful. The current, unpatched, server turns on SO_KEEPALIVE
always, but the typical OS defaults set the timeout before probing to
about 2 hours.

-O

Oliver Jowett <oliver(at)opencloud(dot)com> 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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> I am not very excited about adding four more GUC variables, and I am
> thinking we could just have it use the OS defaults and see if we need
> more later, so that would add only one GUC.

Huh? "Use the OS defaults" is where we are now.

I'm unconvinced of the value of allowing people to turn off keepalive
entirely, and therefore I question the need for the boolean
tcp_keepalives GUC, but the other three appear useful.

regards, tom lane

Tom Lane wrote:
> Oliver Jowett <oliver(at)opencloud(dot)com> 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

Is this patch being worked on?

---------------------------------------------------------------------------

Oliver Jowett wrote:
> Tom Lane wrote:
> > Oliver Jowett <oliver(at)opencloud(dot)com> 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
>

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

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.

-O

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(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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Oliver Jowett wrote:
>> 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.

I wanted it fixed to not do a lot of extra kernel calls during
backend startup...

regards, tom lane

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

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 -0000 1.335
> --- doc/src/sgml/runtime.sgml 4 Jul 2005 10:41:33 -0000
> ***************
> *** 894,899 ****
> --- 894,946 ----
> </listitem>
> </varlistentry>
>
> + <varlistentry id="guc-tcp-keepalives-idle" xreflabel="tcp_keepalives_idle">
> + <term><varname>tcp_keepalives_idle</varname> (<type>integer</type>)</term>
> + <indexterm>
> + <primary><varname>tcp_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">
> + <term><varname>tcp_keepalives_interval</varname> (<type>integer</type>)</term>
> + <indexterm>
> + <primary><varname>tcp_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">
> + <term><varname>tcp_keepalives_count</varname> (<type>integer</type>)</term>
> + <indexterm>
> + <primary><varname>tcp_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 -0000 1.176
> --- src/backend/libpq/pqcomm.c 4 Jul 2005 10:41:33 -0000
> ***************
> *** 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))
> + return 0;
> +
> + if (port->keepalives_idle != 0)
> + return port->keepalives_idle;
> +
> + if (port->default_keepalives_idle == 0)
> + {
> + socklen_t size = sizeof(port->default_keepalives_idle);
> + if (getsockopt(port->sock, SOL_TCP, TCP_KEEPIDLE,
> + (char *) &port->default_keepalives_idle,
> + &size) < 0)
> + {
> + elog(LOG, "getsockopt(TCP_KEEPIDLE) failed: %m");
> + return -1;
> + }
> + }
> +
> + return port->default_keepalives_idle;
> + #else
> + return 0;
> + #endif
> + }
> +
> + int
> + pq_setkeepalivesidle(int idle, Port *port)
> + {
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return STATUS_OK;
> +
> + #ifdef TCP_KEEPIDLE
> + if (idle == port->keepalives_idle)
> + return STATUS_OK;
> +
> + if (port->default_keepalives_idle == 0)
> + {
> + if (pq_getkeepalivesidle(port) < 0)
> + return STATUS_ERROR;
> + }
> +
> + if (idle == 0)
> + idle = port->default_keepalives_idle;
> +
> + if (setsockopt(port->sock, SOL_TCP, TCP_KEEPIDLE,
> + (char *) &idle, sizeof(idle)) < 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPIDLE) failed: %m");
> + return STATUS_ERROR;
> + }
> +
> + port->keepalives_idle = idle;
> + #else
> + if (idle != 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPIDLE) not supported");
> + return STATUS_ERROR;
> + }
> + #endif
> +
> + return STATUS_OK;
> + }
> +
> + int
> + pq_getkeepalivesinterval(Port *port)
> + {
> + #ifdef TCP_KEEPINTVL
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return 0;
> +
> + if (port->keepalives_interval != 0)
> + return port->keepalives_interval;
> +
> + if (port->default_keepalives_interval == 0)
> + {
> + socklen_t size = sizeof(port->default_keepalives_interval);
> + if (getsockopt(port->sock, SOL_TCP, TCP_KEEPINTVL,
> + (char *) &port->default_keepalives_interval,
> + &size) < 0)
> + {
> + elog(LOG, "getsockopt(TCP_KEEPINTVL) failed: %m");
> + return -1;
> + }
> + }
> +
> + return port->default_keepalives_interval;
> + #else
> + return 0;
> + #endif
> + }
> +
> + int
> + pq_setkeepalivesinterval(int interval, Port *port)
> + {
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return STATUS_OK;
> +
> + #ifdef TCP_KEEPINTVL
> + if (interval == port->keepalives_interval)
> + return STATUS_OK;
> +
> + if (port->default_keepalives_interval == 0) {
> + if (pq_getkeepalivesinterval(port) < 0)
> + return STATUS_ERROR;
> + }
> +
> + if (interval == 0)
> + interval = port->default_keepalives_interval;
> +
> + if (setsockopt(port->sock, SOL_TCP, TCP_KEEPINTVL,
> + (char *) &interval, sizeof(interval)) < 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPINTVL) failed: %m");
> + return STATUS_ERROR;
> + }
> +
> + port->keepalives_interval = interval;
> + #else
> + if (interval != 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPINTVL) not supported");
> + return STATUS_ERROR;
> + }
> + #endif
> +
> + return STATUS_OK;
> + }
> +
> + int
> + pq_getkeepalivescount(Port *port)
> + {
> + #ifdef TCP_KEEPCNT
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return 0;
> +
> + if (port->keepalives_count != 0)
> + return port->keepalives_count;
> +
> + if (port->default_keepalives_count == 0)
> + {
> + socklen_t size = sizeof(port->default_keepalives_count);
> + if (getsockopt(port->sock, SOL_TCP, TCP_KEEPCNT,
> + (char *) &port->default_keepalives_count,
> + &size) < 0)
> + {
> + elog(LOG, "getsockopt(TCP_KEEPCNT) failed: %m");
> + return -1;
> + }
> + }
> +
> + return port->default_keepalives_count;
> + #else
> + return 0;
> + #endif
> + }
> +
> + int
> + pq_setkeepalivescount(int count, Port *port)
> + {
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return STATUS_OK;
> +
> + #ifdef TCP_KEEPCNT
> + if (count == port->keepalives_count)
> + return STATUS_OK;
> +
> + if (port->default_keepalives_count == 0) {
> + if (pq_getkeepalivescount(port) < 0)
> + return STATUS_ERROR;
> + }
> +
> + if (count == 0)
> + count = port->default_keepalives_count;
> +
> + if (setsockopt(port->sock, SOL_TCP, TCP_KEEPCNT,
> + (char *) &count, sizeof(count)) < 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPCNT) failed: %m");
> + return STATUS_ERROR;
> + }
> +
> + port->keepalives_count = count;
> + #else
> + if (count != 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPCNT) not supported");
> + return STATUS_ERROR;
> + }
> + #endif
> +
> + return STATUS_OK;
> + }
> Index: src/backend/utils/misc/guc.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
> retrieving revision 1.271
> diff -u -c -r1.271 guc.c
> *** src/backend/utils/misc/guc.c 28 Jun 2005 05:09:02 -0000 1.271
> --- src/backend/utils/misc/guc.c 4 Jul 2005 10:41:33 -0000
> ***************
> *** 117,122 ****
> --- 117,128 ----
> static bool assign_transaction_read_only(bool newval, bool doit, GucSource source);
> static const char *assign_canonical_path(const char *newval, bool doit, GucSource source);
>
> + static bool assign_tcp_keepalives_idle(int newval, bool doit, GucSource source);
> + static bool assign_tcp_keepalives_interval(int newval, bool doit, GucSource source);
> + static bool assign_tcp_keepalives_count(int newval, bool doit, GucSource source);
> + static const char *show_tcp_keepalives_idle(void);
> + static const char *show_tcp_keepalives_interval(void);
> + static const char *show_tcp_keepalives_count(void);
>
> /*
> * GUC option variables that are exported from this module
> ***************
> *** 158,163 ****
> --- 164,172 ----
> char *IdentFileName;
> char *external_pid_file;
>
> + int tcp_keepalives_idle;
> + int tcp_keepalives_interval;
> + int tcp_keepalives_count;
>
> /*
> * These variables are all dummies that don't do anything, except in some
> ***************
> *** 1375,1380 ****
> --- 1384,1418 ----
> BLCKSZ, BLCKSZ, BLCKSZ, NULL, NULL
> },
>
> + {
> + {"tcp_keepalives_idle", PGC_USERSET, CLIENT_CONN_OTHER,
> + gettext_noop("Seconds between issuing TCP keepalives."),
> + gettext_noop("A value of 0 uses the system default."),
> + },
> + &tcp_keepalives_idle,
> + 0, 0, INT_MAX, assign_tcp_keepalives_idle, show_tcp_keepalives_idle
> + },
> +
> + {
> + {"tcp_keepalives_interval", PGC_USERSET, CLIENT_CONN_OTHER,
> + gettext_noop("Seconds between TCP keepalive retransmits."),
> + gettext_noop("A value of 0 uses the system default."),
> + },
> + &tcp_keepalives_interval,
> + 0, 0, INT_MAX, assign_tcp_keepalives_interval, show_tcp_keepalives_interval
> + },
> +
> + {
> + {"tcp_keepalives_count", PGC_USERSET, CLIENT_CONN_OTHER,
> + gettext_noop("Maximum number of TCP keepalive retransmits."),
> + gettext_noop("This controls the number of consecutive keepalive retransmits that can be "
> + "lost before a connection is considered dead. A value of 0 uses the "
> + "system default."),
> + },
> + &tcp_keepalives_count,
> + 0, 0, INT_MAX, assign_tcp_keepalives_count, show_tcp_keepalives_count
> + },
> +
> /* End-of-list marker */
> {
> {NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL
> ***************
> *** 5744,5748 ****
> --- 5782,5842 ----
> return newval;
> }
>
> + static bool
> + assign_tcp_keepalives_idle(int newval, bool doit, GucSource source)
> + {
> + if (doit && MyProcPort != NULL)
> + {
> + return (pq_setkeepalivesidle(newval, MyProcPort) == STATUS_OK);
> + }
> +
> + return true;
> + }
> +
> + static const char *
> + show_tcp_keepalives_idle(void)
> + {
> + static char nbuf[32];
> + snprintf(nbuf, sizeof(nbuf), "%d", MyProcPort == NULL ? 0 : pq_getkeepalivesidle(MyProcPort));
> + return nbuf;
> + }
> +
> + static bool
> + assign_tcp_keepalives_interval(int newval, bool doit, GucSource source)
> + {
> + if (doit && MyProcPort != NULL)
> + {
> + return (pq_setkeepalivesinterval(newval, MyProcPort) == STATUS_OK);
> + }
> +
> + return true;
> + }
> +
> + static const char *
> + show_tcp_keepalives_interval(void)
> + {
> + static char nbuf[32];
> + snprintf(nbuf, sizeof(nbuf), "%d", MyProcPort == NULL ? 0 : pq_getkeepalivesinterval(MyProcPort));
> + return nbuf;
> + }
> +
> + static bool
> + assign_tcp_keepalives_count(int newval, bool doit, GucSource source)
> + {
> + if (doit && MyProcPort != NULL)
> + {
> + return (pq_setkeepalivescount(newval, MyProcPort) == STATUS_OK);
> + }
> +
> + return true;
> + }
> +
> + static const char *
> + show_tcp_keepalives_count(void)
> + {
> + static char nbuf[32];
> + snprintf(nbuf, sizeof(nbuf), "%d", MyProcPort == NULL ? 0 : pq_getkeepalivescount(MyProcPort));
> + return nbuf;
> + }
>
> #include "guc-file.c"
> Index: src/backend/utils/misc/postgresql.conf.sample
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v
> retrieving revision 1.151
> diff -u -c -r1.151 postgresql.conf.sample
> *** src/backend/utils/misc/postgresql.conf.sample 2 Jul 2005 18:46:45 -0000 1.151
> --- src/backend/utils/misc/postgresql.conf.sample 4 Jul 2005 10:41:33 -0000
> ***************
> *** 70,75 ****
> --- 70,80 ----
> #krb_caseins_users = off
> #krb_srvname = 'postgres'
>
> + # - TCP Keepalives -
> + # see 'man 7 tcp' for details
> + #tcp_keepalives_idle = 0 # TCP_KEEPIDLE, in seconds; 0 uses the system default.
> + #tcp_keepalives_interval = 0 # TCP_KEEPINTVL, in seconds; 0 uses the system default.
> + #tcp_keepalives_count = 0 # TCP_KEEPCNT, in seconds; 0 uses the system default.
>
> #---------------------------------------------------------------------------
> # RESOURCE USAGE (except WAL)
> Index: src/bin/psql/tab-complete.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/bin/psql/tab-complete.c,v
> retrieving revision 1.133
> diff -u -c -r1.133 tab-complete.c
> *** src/bin/psql/tab-complete.c 22 Jun 2005 21:14:30 -0000 1.133
> --- src/bin/psql/tab-complete.c 4 Jul 2005 10:41:33 -0000
> ***************
> *** 600,605 ****
> --- 600,608 ----
> "superuser_reserved_connections",
> "syslog_facility",
> "syslog_ident",
> + "tcp_keepalives_idle",
> + "tcp_keepalives_interval",
> + "tcp_keepalives_count",
> "temp_buffers",
> "TimeZone",
> "trace_notify",
> Index: src/include/libpq/libpq-be.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/include/libpq/libpq-be.h,v
> retrieving revision 1.49
> diff -u -c -r1.49 libpq-be.h
> *** src/include/libpq/libpq-be.h 31 Dec 2004 22:03:32 -0000 1.49
> --- src/include/libpq/libpq-be.h 4 Jul 2005 10:41:33 -0000
> ***************
> *** 25,30 ****
> --- 25,33 ----
> #include <openssl/ssl.h>
> #include <openssl/err.h>
> #endif
> + #ifdef HAVE_NETINET_TCP_H
> + #include <netinet/tcp.h>
> + #endif
>
> #include "libpq/hba.h"
> #include "libpq/pqcomm.h"
> ***************
> *** 92,100 ****
> --- 95,131 ----
> char peer_cn[SM_USER + 1];
> unsigned long count;
> #endif
> +
> + /*
> + * TCP keepalive settings;
> + * default values are 0 if AF_UNIX or not yet known;
> + * current values are 0 if AF_UNIX or using the default.
> + */
> + #ifdef TCP_KEEPIDLE
> + int default_keepalives_idle;
> + int keepalives_idle;
> + #endif
> + #ifdef TCP_KEEPINTVL
> + int default_keepalives_interval;
> + int keepalives_interval;
> + #endif
> + #ifdef TCP_KEEPCNT
> + int default_keepalives_count;
> + int keepalives_count;
> + #endif
> } Port;
>
>
> extern ProtocolVersion FrontendProtocol;
>
> + /* TCP keepalives configuration. These are no-ops on an AF_UNIX socket. */
> +
> + extern int pq_getkeepalivesidle(Port *port);
> + extern int pq_getkeepalivesinterval(Port *port);
> + extern int pq_getkeepalivescount(Port *port);
> +
> + extern int pq_setkeepalivesidle(int idle, Port *port);
> + extern int pq_setkeepalivesinterval(int interval, Port *port);
> + extern int pq_setkeepalivescount(int count, Port *port);
> +
> #endif /* LIBPQ_BE_H */
> Index: src/include/utils/guc.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/include/utils/guc.h,v
> retrieving revision 1.61
> diff -u -c -r1.61 guc.h
> *** src/include/utils/guc.h 26 Jun 2005 03:04:12 -0000 1.61
> --- src/include/utils/guc.h 4 Jul 2005 10:41:33 -0000
> ***************
> *** 134,139 ****
> --- 134,142 ----
> extern char *IdentFileName;
> extern char *external_pid_file;
>
> + extern int tcp_keepalives_idle;
> + extern int tcp_keepalives_interval;
> + extern int tcp_keepalives_count;
>
> extern void SetConfigOption(const char *name, const char *value,
> GucContext context, GucSource source);

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

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

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

Patch applied. Thanks.

---------------------------------------------------------------------------

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 -0000 1.335
> --- doc/src/sgml/runtime.sgml 4 Jul 2005 10:41:33 -0000
> ***************
> *** 894,899 ****
> --- 894,946 ----
> </listitem>
> </varlistentry>
>
> + <varlistentry id="guc-tcp-keepalives-idle" xreflabel="tcp_keepalives_idle">
> + <term><varname>tcp_keepalives_idle</varname> (<type>integer</type>)</term>
> + <indexterm>
> + <primary><varname>tcp_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">
> + <term><varname>tcp_keepalives_interval</varname> (<type>integer</type>)</term>
> + <indexterm>
> + <primary><varname>tcp_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">
> + <term><varname>tcp_keepalives_count</varname> (<type>integer</type>)</term>
> + <indexterm>
> + <primary><varname>tcp_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 -0000 1.176
> --- src/backend/libpq/pqcomm.c 4 Jul 2005 10:41:33 -0000
> ***************
> *** 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))
> + return 0;
> +
> + if (port->keepalives_idle != 0)
> + return port->keepalives_idle;
> +
> + if (port->default_keepalives_idle == 0)
> + {
> + socklen_t size = sizeof(port->default_keepalives_idle);
> + if (getsockopt(port->sock, SOL_TCP, TCP_KEEPIDLE,
> + (char *) &port->default_keepalives_idle,
> + &size) < 0)
> + {
> + elog(LOG, "getsockopt(TCP_KEEPIDLE) failed: %m");
> + return -1;
> + }
> + }
> +
> + return port->default_keepalives_idle;
> + #else
> + return 0;
> + #endif
> + }
> +
> + int
> + pq_setkeepalivesidle(int idle, Port *port)
> + {
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return STATUS_OK;
> +
> + #ifdef TCP_KEEPIDLE
> + if (idle == port->keepalives_idle)
> + return STATUS_OK;
> +
> + if (port->default_keepalives_idle == 0)
> + {
> + if (pq_getkeepalivesidle(port) < 0)
> + return STATUS_ERROR;
> + }
> +
> + if (idle == 0)
> + idle = port->default_keepalives_idle;
> +
> + if (setsockopt(port->sock, SOL_TCP, TCP_KEEPIDLE,
> + (char *) &idle, sizeof(idle)) < 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPIDLE) failed: %m");
> + return STATUS_ERROR;
> + }
> +
> + port->keepalives_idle = idle;
> + #else
> + if (idle != 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPIDLE) not supported");
> + return STATUS_ERROR;
> + }
> + #endif
> +
> + return STATUS_OK;
> + }
> +
> + int
> + pq_getkeepalivesinterval(Port *port)
> + {
> + #ifdef TCP_KEEPINTVL
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return 0;
> +
> + if (port->keepalives_interval != 0)
> + return port->keepalives_interval;
> +
> + if (port->default_keepalives_interval == 0)
> + {
> + socklen_t size = sizeof(port->default_keepalives_interval);
> + if (getsockopt(port->sock, SOL_TCP, TCP_KEEPINTVL,
> + (char *) &port->default_keepalives_interval,
> + &size) < 0)
> + {
> + elog(LOG, "getsockopt(TCP_KEEPINTVL) failed: %m");
> + return -1;
> + }
> + }
> +
> + return port->default_keepalives_interval;
> + #else
> + return 0;
> + #endif
> + }
> +
> + int
> + pq_setkeepalivesinterval(int interval, Port *port)
> + {
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return STATUS_OK;
> +
> + #ifdef TCP_KEEPINTVL
> + if (interval == port->keepalives_interval)
> + return STATUS_OK;
> +
> + if (port->default_keepalives_interval == 0) {
> + if (pq_getkeepalivesinterval(port) < 0)
> + return STATUS_ERROR;
> + }
> +
> + if (interval == 0)
> + interval = port->default_keepalives_interval;
> +
> + if (setsockopt(port->sock, SOL_TCP, TCP_KEEPINTVL,
> + (char *) &interval, sizeof(interval)) < 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPINTVL) failed: %m");
> + return STATUS_ERROR;
> + }
> +
> + port->keepalives_interval = interval;
> + #else
> + if (interval != 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPINTVL) not supported");
> + return STATUS_ERROR;
> + }
> + #endif
> +
> + return STATUS_OK;
> + }
> +
> + int
> + pq_getkeepalivescount(Port *port)
> + {
> + #ifdef TCP_KEEPCNT
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return 0;
> +
> + if (port->keepalives_count != 0)
> + return port->keepalives_count;
> +
> + if (port->default_keepalives_count == 0)
> + {
> + socklen_t size = sizeof(port->default_keepalives_count);
> + if (getsockopt(port->sock, SOL_TCP, TCP_KEEPCNT,
> + (char *) &port->default_keepalives_count,
> + &size) < 0)
> + {
> + elog(LOG, "getsockopt(TCP_KEEPCNT) failed: %m");
> + return -1;
> + }
> + }
> +
> + return port->default_keepalives_count;
> + #else
> + return 0;
> + #endif
> + }
> +
> + int
> + pq_setkeepalivescount(int count, Port *port)
> + {
> + if (IS_AF_UNIX(port->laddr.addr.ss_family))
> + return STATUS_OK;
> +
> + #ifdef TCP_KEEPCNT
> + if (count == port->keepalives_count)
> + return STATUS_OK;
> +
> + if (port->default_keepalives_count == 0) {
> + if (pq_getkeepalivescount(port) < 0)
> + return STATUS_ERROR;
> + }
> +
> + if (count == 0)
> + count = port->default_keepalives_count;
> +
> + if (setsockopt(port->sock, SOL_TCP, TCP_KEEPCNT,
> + (char *) &count, sizeof(count)) < 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPCNT) failed: %m");
> + return STATUS_ERROR;
> + }
> +
> + port->keepalives_count = count;
> + #else
> + if (count != 0)
> + {
> + elog(LOG, "setsockopt(TCP_KEEPCNT) not supported");
> + return STATUS_ERROR;
> + }
> + #endif
> +
> + return STATUS_OK;
> + }
> Index: src/backend/utils/misc/guc.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
> retrieving revision 1.271
> diff -u -c -r1.271 guc.c
> *** src/backend/utils/misc/guc.c 28 Jun 2005 05:09:02 -0000 1.271
> --- src/backend/utils/misc/guc.c 4 Jul 2005 10:41:33 -0000
> ***************
> *** 117,122 ****
> --- 117,128 ----
> static bool assign_transaction_read_only(bool newval, bool doit, GucSource source);
> static const char *assign_canonical_path(const char *newval, bool doit, GucSource source);
>
> + static bool assign_tcp_keepalives_idle(int newval, bool doit, GucSource source);
> + static bool assign_tcp_keepalives_interval(int newval, bool doit, GucSource source);
> + static bool assign_tcp_keepalives_count(int newval, bool doit, GucSource source);
> + static const char *show_tcp_keepalives_idle(void);
> + static const char *show_tcp_keepalives_interval(void);
> + static const char *show_tcp_keepalives_count(void);
>
> /*
> * GUC option variables that are exported from this module
> ***************
> *** 158,163 ****
> --- 164,172 ----
> char *IdentFileName;
> char *external_pid_file;
>
> + int tcp_keepalives_idle;
> + int tcp_keepalives_interval;
> + int tcp_keepalives_count;
>
> /*
> * These variables are all dummies that don't do anything, except in some
> ***************
> *** 1375,1380 ****
> --- 1384,1418 ----
> BLCKSZ, BLCKSZ, BLCKSZ, NULL, NULL
> },
>
> + {
> + {"tcp_keepalives_idle", PGC_USERSET, CLIENT_CONN_OTHER,
> + gettext_noop("Seconds between issuing TCP keepalives."),
> + gettext_noop("A value of 0 uses the system default."),
> + },
> + &tcp_keepalives_idle,
> + 0, 0, INT_MAX, assign_tcp_keepalives_idle, show_tcp_keepalives_idle
> + },
> +
> + {
> + {"tcp_keepalives_interval", PGC_USERSET, CLIENT_CONN_OTHER,
> + gettext_noop("Seconds between TCP keepalive retransmits."),
> + gettext_noop("A value of 0 uses the system default."),
> + },
> + &tcp_keepalives_interval,
> + 0, 0, INT_MAX, assign_tcp_keepalives_interval, show_tcp_keepalives_interval
> + },
> +
> + {
> + {"tcp_keepalives_count", PGC_USERSET, CLIENT_CONN_OTHER,
> + gettext_noop("Maximum number of TCP keepalive retransmits."),
> + gettext_noop("This controls the number of consecutive keepalive retransmits that can be "
> + "lost before a connection is considered dead. A value of 0 uses the "
> + "system default."),
> + },
> + &tcp_keepalives_count,
> + 0, 0, INT_MAX, assign_tcp_keepalives_count, show_tcp_keepalives_count
> + },
> +
> /* End-of-list marker */
> {
> {NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL
> ***************
> *** 5744,5748 ****
> --- 5782,5842 ----
> return newval;
> }
>
> + static bool
> + assign_tcp_keepalives_idle(int newval, bool doit, GucSource source)
> + {
> + if (doit && MyProcPort != NULL)
> + {
> + return (pq_setkeepalivesidle(newval, MyProcPort) == STATUS_OK);
> + }
> +
> + return true;
> + }
> +
> + static const char *
> + show_tcp_keepalives_idle(void)
> + {
> + static char nbuf[32];
> + snprintf(nbuf, sizeof(nbuf), "%d", MyProcPort == NULL ? 0 : pq_getkeepalivesidle(MyProcPort));
> + return nbuf;
> + }
> +
> + static bool
> + assign_tcp_keepalives_interval(int newval, bool doit, GucSource source)
> + {
> + if (doit && MyProcPort != NULL)
> + {
> + return (pq_setkeepalivesinterval(newval, MyProcPort) == STATUS_OK);
> + }
> +
> + return true;
> + }
> +
> + static const char *
> + show_tcp_keepalives_interval(void)
> + {
> + static char nbuf[32];
> + snprintf(nbuf, sizeof(nbuf), "%d", MyProcPort == NULL ? 0 : pq_getkeepalivesinterval(MyProcPort));
> + return nbuf;
> + }
> +
> + static bool
> + assign_tcp_keepalives_count(int newval, bool doit, GucSource source)
> + {
> + if (doit && MyProcPort != NULL)
> + {
> + return (pq_setkeepalivescount(newval, MyProcPort) == STATUS_OK);
> + }
> +
> + return true;
> + }
> +
> + static const char *
> + show_tcp_keepalives_count(void)
> + {
> + static char nbuf[32];
> + snprintf(nbuf, sizeof(nbuf), "%d", MyProcPort == NULL ? 0 : pq_getkeepalivescount(MyProcPort));
> + return nbuf;
> + }
>
> #include "guc-file.c"
> Index: src/backend/utils/misc/postgresql.conf.sample
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v
> retrieving revision 1.151
> diff -u -c -r1.151 postgresql.conf.sample
> *** src/backend/utils/misc/postgresql.conf.sample 2 Jul 2005 18:46:45 -0000 1.151
> --- src/backend/utils/misc/postgresql.conf.sample 4 Jul 2005 10:41:33 -0000
> ***************
> *** 70,75 ****
> --- 70,80 ----
> #krb_caseins_users = off
> #krb_srvname = 'postgres'
>
> + # - TCP Keepalives -
> + # see 'man 7 tcp' for details
> + #tcp_keepalives_idle = 0 # TCP_KEEPIDLE, in seconds; 0 uses the system default.
> + #tcp_keepalives_interval = 0 # TCP_KEEPINTVL, in seconds; 0 uses the system default.
> + #tcp_keepalives_count = 0 # TCP_KEEPCNT, in seconds; 0 uses the system default.
>
> #---------------------------------------------------------------------------
> # RESOURCE USAGE (except WAL)
> Index: src/bin/psql/tab-complete.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/bin/psql/tab-complete.c,v
> retrieving revision 1.133
> diff -u -c -r1.133 tab-complete.c
> *** src/bin/psql/tab-complete.c 22 Jun 2005 21:14:30 -0000 1.133
> --- src/bin/psql/tab-complete.c 4 Jul 2005 10:41:33 -0000
> ***************
> *** 600,605 ****
> --- 600,608 ----
> "superuser_reserved_connections",
> "syslog_facility",
> "syslog_ident",
> + "tcp_keepalives_idle",
> + "tcp_keepalives_interval",
> + "tcp_keepalives_count",
> "temp_buffers",
> "TimeZone",
> "trace_notify",
> Index: src/include/libpq/libpq-be.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/include/libpq/libpq-be.h,v
> retrieving revision 1.49
> diff -u -c -r1.49 libpq-be.h
> *** src/include/libpq/libpq-be.h 31 Dec 2004 22:03:32 -0000 1.49
> --- src/include/libpq/libpq-be.h 4 Jul 2005 10:41:33 -0000
> ***************
> *** 25,30 ****
> --- 25,33 ----
> #include <openssl/ssl.h>
> #include <openssl/err.h>
> #endif
> + #ifdef HAVE_NETINET_TCP_H
> + #include <netinet/tcp.h>
> + #endif
>
> #include "libpq/hba.h"
> #include "libpq/pqcomm.h"
> ***************
> *** 92,100 ****
> --- 95,131 ----
> char peer_cn[SM_USER + 1];
> unsigned long count;
> #endif
> +
> + /*
> + * TCP keepalive settings;
> + * default values are 0 if AF_UNIX or not yet known;
> + * current values are 0 if AF_UNIX or using the default.
> + */
> + #ifdef TCP_KEEPIDLE
> + int default_keepalives_idle;
> + int keepalives_idle;
> + #endif
> + #ifdef TCP_KEEPINTVL
> + int default_keepalives_interval;
> + int keepalives_interval;
> + #endif
> + #ifdef TCP_KEEPCNT
> + int default_keepalives_count;
> + int keepalives_count;
> + #endif
> } Port;
>
>
> extern ProtocolVersion FrontendProtocol;
>
> + /* TCP keepalives configuration. These are no-ops on an AF_UNIX socket. */
> +
> + extern int pq_getkeepalivesidle(Port *port);
> + extern int pq_getkeepalivesinterval(Port *port);
> + extern int pq_getkeepalivescount(Port *port);
> +
> + extern int pq_setkeepalivesidle(int idle, Port *port);
> + extern int pq_setkeepalivesinterval(int interval, Port *port);
> + extern int pq_setkeepalivescount(int count, Port *port);
> +
> #endif /* LIBPQ_BE_H */
> Index: src/include/utils/guc.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/include/utils/guc.h,v
> retrieving revision 1.61
> diff -u -c -r1.61 guc.h
> *** src/include/utils/guc.h 26 Jun 2005 03:04:12 -0000 1.61
> --- src/include/utils/guc.h 4 Jul 2005 10:41:33 -0000
> ***************
> *** 134,139 ****
> --- 134,142 ----
> extern char *IdentFileName;
> extern char *external_pid_file;
>
> + extern int tcp_keepalives_idle;
> + extern int tcp_keepalives_interval;
> + extern int tcp_keepalives_count;
>
> extern void SetConfigOption(const char *name, const char *value,
> GucContext context, GucSource source);

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

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