From: | Daniel Farina <daniel(at)heroku(dot)com> |
---|---|
To: | Jeff Davis <pgsql(at)j-davis(dot)com> |
Cc: | 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-03-29 07:04:20 |
Message-ID: | CAAZKuFax3mU4b-4LRgJDyCdB_tGaP+YPLrU2xx-nj8ySqj4YzA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
--
fdr
Attachment | Content-Type | Size |
---|---|---|
Extend-same-role-backend-management-to-pg_terminate_backend-v2.patch | application/octet-stream | 4.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Dimitri Fontaine | 2012-03-29 07:51:30 | Re: Finer Extension dependencies |
Previous Message | Hitoshi Harada | 2012-03-29 06:00:26 | Re: Finer Extension dependencies |