Terminating a backend

I have an idea for this TODO item:

* Allow administrators to safely terminate individual sessions either
via an SQL function or SIGTERM

Lock table corruption following SIGTERM of an individual backend
has been reported in 8.0. A possible cause was fixed in 8.1, but
it is unknown whether other problems exist. This item mostly
requires additional testing rather than of writing any new code.

http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php

When we get the termination signal, why can't we just set a global
boolean, do a query cancel, and in the setjmp() code block check the
global and exit --- at that stage we know we have released all locks and
can exit cleanly.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> I have an idea for this TODO item:
> ...
> When we get the termination signal, why can't we just set a global
> boolean, do a query cancel, and in the setjmp() code block check the
> global and exit --- at that stage we know we have released all locks and
> can exit cleanly.

How does that differ from what happens now?

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > I have an idea for this TODO item:
> > ...
> > When we get the termination signal, why can't we just set a global
> > boolean, do a query cancel, and in the setjmp() code block check the
> > global and exit --- at that stage we know we have released all locks and
> > can exit cleanly.
>
> How does that differ from what happens now?

We would _terminate_/exit, not just cancel the query.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> I have an idea for this TODO item:
>
> * Allow administrators to safely terminate individual sessions either
> via an SQL function or SIGTERM
>
> Lock table corruption following SIGTERM of an individual backend
> has been reported in 8.0. A possible cause was fixed in 8.1, but
> it is unknown whether other problems exist. This item mostly
> requires additional testing rather than of writing any new code.
>
> http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php
>
> When we get the termination signal, why can't we just set a global
> boolean, do a query cancel, and in the setjmp() code block check the
> global and exit --- at that stage we know we have released all locks and
> can exit cleanly.

Should I add this as a TODO? Seems so. Tom commented in the patches
queue that it will not work but I don't understand why.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Bruce Momjian wrote:
>> When we get the termination signal, why can't we just set a global
>> boolean, do a query cancel, and in the setjmp() code block check the
>> global and exit --- at that stage we know we have released all locks and
>> can exit cleanly.

> Should I add this as a TODO? Seems so. Tom commented in the patches
> queue that it will not work but I don't understand why.

The problem with treating it like elog(ERROR) is that you're at the
mercy of user-defined code as to whether you'll actually exit or not.
UDFs can trap elog(ERROR).

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Bruce Momjian wrote:
> >> When we get the termination signal, why can't we just set a global
> >> boolean, do a query cancel, and in the setjmp() code block check the
> >> global and exit --- at that stage we know we have released all locks and
> >> can exit cleanly.
>
> > Should I add this as a TODO? Seems so. Tom commented in the patches
> > queue that it will not work but I don't understand why.
>
> The problem with treating it like elog(ERROR) is that you're at the
> mercy of user-defined code as to whether you'll actually exit or not.
> UDFs can trap elog(ERROR).

Well, we can punt and blame the writer of the UDF if the signal is not
timely honored. Having something that works for 98% of the cases, can
be fixed for 1% of the remainder, and only fails in 1% (proprietary code
that cannot be fixed) is better than having nothing at all.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Bruce Momjian wrote:
> >> When we get the termination signal, why can't we just set a global
> >> boolean, do a query cancel, and in the setjmp() code block check the
> >> global and exit --- at that stage we know we have released all locks and
> >> can exit cleanly.
>
> > Should I add this as a TODO? Seems so. Tom commented in the patches
> > queue that it will not work but I don't understand why.
>
> The problem with treating it like elog(ERROR) is that you're at the
> mercy of user-defined code as to whether you'll actually exit or not.
> UDFs can trap elog(ERROR).

I don't understand. I was never considering elog(ERROR).

Right now for cancel we have:

pqsignal(SIGINT, StatementCancelHandler);

I am suggesting we add a new fuction pg_terminate_backend() that does
everything just like cancel, but also sets a global variable that we
check in the loop where we look for the next command and if it is set,
we exit the backend.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> I am suggesting we add a new fuction pg_terminate_backend() that does
> everything just like cancel, but also sets a global variable that we
> check in the loop where we look for the next command and if it is set,
> we exit the backend.

And if you never *get* to that loop, what have you accomplished?

Keep in mind that 99% of the excuse for people to want to use SIGTERM is
that the backend isn't responding to SIGINT. If you've fixed things so
that SIGTERM cannot get them out of any situation that SIGINT doesn't
get them out of, I don't think it's a step forward.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > I am suggesting we add a new fuction pg_terminate_backend() that does
> > everything just like cancel, but also sets a global variable that we
> > check in the loop where we look for the next command and if it is set,
> > we exit the backend.
>
> And if you never *get* to that loop, what have you accomplished?
>
> Keep in mind that 99% of the excuse for people to want to use SIGTERM is
> that the backend isn't responding to SIGINT. If you've fixed things so
> that SIGTERM cannot get them out of any situation that SIGINT doesn't
> get them out of, I don't think it's a step forward.

