Change pg_cancel_*() to ignore current backend
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.
Any objections to modifying those functions so they do nothing when
handed the PID of the current backend?
--
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 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
Show quoted text
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
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
Attachments:
pg__backend_skip_pid.patchtext/plain; charset=UTF-8; name=pg__backend_skip_pid.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 89a609f..b405876 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -16508,7 +16508,7 @@ SELECT set_config('log_statement_stats', 'off', false);
<tbody>
<row>
<entry>
- <literal><function>pg_cancel_backend(<parameter>pid</parameter> <type>int</>)</function></literal>
+ <literal><function>pg_cancel_backend(<parameter>pid</parameter> <type>int</><optional>, <parameter>skip_my_pid</> <type>boolean</> </optional>)</function></literal>
</entry>
<entry><type>boolean</type></entry>
<entry>Cancel a backend's current query. This is also allowed if the
@@ -16532,7 +16532,7 @@ SELECT set_config('log_statement_stats', 'off', false);
</row>
<row>
<entry>
- <literal><function>pg_terminate_backend(<parameter>pid</parameter> <type>int</>)</function></literal>
+ <literal><function>pg_terminate_backend(<parameter>pid</parameter> <type>int</><optional>, <parameter>skip_my_pid</> <type>boolean</> </optional>)</function></literal>
</entry>
<entry><type>boolean</type></entry>
<entry>Terminate a backend. This is also allowed if the calling role
@@ -16562,6 +16562,10 @@ SELECT set_config('log_statement_stats', 'off', false);
The role of an active backend can be found from the
<structfield>usename</structfield> column of the
<structname>pg_stat_activity</structname> view.
+
+ There is an optional second parameter of type <type>boolean</type>. If
+ <literal>true</> (the default), <function>pg_cancel_backend</> and
+ <function>pg_terminate_backend</> will not signal the current backend.
</para>
<para>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 18921c4..a0cc975 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -869,6 +869,14 @@ COMMENT ON FUNCTION ts_debug(text) IS
--
CREATE OR REPLACE FUNCTION
+ pg_cancel_backend(pid int, skip_my_pid boolean DEFAULT true)
+ RETURNS boolean STRICT VOLATILE LANGUAGE internal AS 'pg_cancel_backend';
+
+CREATE OR REPLACE FUNCTION
+ pg_terminate_backend(pid int, skip_my_pid boolean DEFAULT true)
+ RETURNS boolean STRICT VOLATILE LANGUAGE internal AS 'pg_terminate_backend';
+
+CREATE OR REPLACE FUNCTION
pg_start_backup(label text, fast boolean DEFAULT false)
RETURNS pg_lsn STRICT VOLATILE LANGUAGE internal AS 'pg_start_backup';
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 61d609f..dce8498 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -94,8 +94,12 @@ current_query(PG_FUNCTION_ARGS)
#define SIGNAL_BACKEND_NOPERMISSION 2
#define SIGNAL_BACKEND_NOSUPERUSER 3
static int
-pg_signal_backend(int pid, int sig)
+pg_signal_backend(int pid, int sig, bool skip_own_pid)
{
+ /* Skip our own pid unless we're told not to */
+ if (skip_own_pid && pid == MyProcPid)
+ return SIGNAL_BACKEND_SUCCESS;
+
PGPROC *proc = BackendPidGetProc(pid);
/*
@@ -158,7 +162,7 @@ pg_signal_backend(int pid, int sig)
Datum
pg_cancel_backend(PG_FUNCTION_ARGS)
{
- int r = pg_signal_backend(PG_GETARG_INT32(0), SIGINT);
+ int r = pg_signal_backend(PG_GETARG_INT32(0), SIGINT, PG_GETARG_BOOL(1));
if (r == SIGNAL_BACKEND_NOSUPERUSER)
ereport(ERROR,
@@ -182,7 +186,7 @@ pg_cancel_backend(PG_FUNCTION_ARGS)
Datum
pg_terminate_backend(PG_FUNCTION_ARGS)
{
- int r = pg_signal_backend(PG_GETARG_INT32(0), SIGTERM);
+ int r = pg_signal_backend(PG_GETARG_INT32(0), SIGTERM, PG_GETARG_BOOL(1));
if (r == SIGNAL_BACKEND_NOSUPERUSER)
ereport(ERROR,
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index b5b9345..475545b 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3128,9 +3128,9 @@ DESCR("get OID of current session's temp schema, if any");
DATA(insert OID = 2855 ( pg_is_other_temp_schema PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 16 "26" _null_ _null_ _null_ _null_ _null_ pg_is_other_temp_schema _null_ _null_ _null_ ));
DESCR("is schema another session's temp schema?");
-DATA(insert OID = 2171 ( pg_cancel_backend PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 16 "23" _null_ _null_ _null_ _null_ _null_ pg_cancel_backend _null_ _null_ _null_ ));
+DATA(insert OID = 2171 ( pg_cancel_backend PGNSP PGUID 12 1 0 0 0 f f f f t f v 2 0 16 "23 16" _null_ _null_ _null_ _null_ _null_ pg_cancel_backend _null_ _null_ _null_ ));
DESCR("cancel a server process' current query");
-DATA(insert OID = 2096 ( pg_terminate_backend PGNSP PGUID 12 1 0 0 0 f f f f t f v 1 0 16 "23" _null_ _null_ _null_ _null_ _null_ pg_terminate_backend _null_ _null_ _null_ ));
+DATA(insert OID = 2096 ( pg_terminate_backend PGNSP PGUID 12 1 0 0 0 f f f f t f v 2 0 16 "23 16" _null_ _null_ _null_ _null_ _null_ pg_terminate_backend _null_ _null_ _null_ ));
DESCR("terminate a server process");
DATA(insert OID = 2172 ( pg_start_backup PGNSP PGUID 12 1 0 0 0 f f f f t f v 2 0 3220 "25 16" _null_ _null_ _null_ _null_ _null_ pg_start_backup _null_ _null_ _null_ ));
DESCR("prepare for taking an online backup");
diff --git a/src/test/regress/expected/hs_standby_functions.out b/src/test/regress/expected/hs_standby_functions.out
index 16d50a8..7ab98a6 100644
--- a/src/test/regress/expected/hs_standby_functions.out
+++ b/src/test/regress/expected/hs_standby_functions.out
@@ -36,5 +36,5 @@ from pg_locks where virtualxid = '1/1';
(1 row)
-- suicide is painless
-select pg_cancel_backend(pg_backend_pid());
+select pg_cancel_backend(pg_backend_pid(), false);
ERROR: canceling statement due to user request
diff --git a/src/test/regress/sql/hs_standby_functions.sql b/src/test/regress/sql/hs_standby_functions.sql
index 7577045..e953de2 100644
--- a/src/test/regress/sql/hs_standby_functions.sql
+++ b/src/test/regress/sql/hs_standby_functions.sql
@@ -21,4 +21,4 @@ select locktype, virtualxid, virtualtransaction, mode, granted
from pg_locks where virtualxid = '1/1';
-- suicide is painless
-select pg_cancel_backend(pg_backend_pid());
+select pg_cancel_backend(pg_backend_pid(), false);
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
Show quoted text
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
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.
Show quoted text
--
- David Steele
david@pgmasters.net
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
On Wed, May 20, 2015 at 8:46 PM, Andres Freund <andres@anarazel.de> wrote:
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.
+1. I'm not saying this isn't annoying - I've been annoyed by it
myself - but IMHO it's really not worth having two functions that do
99% the same thing. Then, instead of having to remember to exclude
your own backend using the same SQL syntax you use for everything
else, you have to remember which of two similarly-named functions to
call if you don't want to kill your own backend. That might be better
for some people, but it won't be better for everyone.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
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/21/15 7:12 AM, Robert Haas wrote:
On Wed, May 20, 2015 at 8:46 PM, Andres Freund <andres@anarazel.de> wrote:
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.+1. I'm not saying this isn't annoying - I've been annoyed by it
myself - but IMHO it's really not worth having two functions that do
99% the same thing. Then, instead of having to remember to exclude
your own backend using the same SQL syntax you use for everything
else, you have to remember which of two similarly-named functions to
call if you don't want to kill your own backend. That might be better
for some people, but it won't be better for everyone.
OTOH, if we allowed RAISE FATAL in plpgsql there'd be no need for
self-termination via pg_terminate_backend(). I also don't see a problem
with allowing self-termination, with the default being to disallow it.
I suspect the number of people writing code that need self-termination
is very, very small, whereas probably every DBA out there has been
bitten by this. This is the type of thing that gives Postgres a
reputation for being 'hard to use'. I would think the benefit of the
larger group would outweigh the pain the smaller group would feel
changing their code, but of course that's just my opinion and I don't
see any easy way to quantify that.
You and Andreas think it's fine as-is.
Tom and Jon Nelson maybe don't like it as-is, but won't break backwards
compatibility.
David Steele and I seem fine with breaking compat., not sure about Fabrizio.
Given the opposition unless some others speak up I'll just drop it.
--
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 Fri, May 22, 2015 at 3:28 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
You and Andreas think it's fine as-is.
Tom and Jon Nelson maybe don't like it as-is, but won't break backwards
compatibility.
David Steele and I seem fine with breaking compat., not sure about
Fabrizio.
+1 to add a second parameter and -1 to break backward compatibility with
the "default = true".
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
2015-05-22 20:28 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:
On 5/21/15 7:12 AM, Robert Haas wrote:
On Wed, May 20, 2015 at 8:46 PM, Andres Freund <andres@anarazel.de>
wrote: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.+1. I'm not saying this isn't annoying - I've been annoyed by it
myself - but IMHO it's really not worth having two functions that do
99% the same thing. Then, instead of having to remember to exclude
your own backend using the same SQL syntax you use for everything
else, you have to remember which of two similarly-named functions to
call if you don't want to kill your own backend. That might be better
for some people, but it won't be better for everyone.OTOH, if we allowed RAISE FATAL in plpgsql there'd be no need for
self-termination via pg_terminate_backend(). I also don't see a problem
with allowing self-termination, with the default being to disallow it.I suspect the number of people writing code that need self-termination is
very, very small, whereas probably every DBA out there has been bitten by
this. This is the type of thing that gives Postgres a reputation for being
'hard to use'. I would think the benefit of the larger group would outweigh
the pain the smaller group would feel changing their code, but of course
that's just my opinion and I don't see any easy way to quantify that.You and Andreas think it's fine as-is.
Tom and Jon Nelson maybe don't like it as-is, but won't break backwards
compatibility.
I am with Tom and Jon - I don't see a good enough reason why to change long
used behave without more users reports. Possibility to kill self is simple
test, so this feature is working.
Regards
Pavel
Show quoted text
David Steele and I seem fine with breaking compat., not sure about
Fabrizio.Given the opposition unless some others speak up I'll just drop it.
--
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 May 19, 2015, at 6:59 PM, Jim Nasby <Jim.Nasby@bluetreble.com> 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.
I'm just a lurker, but regularly get bitten by this exact thing.
Rather than change the behavior of pg_cancel/terminate_backend(), why not change pg_stat_activity to exclude the current session? Seems like showing a row in pg_stat_activity for "SELECT * FROM pg_stat_activity" is kinda useless anyways.
eric
PROPRIETARY AND COMPANY CONFIDENTIAL COMMUNICATIONS
The information contained in this communication is intended only for
the use of the addressee. Any other use is strictly prohibited.
Please notify the sender if you have received this message in error.
This communication is protected by applicable legal privileges and is
company confidential.
--
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/22/15 3:08 PM, Eric Ridge 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.
I'm just a lurker, but regularly get bitten by this exact thing.
Rather than change the behavior of pg_cancel/terminate_backend(), why not change pg_stat_activity to exclude the current session? Seems like showing a row in pg_stat_activity for "SELECT * FROM pg_stat_activity" is kinda useless anyways.
Interesting idea. I suspect that would be even more invasive than
modifying the functions though...
--
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/22/15 3:08 PM, Eric Ridge wrote:
Rather than change the behavior of pg_cancel/terminate_backend(), why not change pg_stat_activity to exclude the current session? Seems like showing a row in pg_stat_activity for "SELECT * FROM pg_stat_activity" is kinda useless anyways.
Interesting idea. I suspect that would be even more invasive than
modifying the functions though...
-1 ... some other columns in pg_stat_activity are potentially useful even
for the current session, eg session and transaction start times.
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/22/15 4:51 PM, Tom Lane wrote:
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
On 5/22/15 3:08 PM, Eric Ridge wrote:
Rather than change the behavior of pg_cancel/terminate_backend(), why not change pg_stat_activity to exclude the current session? Seems like showing a row in pg_stat_activity for "SELECT * FROM pg_stat_activity" is kinda useless anyways.
Interesting idea. I suspect that would be even more invasive than
modifying the functions though...-1 ... some other columns in pg_stat_activity are potentially useful even
for the current session, eg session and transaction start times.
AFAICT the only field you can get in pg_stat_activity and nowhere else
is session start time (txn start is always now(), no?).
If that's the only objection about eliminating the current backend from
pg_stat_activity then I'd say we should just add a session_start_time
function to handle that.
--
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-22 17:29:03 -0500, Jim Nasby wrote:
On 5/22/15 4:51 PM, Tom Lane wrote:
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
On 5/22/15 3:08 PM, Eric Ridge wrote:
Rather than change the behavior of pg_cancel/terminate_backend(), why not change pg_stat_activity to exclude the current session? Seems like showing a row in pg_stat_activity for "SELECT * FROM pg_stat_activity" is kinda useless anyways.
Interesting idea. I suspect that would be even more invasive than
modifying the functions though...-1 ... some other columns in pg_stat_activity are potentially useful even
for the current session, eg session and transaction start times.AFAICT the only field you can get in pg_stat_activity and nowhere else is
session start time (txn start is always now(), no?).If that's the only objection about eliminating the current backend from
pg_stat_activity then I'd say we should just add a session_start_time
function to handle that.
That's far too big a backward compat break for something of very minor
benefit.
This whole discussion seems to be about making it easier to run SELECT
pg_cancel_backend(pid) FROM pg_stat_activity;. But that shouldn't be
made easier! If anything harder.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
This whole discussion seems to be about making it easier to run SELECT
pg_cancel_backend(pid) FROM pg_stat_activity;. But that shouldn't be
made easier! If anything harder.
Indeed. I find it hard to believe that there's a real problem here, and
I absolutely will resist breaking backwards compatibility to solve it.
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 05/22/2015 03:35 PM, Andres Freund wrote:
On 2015-05-22 17:29:03 -0500, Jim Nasby wrote:
On 5/22/15 4:51 PM, Tom Lane wrote:
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
On 5/22/15 3:08 PM, Eric Ridge wrote:
Rather than change the behavior of pg_cancel/terminate_backend(), why not change pg_stat_activity to exclude the current session? Seems like showing a row in pg_stat_activity for "SELECT * FROM pg_stat_activity" is kinda useless anyways.
Interesting idea. I suspect that would be even more invasive than
modifying the functions though...-1 ... some other columns in pg_stat_activity are potentially useful even
for the current session, eg session and transaction start times.AFAICT the only field you can get in pg_stat_activity and nowhere else is
session start time (txn start is always now(), no?).If that's the only objection about eliminating the current backend from
pg_stat_activity then I'd say we should just add a session_start_time
function to handle that.That's far too big a backward compat break for something of very minor
benefit.This whole discussion seems to be about making it easier to run SELECT
pg_cancel_backend(pid) FROM pg_stat_activity;. But that shouldn't be
made easier! If anything harder.
Linux doesn't prevent you from running Kill on your own process, either.
Would be interesting to find out how their discussion on the same topic
went.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Import Notes
Reply to msg id not found: WM9f74fd4e436e39da37ef45c4c00023c3afecd688d78909e00be0ebcc30e8df544a775742c4aef628771161099c83828f@asav-3.01.com
On Fri, May 22, 2015 at 4:51 PM Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
Interesting idea. I suspect that would be even more invasive than
modifying the functions though...
Here's the solution. I can't see how anyone could possibly disagree with
this... ;)
Change the sort order for pg_stat_activity so the current session sorts
last. That way all the other sessions get killed first when doing select
pg_terminate_backend(pid) from pg_stat_activity.
When I get bitten by this, I don't really care that my session gets killed,
I care that it gets killed before all the others. Personally, I only do
this sort of thing interactively via psql, and typically ^D as soon as the
query finishes (which means the second time I run psql and add a WHERE
clause to the query).
Alternatively, queue up pg_cancel/terminate_backend calls and process them
only on successful transaction commit, in such an order that the current
session is processed last. They'd be consistent with NOTIFY too, which
might be an added bonus.
eric
ps, sorry my last message was from corporate email w/ the stupid "legal"
disclaimer.
On Sat, May 23, 2015 at 7:43 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andres Freund <andres@anarazel.de> writes:
This whole discussion seems to be about making it easier to run SELECT
pg_cancel_backend(pid) FROM pg_stat_activity;. But that shouldn't be
made easier! If anything harder.Indeed. I find it hard to believe that there's a real problem here, and
I absolutely will resist breaking backwards compatibility to solve it.
+1. Reading this thread I don't see why there is actually a problem...
The whole discussion is about moving the check at SQL-level with
pg_backend_pid(), that people are used to for a couple of releases
now, into pg_cancel_backend() or within a new function. I am not
really convinced that it is worth having a new interface for that.
--
Michael--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers