Thread: 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
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
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
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
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> 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
Attachment
<div dir="ltr">On Wed, May 20, 2015 at 2:40 AM, Jim Nasby <<a href="mailto:Jim.Nasby@bluetreble.com">Jim.Nasby@bluetreble.com</a>>wrote:<br />><br />> On 5/19/15 9:19 PM, Fabríziode Royes Mello wrote:<br />>><br />>> We could add a second parameter to the current functions:<br/>>> allow_own_pid DEFAULT false. To me that seems better than an<br />>> entirely separateset of functions.<br />>><br />>><br />>> +1 to add a second parameter to current functions.<br/>><br />><br />> Instead of allow_own_pid, I went with skip_own_pid. I have the function still returningtrue even when it skips it's own PID... that seems a bit weird, but I think it's better than returning false. Unlesssomeone thinks it should return NULL, but I don't see that as any better either.<br />><br /><br />Isn't betteruse 'false' as the default value of the new parameter?<br /><br />Regards,<br /><br />--<br />Fabrízio de Royes Mello<br/>Consultoria/Coaching PostgreSQL<br />>> Timbira: <a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/>>> Blog: <a href="http://fabriziomello.github.io">http://fabriziomello.github.io</a><br/>>> Linkedin: <a href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a><br/>>> Github: <a href="http://github.com/fabriziomello">http://github.com/fabriziomello</a></div>
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
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
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
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
<p dir="ltr"><br /> On May 20, 2015 6:43 AM, "David Steele" <<a href="mailto:david@pgmasters.net">david@pgmasters.net</a>>wrote:<br /> ><br /> > On 5/20/15 1:40 AM, Jim Nasby wrote:<br/> > > On 5/19/15 9:19 PM, Fabrízio de Royes Mello wrote:<br /> > >> We could add a second parameterto the current functions:<br /> > >> allow_own_pid DEFAULT false. To me that seems better than an<br/> > >> entirely separate set of functions.<br /> > >><br /> > >><br /> > >>+1 to add a second parameter to current functions.<br /> > ><br /> > > Instead of allow_own_pid, I wentwith skip_own_pid. I have the function<br /> > > still returning true even when it skips it's own PID... that seemsa bit<br /> > > weird, but I think it's better than returning false. Unless someone<br /> > > thinks itshould return NULL, but I don't see that as any better either.<br /> ><br /> > +1. I agree that cancelling/killingyour own process should not be the<br /> > default behavior.<br /><p dir="ltr">-1. It breaks backwardscompatibility. I use this function a fair bit to terminate the current backend all the time. Changing the signatureis great, by make the default behavior retain the current behavior.<p dir="ltr">><br /> > --<br /> > -David Steele<br /> > <a href="mailto:david@pgmasters.net">david@pgmasters.net</a><br /> ><br />
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
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
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.
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
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
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'snow a logical replication (in that case IIRC londiste) standby, and a trigger was installed to rediredt existingwriters. 2) A function detects that something has has gone rather wrong with a session state and wants to force a reconnect. I'veseen 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.
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
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
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
<div dir="ltr"><div class="gmail_extra">On Fri, May 22, 2015 at 3:28 PM, Jim Nasby <<a href="mailto:Jim.Nasby@bluetreble.com">Jim.Nasby@bluetreble.com</a>>wrote:<br />><br />> You and Andreas think it'sfine as-is.<br />> Tom and Jon Nelson maybe don't like it as-is, but won't break backwards compatibility.<br />>David Steele and I seem fine with breaking compat., not sure about Fabrizio.<br />><br /><br /></div><div class="gmail_extra">+1to add a second parameter and -1 to break backward compatibility with the "default = true".<br /></div><divclass="gmail_extra"><br /></div><div class="gmail_extra">Regards,<br /><br /></div><div class="gmail_extra">--<br/>Fabrízio de Royes Mello<br />Consultoria/Coaching PostgreSQL<br />>> Timbira: <a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/>>> Blog: <a href="http://fabriziomello.github.io">http://fabriziomello.github.io</a><br/>>> Linkedin: <a href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a><br/>>> Github: <a href="http://github.com/fabriziomello">http://github.com/fabriziomello</a></div></div>
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
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 bunchof backends at once, and I can't think of any reason why you'd ever want to call a pg_cancel_* function with your ownPID. 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 currentsession? 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.
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 bunchof backends at once, and I can't think of any reason why you'd ever want to call a pg_cancel_* function with your ownPID. > 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 currentsession? 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
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
currentsession? 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
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 currentsession? 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
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 currentsession? 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.
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
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 currentsession? 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 discussionon the same topic went. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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