What I hear people ask is that they don't want the backend to read the
next command but to exit. That seems like a reasonable request.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> Keep in mind that 99% of the excuse for people to want to use SIGTERM is
>> that the backend isn't responding to SIGINT. If you've fixed things so
>> that SIGTERM cannot get them out of any situation that SIGINT doesn't
>> get them out of, I don't think it's a step forward.

> What I hear people ask is that they don't want the backend to read the
> next command but to exit. That seems like a reasonable request.

[shrug...] They can do that now, most of the time. What this is about
is dealing with corner cases, and in that respect what your proposal
will do is replace soluble problems with insoluble ones. But I suppose
I can't stop you if you're insistent.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Tom Lane wrote:
> >> Keep in mind that 99% of the excuse for people to want to use SIGTERM is
> >> that the backend isn't responding to SIGINT. If you've fixed things so
> >> that SIGTERM cannot get them out of any situation that SIGINT doesn't
> >> get them out of, I don't think it's a step forward.
>
> > What I hear people ask is that they don't want the backend to read the
> > next command but to exit. That seems like a reasonable request.
>
> [shrug...] They can do that now, most of the time. What this is about
> is dealing with corner cases, and in that respect what your proposal
> will do is replace soluble problems with insoluble ones. But I suppose
> I can't stop you if you're insistent.

I am kind of confused by your reaction to my idea. I thought we agreed
that there was going to be no way to cleanly terminate a backend at an
arbitrary time, and I thought we were getting better at having query
cancel work in most cases, so it seems combining these two ideas that
query cancel with an immediate exit from the query loop was a perfect
solution to a feature request we get regularly.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> I have an idea for this TODO item:
>
> * Allow administrators to safely terminate individual sessions either
> via an SQL function or SIGTERM
>
> Lock table corruption following SIGTERM of an individual backend
> has been reported in 8.0. A possible cause was fixed in 8.1, but
> it is unknown whether other problems exist. This item mostly
> requires additional testing rather than of writing any new code.
>
> http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php
>
> When we get the termination signal, why can't we just set a global
> boolean, do a query cancel, and in the setjmp() code block check the
> global and exit --- at that stage we know we have released all locks and
> can exit cleanly.

I have implemented this idea with the attached patch.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Bruce Momjian wrote:
> > I have an idea for this TODO item:
> >
> > * Allow administrators to safely terminate individual sessions either
> > via an SQL function or SIGTERM
> >
> > Lock table corruption following SIGTERM of an individual backend
> > has been reported in 8.0. A possible cause was fixed in 8.1, but
> > it is unknown whether other problems exist. This item mostly
> > requires additional testing rather than of writing any new code.
> >
> > http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php
> >
> > When we get the termination signal, why can't we just set a global
> > boolean, do a query cancel, and in the setjmp() code block check the
> > global and exit --- at that stage we know we have released all locks and
> > can exit cleanly.
>
> I have implemented this idea with the attached patch.

One problem I have with my patch is that SIGTERM is currently used by
the postmaster to shut down backends. Now because the postmaster knows
that all backend are terminating, it can accept a dirtier shutdown than
one where we are terminating just one backend and the rest are going to
keep running. The new SIGTERM coding is going to exit a backend only in
a place where cancel is checked.

I need some thoughts in this area.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> When we get the termination signal, why can't we just set a global
>> boolean, do a query cancel, and in the setjmp() code block check the
>> global and exit --- at that stage we know we have released all locks and
>> can exit cleanly.

> I have implemented this idea with the attached patch.

It was already explained to you why this is a bad idea.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> >> When we get the termination signal, why can't we just set a global
> >> boolean, do a query cancel, and in the setjmp() code block check the
> >> global and exit --- at that stage we know we have released all locks and
> >> can exit cleanly.
>
> > I have implemented this idea with the attached patch.
>
> It was already explained to you why this is a bad idea.

Yes, I remember your comments:

http://archives.postgresql.org/pgsql-hackers/2008-03/msg00145.php

Keep in mind that 99% of the excuse for people to want to use SIGTERM is
that the backend isn't responding to SIGINT. If you've fixed things so
that SIGTERM cannot get them out of any situation that SIGINT doesn't
get them out of, I don't think it's a step forward.

...

[shrug...] They can do that now, most of the time. What this is about
is dealing with corner cases, and in that respect what your proposal
will do is replace soluble problems with insoluble ones. But I suppose
I can't stop you if you're insistent.

I need more than one person to tell me it is a bad idea because I think
it is a good idea.

If we tell people SIGTERM is not safe to use then why is making it safe
worse. And if it is safe, we can just add a function to enable SIGTERM
to the backend without doing the query cancel longjump().

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> > > When we get the termination signal, why can't we just set a global
> > > boolean, do a query cancel, and in the setjmp() code block check the
> > > global and exit --- at that stage we know we have released all locks and
> > > can exit cleanly.
> >
> > I have implemented this idea with the attached patch.
>
> One problem I have with my patch is that SIGTERM is currently used by
> the postmaster to shut down backends. Now because the postmaster knows
> that all backend are terminating, it can accept a dirtier shutdown than
> one where we are terminating just one backend and the rest are going to
> keep running. The new SIGTERM coding is going to exit a backend only in
> a place where cancel is checked.

I have a idea --- to have pg_terminate_backend() set a PGPROC boolean
and then send a query cancel signal to the backend --- the backend can
then check the boolean and exit if required. I will work on a new
version of this patch tomorrow/Monday.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:

> I have a idea --- to have pg_terminate_backend() set a PGPROC boolean
> and then send a query cancel signal to the backend --- the backend can
> then check the boolean and exit if required. I will work on a new
> version of this patch tomorrow/Monday.

That's fine, even if it fails on 0.1% of the cases, but you can't use
SIGTERM to implement that because it already has a different meaning.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

oikBruce Momjian wrote:
> Bruce Momjian wrote:
> > > > When we get the termination signal, why can't we just set a global
> > > > boolean, do a query cancel, and in the setjmp() code block check the
> > > > global and exit --- at that stage we know we have released all locks and
> > > > can exit cleanly.
> > >
> > > I have implemented this idea with the attached patch.
> >
> > One problem I have with my patch is that SIGTERM is currently used by
> > the postmaster to shut down backends. Now because the postmaster knows
> > that all backend are terminating, it can accept a dirtier shutdown than
> > one where we are terminating just one backend and the rest are going to
> > keep running. The new SIGTERM coding is going to exit a backend only in
> > a place where cancel is checked.
>
> I have a idea --- to have pg_terminate_backend() set a PGPROC boolean
> and then send a query cancel signal to the backend --- the backend can
> then check the boolean and exit if required. I will work on a new
> version of this patch tomorrow/Monday.

Updated patch attached. I didn't modify SIGTERM at all but set a PRPROC
boolean and piggybacked on SIGINT. I think I got the PGPROC locking
right. I had to split apart pg_signal_backend() so I could do the
permission and pid checks independent of the signalling, because I
pg_terminate_backend() needs to check, then set the PGPROC variable,
then send the signal.

I also added an administration doc mention about when to use
pg_terminate_backend().

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Applied.

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

Bruce Momjian wrote:
> oikBruce Momjian wrote:
> > Bruce Momjian wrote:
> > > > > When we get the termination signal, why can't we just set a global
> > > > > boolean, do a query cancel, and in the setjmp() code block check the
> > > > > global and exit --- at that stage we know we have released all locks and
> > > > > can exit cleanly.
> > > >
> > > > I have implemented this idea with the attached patch.
> > >
> > > One problem I have with my patch is that SIGTERM is currently used by
> > > the postmaster to shut down backends. Now because the postmaster knows
> > > that all backend are terminating, it can accept a dirtier shutdown than
> > > one where we are terminating just one backend and the rest are going to
> > > keep running. The new SIGTERM coding is going to exit a backend only in
> > > a place where cancel is checked.
> >
> > I have a idea --- to have pg_terminate_backend() set a PGPROC boolean
> > and then send a query cancel signal to the backend --- the backend can
> > then check the boolean and exit if required. I will work on a new
> > version of this patch tomorrow/Monday.
>
> Updated patch attached. I didn't modify SIGTERM at all but set a PRPROC
> boolean and piggybacked on SIGINT. I think I got the PGPROC locking
> right. I had to split apart pg_signal_backend() so I could do the
> permission and pid checks independent of the signalling, because I
> pg_terminate_backend() needs to check, then set the PGPROC variable,
> then send the signal.
>
> I also added an administration doc mention about when to use
> pg_terminate_backend().
>
> --
> Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
> EnterpriseDB http://enterprisedb.com
>
> + If your life is a hard drive, Christ can be your backup. +

>
> --
> Sent via pgsql-patches mailing list (pgsql-patches(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-patches

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +