Change pg_cancel_*() to ignore current backend

On 2015-05-20 00:59, Jim Nasby wrote:

I find it annoying to have to specifically exclude pg_backend_pid() from
pg_stat_activity if I'm trying to kill a bunch of backends at once, and
I can't think of any reason why you'd ever want to call a pg_cancel_*
function with your own PID.

That's a rather easy way of testing that you're handling FATAL errors
correctly from a driver/whatever.

.m

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

On Tue, May 19, 2015 at 4:23 PM, Marko Tiikkaja <marko@joh.to> wrote:

On 2015-05-20 00:59, Jim Nasby wrote:

I find it annoying to have to specifically exclude pg_backend_pid() from
pg_stat_activity if I'm trying to kill a bunch of backends at once, and
I can't think of any reason why you'd ever want to call a pg_cancel_*
function with your own PID.

That's a rather easy way of testing that you're handling FATAL errors
correctly from a driver/whatever.

I'm having trouble thinking of a PC name for the function we create that
should do this; while changing the pg_cancel_* functions to operate more
safely.

David J.​

On 5/19/15 6:30 PM, David G. Johnston wrote:

On Tue, May 19, 2015 at 4:23 PM, Marko Tiikkaja <marko@joh.to
<mailto:marko@joh.to>>wrote:

On 2015-05-20 00:59, Jim Nasby wrote:

I find it annoying to have to specifically exclude
pg_backend_pid() from
pg_stat_activity if I'm trying to kill a bunch of backends at
once, and
I can't think of any reason why you'd ever want to call a
pg_cancel_*
function with your own PID.

That's a rather easy way of testing that you're handling FATAL
errors correctly from a driver/whatever.

I'm having trouble thinking of a PC name for the function we create that
should do this; while changing the pg_cancel_* functions to operate more
safely.

We could add a second parameter to the current functions: allow_own_pid
DEFAULT false. To me that seems better than an entirely separate set of
functions.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

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

Em terça-feira, 19 de maio de 2015, Jim Nasby <Jim.Nasby@bluetreble.com>
escreveu:

On 5/19/15 6:30 PM, David G. Johnston wrote:

On Tue, May 19, 2015 at 4:23 PM, Marko Tiikkaja <marko@joh.to
<mailto:marko@joh.to>>wrote:

On 2015-05-20 00:59, Jim Nasby wrote:

I find it annoying to have to specifically exclude
pg_backend_pid() from
pg_stat_activity if I'm trying to kill a bunch of backends at
once, and
I can't think of any reason why you'd ever want to call a
pg_cancel_*
function with your own PID.

That's a rather easy way of testing that you're handling FATAL
errors correctly from a driver/whatever.

I'm having trouble thinking of a PC name for the function we create that
should do this; while changing the pg_cancel_* functions to operate more
safely.

We could add a second parameter to the current functions: allow_own_pid
DEFAULT false. To me that seems better than an entirely separate set of
functions.

+1 to add a second parameter to current functions.

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

On 5/19/15 9:19 PM, Fabrízio de Royes Mello wrote:

We could add a second parameter to the current functions:
allow_own_pid DEFAULT false. To me that seems better than an
entirely separate set of functions.

+1 to add a second parameter to current functions.

Instead of allow_own_pid, I went with skip_own_pid. I have the function
still returning true even when it skips it's own PID... that seems a bit
weird, but I think it's better than returning false. Unless someone
thinks it should return NULL, but I don't see that as any better either.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

On Wed, May 20, 2015 at 2:40 AM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:

On 5/19/15 9:19 PM, Fabrízio de Royes Mello wrote:

We could add a second parameter to the current functions:
allow_own_pid DEFAULT false. To me that seems better than an
entirely separate set of functions.

+1 to add a second parameter to current functions.

Instead of allow_own_pid, I went with skip_own_pid. I have the function

still returning true even when it skips it's own PID... that seems a bit
weird, but I think it's better than returning false. Unless someone thinks
it should return NULL, but I don't see that as any better either.

Isn't better use 'false' as the default value of the new parameter?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

On 5/20/15 1:40 AM, Jim Nasby wrote:

On 5/19/15 9:19 PM, Fabrízio de Royes Mello wrote:

We could add a second parameter to the current functions:
allow_own_pid DEFAULT false. To me that seems better than an
entirely separate set of functions.

+1 to add a second parameter to current functions.

Instead of allow_own_pid, I went with skip_own_pid. I have the function
still returning true even when it skips it's own PID... that seems a bit
weird, but I think it's better than returning false. Unless someone
thinks it should return NULL, but I don't see that as any better either.

+1. I agree that cancelling/killing your own process should not be the
default behavior.

--
- David Steele
david@pgmasters.net

On May 20, 2015 6:43 AM, "David Steele" <david@pgmasters.net> wrote:

On 5/20/15 1:40 AM, Jim Nasby wrote:

On 5/19/15 9:19 PM, Fabrízio de Royes Mello wrote:

We could add a second parameter to the current functions:
allow_own_pid DEFAULT false. To me that seems better than an
entirely separate set of functions.

+1 to add a second parameter to current functions.

Instead of allow_own_pid, I went with skip_own_pid. I have the function
still returning true even when it skips it's own PID... that seems a bit
weird, but I think it's better than returning false. Unless someone
thinks it should return NULL, but I don't see that as any better either.

+1. I agree that cancelling/killing your own process should not be the
default behavior.

-1. It breaks backwards compatibility. I use this function a fair bit to
terminate the current backend all the time. Changing the signature is
great, by make the default behavior retain the current behavior.

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

On 5/19/15 9:19 PM, Fabrízio de Royes Mello wrote:

+1 to add a second parameter to current functions.

Instead of allow_own_pid, I went with skip_own_pid. I have the function
still returning true even when it skips it's own PID... that seems a bit
weird, but I think it's better than returning false. Unless someone
thinks it should return NULL, but I don't see that as any better either.

The implementation would probably be considerably simpler if you treated
these as separate functions at the SQL/C level, ie overload rather than
try to treat the added parameter as having a default.

regards, tom lane

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

David Steele <david@pgmasters.net> writes:

+1. I agree that cancelling/killing your own process should not be the
default behavior.

I think backwards compatibility probably trumps that argument. I have
no objection to providing a different call that behaves this way, but
changing the behavior of existing applications will face a *much*
higher barrier to acceptance. Especially since a real use-case for
the current behavior was shown upthread, which means you can't argue
that it's simply a bug.

regards, tom lane

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

On 5/20/15 10:09 AM, Tom Lane wrote:

David Steele <david@pgmasters.net> writes:

+1. I agree that cancelling/killing your own process should not be the
default behavior.

I think backwards compatibility probably trumps that argument. I have
no objection to providing a different call that behaves this way, but
changing the behavior of existing applications will face a *much*
higher barrier to acceptance. Especially since a real use-case for
the current behavior was shown upthread, which means you can't argue
that it's simply a bug.

Just my 2 cents, but I think the argument for the default behavior is
coming from a hacker viewpoint rather than a user viewpoint. I know
it's handy for testing but how many real-world scenarios are there?

I've recently jumped the fence after being strictly a user for sixteen
years so that's still my default perspective. I was definitely annoyed
when pg_stat_activity.pid changed in 9.2 but it was still the right
thing to do.

--
- David Steele
david@pgmasters.net

On Wed, May 20, 2015 at 9:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I think backwards compatibility probably trumps that argument. I have
no objection to providing a different call that behaves this way, but
changing the behavior of existing applications will face a *much*
higher barrier to acceptance. Especially since a real use-case for
the current behavior was shown upthread, which means you can't argue
that it's simply a bug.

regards, tom lane

Agree.
It breaks backwards compatibility. I use this function a fair bit to
terminate the current backend all the time.

--
Jon

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

On 5/20/15 8:47 AM, Tom Lane wrote:

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

On 5/19/15 9:19 PM, Fabrízio de Royes Mello wrote:

+1 to add a second parameter to current functions.

Instead of allow_own_pid, I went with skip_own_pid. I have the function
still returning true even when it skips it's own PID... that seems a bit
weird, but I think it's better than returning false. Unless someone
thinks it should return NULL, but I don't see that as any better either.

The implementation would probably be considerably simpler if you treated
these as separate functions at the SQL/C level, ie overload rather than
try to treat the added parameter as having a default.

AFAICS that's just a minor change to what I'm doing in
catalog/system_view.sql and nothing else, so I'm not seeing the win.
What am I missing?

Now that we have default parameters my preference is to use them if for
no other reason than reduce bloat in \df...

BTW, is there a reason we're putting function SQL in that file other
than it was a convenient place?
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

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

On 5/20/15 11:15 AM, Jon Nelson wrote:

On Wed, May 20, 2015 at 9:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I think backwards compatibility probably trumps that argument. I have
no objection to providing a different call that behaves this way, but
changing the behavior of existing applications will face a *much*
higher barrier to acceptance. Especially since a real use-case for
the current behavior was shown upthread, which means you can't argue
that it's simply a bug.

regards, tom lane

Agree.
It breaks backwards compatibility. I use this function a fair bit to
terminate the current backend all the time.

Could you elaborate on your use case for doing that?

Echoing David's comment elsewhere, I suspect non-developers won't have a
use for self-termination. I don't see how self-cancel even makes sense,
and generally if you want to terminate the connection there's easier
ways to do that then "SELECT pg_terminate_backend(pg_backend_pid())".

I certainly don't want to cause pain for developers, but is this really
that common?

BTW, if someone had an awesome idea for a new function name then we
could just go that route. I can't think of anything better than
pg_*_session. Though, I guess we could do pg_terminate_session and
pg_cancel_query, which wouldn't be horrid.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

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

On 2015-05-20 18:48:59 -0500, Jim Nasby wrote:

and generally if you want to terminate the connection there's easier
ways to do that then "SELECT pg_terminate_backend(pg_backend_pid())".

Which would be what exactly? Say, you're inside a security definer
function.

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

On 5/20/15 6:56 PM, Andres Freund wrote:

On 2015-05-20 18:48:59 -0500, Jim Nasby wrote:

and generally if you want to terminate the connection there's easier
ways to do that then "SELECT pg_terminate_backend(pg_backend_pid())".

Which would be what exactly? Say, you're inside a security definer
function.

Error isn't good enough so you want to kill the backend? I hadn't
considered that; what's the common use case for it? ISTM it'd be better
to allow elog to log and then terminate the backend, but of course that
doesn't help with backwards compatibility. :/

What do people think about pg_cancel_query() and pg_terminate_session()?
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

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

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

On 5/20/15 6:56 PM, Andres Freund wrote:

On 2015-05-20 18:48:59 -0500, Jim Nasby wrote:

and generally if you want to terminate the connection there's easier
ways to do that then "SELECT pg_terminate_backend(pg_backend_pid())".

Which would be what exactly? Say, you're inside a security definer
function.

Error isn't good enough so you want to kill the backend? I hadn't
considered that; what's the common use case for it? ISTM it'd be better
to allow elog to log and then terminate the backend, but of course that
doesn't help with backwards compatibility. :/

That's spelled elog(FATAL), no?

regards, tom lane

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

On 2015-05-20 20:38:51 -0400, Tom Lane wrote:

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

On 5/20/15 6:56 PM, Andres Freund wrote:

On 2015-05-20 18:48:59 -0500, Jim Nasby wrote:

and generally if you want to terminate the connection there's easier
ways to do that then "SELECT pg_terminate_backend(pg_backend_pid())".

Which would be what exactly? Say, you're inside a security definer
function.

Error isn't good enough so you want to kill the backend?

Yep.

I hadn't considered that; what's the common use case for it?

I've seen it basically in two cases:
1) The "role" of the server has changed in some way, and some function
wants to force a reconnect. Say a former master that's now a logical
replication (in that case IIRC londiste) standby, and a trigger was
installed to rediredt existing writers.
2) A function detects that something has has gone rather wrong with a
session state and wants to force a reconnect. I've seen this in a
"handwritten" RLS implementation.

ISTM it'd be better
to allow elog to log and then terminate the backend, but of course that
doesn't help with backwards compatibility. :/

That's spelled elog(FATAL), no?

Which is, to my knowledge, inaccessible from at least plpgsql.

I've a hard time believing it's actually a good idea to change this. It
pretty much seems to only be useful if you're doing unqualified SELECT
pg_cancel_backend(pid) FROM pg_stat_activity; type queries. I don't see
that as something we need to address.

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

Jim Nasby wrote:

BTW, is there a reason we're putting function SQL in that file other than it
was a convenient place?

Probably not. I've looked at that file wondering the same thing a
number of times ...

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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