SEO

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...8/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@momjian.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

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...8/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@momjian.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Bruce Momjian <bruce@momjian.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

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

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.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...3/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@momjian.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

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@momjian.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

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

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

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@momjian.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

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@momjian.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@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-patches

--
Bruce Momjian <bruce@momjian.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches