Re: pg_terminate_backend for same-role

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?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company