Skip site navigation (1) Skip section navigation (2)

Re: pg_terminate_backend for same-role

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Daniel Farina <daniel(at)heroku(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_terminate_backend for same-role
Date: 2012-06-27 12:13:22
Message-ID: CABUevEwwQjf+ucqWMZKxk2fns3X07b=AFYQxAZvVJMEoqzp79w@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On Tue, Jun 26, 2012 at 10:58 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Mar 29, 2012 at 3:04 AM, Daniel Farina <daniel(at)heroku(dot)com> wrote:
>> On Mon, Mar 26, 2012 at 12:14 AM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>>> On Tue, 2012-03-20 at 01:38 -0700, Daniel Farina wrote:
>>>> On Thu, Mar 15, 2012 at 9:39 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>>> > On Fri, Mar 16, 2012 at 8:14 AM, Daniel Farina <daniel(at)heroku(dot)com> wrote:
>>>> >> Parallel to pg_cancel_backend, it'd be nice to allow the user to just
>>>> >> outright kill a backend that they own (politely, with a SIGTERM),
>>>> >> aborting any transactions in progress, including the idle transaction,
>>>> >> and closing the socket.
>>>> >
>>>> > +1
>>>>
>>>> Here's a patch implementing the simple version, with no more guards
>>>> against signal racing than have been seen previously.  The more
>>>> elaborate variants to close those races is being discussed in a
>>>> parallel thread, but I thought I'd get this simple version out there.
>>>
>>> Review:
>>>
>>> After reading through the threads, it looks like there was no real
>>> objection to this approach -- pid recycling is not something we're
>>> concerned about.
>>>
>>> I think you're missing a doc update though, in func.sgml:
>>>
>>> "For the less restrictive <function>pg_cancel_backend</>, the role of an
>>> active backend can be found from
>>> the <structfield>usename</structfield> column of the
>>> <structname>pg_stat_activity</structname> view."
>>>
>>> Also, high-availability.sgml makes reference to pg_cancel_backend(), and
>>> it might be worthwhile to add an "...and pg_terminate_backend()" there.
>>>
>>> Other than that, it looks good to me.
>>
>> Good comments. Patch attached to address the doc issues.  The only
>> iffy thing is that the paragraph "For the less restrictive..." I have
>> opted to remove in its entirely.  I think the documents are already
>> pretty clear about the same-user rule, and I'm not sure if this is the
>> right place for a crash-course on attributes in pg_stat_activity (but
>> maybe it is).
>>
>> "...and pg_terminate_backend" seems exactly right.
>>
>> And I think now that the system post-patch doesn't have such a strange
>> contrast between the ability to send SIGINT vs. SIGTERM, such a
>> contrast may not be necessary.
>>
>> I'm also not sure what the policy is about filling paragraphs in the
>> manual.  I filled one, which increases the sgml churn a bit. git
>> (show|diff) --word-diff helps clean it up.
>
> I went ahead and committed this.
>
> I kinda think we should back-patch this into 9.2.  It doesn't involve
> a catalog change, and would make the behavior consistent between the
> two releases, instead of changing in 9.1 and then changing again in
> 9.2.  Thoughts?

+1.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

In response to

Responses

pgsql-hackers by date

Next:From: Ants AasmaDate: 2012-06-27 12:23:04
Subject: Re: [PATCH] Lazy hashaggregate when no aggregation is needed
Previous:From: Jan UrbańskiDate: 2012-06-27 11:57:06
Subject: Re: plpython issue with Win64 (PG 9.2)

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group