Thread

  1. pg_terminate_backend for same-role

    Daniel Farina <daniel@heroku.com> — 2012-03-15T23:14:03Z

    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.
    
    I imagine the problem is a race condition whereby a pid might be
    reused by another process owned by another user (doesn't that also
    affect pg_cancel_backend?).  Shall we just do everything using the
    MyCancelKey (which I think could just be called "SessionKey",
    "SessionSecret", or even just "Session") as to ensure we have no case
    of mistaken identity? Or does that end up being problematic?
    
    -- 
    fdr
    
    
  2. Re: pg_terminate_backend for same-role

    Fujii Masao <masao.fujii@gmail.com> — 2012-03-16T04:39:48Z

    On Fri, Mar 16, 2012 at 8:14 AM, Daniel Farina <daniel@heroku.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
    
    > I imagine the problem is a race condition whereby a pid might be
    > reused by another process owned by another user (doesn't that also
    > affect pg_cancel_backend?).
    
    Yes, but I think it's too unlikely to happen. Not sure if we really
    should worry about that.
    
    > Shall we just do everything using the
    > MyCancelKey (which I think could just be called "SessionKey",
    > "SessionSecret", or even just "Session") as to ensure we have no case
    > of mistaken identity? Or does that end up being problematic?
    
    What if pid is unfortunately reused after passing the test of MyCancelKey
    and before sending the signal?
    
    Regards,
    
    -- 
    Fujii Masao
    NIPPON TELEGRAPH AND TELEPHONE CORPORATION
    NTT Open Source Software Center
    
    
  3. Re: pg_terminate_backend for same-role

    Daniel Farina <daniel@heroku.com> — 2012-03-16T04:59:20Z

    On Thu, Mar 15, 2012 at 9:39 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
    >> Shall we just do everything using the
    >> MyCancelKey (which I think could just be called "SessionKey",
    >> "SessionSecret", or even just "Session") as to ensure we have no case
    >> of mistaken identity? Or does that end up being problematic?
    >
    > What if pid is unfortunately reused after passing the test of MyCancelKey
    > and before sending the signal?
    
    The way MyCancelKey is checked now is backwards, in my mind.  It seems
    like it would be better checked by the receiving PID (one can use a
    check/recheck also, if so inclined).  Is there a large caveat to that?
    I'm working on a small patch to do this I hope to post soon (modulus
    difficulties), but am fully aware that messing around PGPROC and
    signal handling can be a bit fiddly.
    
    -- 
    fdr
    
    
  4. Re: pg_terminate_backend for same-role

    Tom Lane <tgl@sss.pgh.pa.us> — 2012-03-16T05:33:45Z

    Daniel Farina <daniel@heroku.com> writes:
    > The way MyCancelKey is checked now is backwards, in my mind.  It seems
    > like it would be better checked by the receiving PID (one can use a
    > check/recheck also, if so inclined).  Is there a large caveat to that?
    
    You mean, other than the fact that kill(2) can't transmit such a key?
    
    But actually I don't see what you hope to gain from such a change,
    even if it can be made to work.  Anyone who can do kill(SIGINT) can
    do kill(SIGKILL), say --- so you have to be able to trust the signal
    sender.  What's the point of not trusting it to verify the client
    identity?
    
    			regards, tom lane
    
    
  5. Re: pg_terminate_backend for same-role

    Daniel Farina <daniel@heroku.com> — 2012-03-16T05:41:05Z

    On Thu, Mar 15, 2012 at 10:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > Daniel Farina <daniel@heroku.com> writes:
    >> The way MyCancelKey is checked now is backwards, in my mind.  It seems
    >> like it would be better checked by the receiving PID (one can use a
    >> check/recheck also, if so inclined).  Is there a large caveat to that?
    >
    > You mean, other than the fact that kill(2) can't transmit such a key?
    
    I was planning on using an out-of-line mechanism. Bad idea?
    
    > But actually I don't see what you hope to gain from such a change,
    > even if it can be made to work.  Anyone who can do kill(SIGINT) can
    > do kill(SIGKILL), say --- so you have to be able to trust the signal
    > sender.  What's the point of not trusting it to verify the client
    > identity?
    
    No longer true with pg_cancel_backend not-by-superuser, no?  Now there
    are new people who can do kill(SIGINT) (modulus the already existing
    cancel requests).
    
    -- 
    fdr
    
    
  6. Re: pg_terminate_backend for same-role

    Tom Lane <tgl@sss.pgh.pa.us> — 2012-03-16T05:45:45Z

    Daniel Farina <daniel@heroku.com> writes:
    > On Thu, Mar 15, 2012 at 10:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >> But actually I don't see what you hope to gain from such a change,
    >> even if it can be made to work. Anyone who can do kill(SIGINT) can
    >> do kill(SIGKILL), say --- so you have to be able to trust the signal
    >> sender. What's the point of not trusting it to verify the client
    >> identity?
    
    > No longer true with pg_cancel_backend not-by-superuser, no?
    
    No.  That doesn't affect the above argument in the least.  And in fact
    if there's any question whatsoever as to whether unprivileged
    cross-backend signals are secure, they are not going in in the first
    place.
    
    			regards, tom lane
    
    
  7. Re: pg_terminate_backend for same-role

    Daniel Farina <daniel@heroku.com> — 2012-03-16T06:01:14Z

    On Thu, Mar 15, 2012 at 10:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > Daniel Farina <daniel@heroku.com> writes:
    >> On Thu, Mar 15, 2012 at 10:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >>> But actually I don't see what you hope to gain from such a change,
    >>> even if it can be made to work.  Anyone who can do kill(SIGINT) can
    >>> do kill(SIGKILL), say --- so you have to be able to trust the signal
    >>> sender.  What's the point of not trusting it to verify the client
    >>> identity?
    >
    >> No longer true with pg_cancel_backend not-by-superuser, no?
    >
    > No.  That doesn't affect the above argument in the least.  And in fact
    > if there's any question whatsoever as to whether unprivileged
    > cross-backend signals are secure, they are not going in in the first
    > place.
    
    Okay, well, I believe there is a race in handling common
    administrative signals that *might* possibly matter.  In the past,
    pg_cancel_backend was superuser only, which is a lot like saying "only
    available to people who can be the postgres user and run kill."  The
    cancellation packet is handled via first checking the other backend's
    BackendKey and then SIGINTing it, leaving only the most narrow
    possibility for a misdirected SIGINT.
    
    But it really is unfortunate that I, a user, run a query or have a
    problematic connection of my own role and just want the thing to stop,
    but I can't do anything about it without superuser.  In recognition of
    that, pg_cancel_backend now can operate on backends owned by the same
    user (once again, checked before the signal is processed by the
    receiver, just like with the cancellation packet).
    
    There was some angst (but I'm not sure about how specific or uniform)
    to extend such signaling power to pg_terminate_backend, and the only
    objection I can think of is there is this race, or so it would seem to
    me.  Maybe it's too small to care, in which case we can just extend
    the same policy to pg_terminate_backend, or maybe it's not, in which
    case we could get rid of any signaling race conditions.
    
    The only hypothetical person who would be happy with the current
    situation, if characterized correctly, would be one who thinks that
    pid-race on SIGINT from non-superusers (long has been true in the form
    of the cancellation packet) is okay but on SIGTERM such a race is not.
    
    -- 
    fdr
    
    
  8. Re: pg_terminate_backend for same-role

    Daniel Farina <daniel@heroku.com> — 2012-03-16T09:03:24Z

    On Thu, Mar 15, 2012 at 11:01 PM, Daniel Farina <daniel@heroku.com> wrote:
    > Okay, well, I believe there is a race in handling common
    > administrative signals that *might* possibly matter.  In the past,
    > pg_cancel_backend was superuser only, which is a lot like saying "only
    > available to people who can be the postgres user and run kill."  The
    > cancellation packet is handled via first checking the other backend's
    > BackendKey and then SIGINTing it, leaving only the most narrow
    > possibility for a misdirected SIGINT.
    
    Attached is a patch that sketches a removal of the caveat by relying
    on the BackendId in PGPROC instead of the pid.  Basically, the idea is
    to make it work more like how cancellation keys work, except for
    internal SQL functions.  I think the unsatisfying crux here is the
    uniqueness of BackendId over the life of one *postmaster* invocation:
    
    sinvaladt.c
    
    	MyBackendId = (stateP - &segP->procState[0]) + 1;
    	/* Advertise assigned backend ID in MyProc */
    	MyProc->backendId = MyBackendId;
    
    I'm not sure precisely what to think about how this numbering winds up
    working on quick inspection.  Clearly, if BackendIds can be reused
    quickly then the pid-race problem comes back in spirit right away.
    
    But given the contract of MyBackendId as I understand it (it just has
    to be unique among all backends at any given time), it could be
    changed.  I don't *think* it's used for its arithmetic relationship to
    its underlying components anywhere.
    
    Another similar solution (not attached) would be to send information
    about the originating backend through PGPROC and having the check be
    against those rather than merely a correct and unambiguous
    MyBackendId.
    
    I also see now that cancellation packets does not have this caveat
    because the postmaster is control of all forking and joining in a
    serially executed path, so it need not worry about pid racing.
    
    Another nice use for this might be to, say, deliver the memory or
    performance stats of another process while-in-execution, without
    having to be superuser or and/or gdbing in back to the calling backend.
    
    -- 
    fdr
    
  9. Re: pg_terminate_backend for same-role

    Noah Misch <noah@leadboat.com> — 2012-03-16T22:42:33Z

    On Thu, Mar 15, 2012 at 04:14:03PM -0700, Daniel Farina 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
    
    > I imagine the problem is a race condition whereby a pid might be
    > reused by another process owned by another user (doesn't that also
    > affect pg_cancel_backend?).  Shall we just do everything using the
    > MyCancelKey (which I think could just be called "SessionKey",
    > "SessionSecret", or even just "Session") as to ensure we have no case
    > of mistaken identity? Or does that end up being problematic?
    
    No, I think the hazard you identify here is orthogonal to the question of when
    to authorize pg_terminate_backend().  As you note downthread, protocol-level
    cancellations available in released versions already exhibit this hazard.  I
    wouldn't mind a clean fix for this, but it's an independent subject.
    
    
    Here I discussed a hazard specific to allowing pg_terminate_backend():
    http://archives.postgresql.org/message-id/20110602045955.GC8246@tornado.gateway.2wire.net
    
    To summarize, user code can trap SIGINT cancellations, but it cannot trap
    SIGTERM terminations.  If a backend is executing a SECURITY DEFINER function
    when another backend of the same role calls pg_terminate_backend() thereon,
    the pg_terminate_backend() caller could achieve something he cannot achieve in
    PostgreSQL 9.1.  I vote that this is an acceptable loss.
    
    Thanks,
    nm
    
    
  10. Re: pg_terminate_backend for same-role

    Daniel Farina <daniel@heroku.com> — 2012-03-16T23:42:07Z

    On Fri, Mar 16, 2012 at 3:42 PM, Noah Misch <noah@leadboat.com> wrote:
    > On Thu, Mar 15, 2012 at 04:14:03PM -0700, Daniel Farina 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
    >
    >> I imagine the problem is a race condition whereby a pid might be
    >> reused by another process owned by another user (doesn't that also
    >> affect pg_cancel_backend?).  Shall we just do everything using the
    >> MyCancelKey (which I think could just be called "SessionKey",
    >> "SessionSecret", or even just "Session") as to ensure we have no case
    >> of mistaken identity? Or does that end up being problematic?
    >
    > No, I think the hazard you identify here is orthogonal to the question of when
    > to authorize pg_terminate_backend().  As you note downthread, protocol-level
    > cancellations available in released versions already exhibit this hazard.  I
    > wouldn't mind a clean fix for this, but it's an independent subject.
    
    Hmm. Well, here's a patch that implements exactly that, I think,
    similar to the one posted to this thread, but not using BackendIds,
    but rather the newly-introduced "SessionId".  Would appreciate
    comments.  Because an out-of-band signaling method has been integrated
    more complex behaviors -- such as closing the
    TERM-against-SECURITY-DEFINER-FUNCTION hazard -- can be addressed.
    For now I've only attempted to solve the problem of backend ambiguity,
    which basically necessitated out-of-line information transfer as per
    the usual means.
    
    > Here I discussed a hazard specific to allowing pg_terminate_backend():
    > http://archives.postgresql.org/message-id/20110602045955.GC8246@tornado.gateway.2wire.net
    >
    > To summarize, user code can trap SIGINT cancellations, but it cannot trap
    > SIGTERM terminations.  If a backend is executing a SECURITY DEFINER function
    > when another backend of the same role calls pg_terminate_backend() thereon,
    > the pg_terminate_backend() caller could achieve something he cannot achieve in
    > PostgreSQL 9.1.  I vote that this is an acceptable loss.
    
    I'll throw out a patch that just lets this hazard exist and see what
    happens, although it is obsoleted/incompatible with the one already
    attached.
    
    -- 
    fdr
    
  11. Re: pg_terminate_backend for same-role

    Daniel Farina <daniel@heroku.com> — 2012-03-17T00:00:35Z

    On Fri, Mar 16, 2012 at 4:42 PM, Daniel Farina <daniel@heroku.com> wrote:
    > Hmm. Well, here's a patch that implements exactly that, I think,
    
    That version had some screws loose due to some editor snafus.
    Hopefully all better.
    
    -- 
    fdr
    
  12. Re: pg_terminate_backend for same-role

    Noah Misch <noah@leadboat.com> — 2012-03-17T21:32:22Z

    On Fri, Mar 16, 2012 at 04:42:07PM -0700, Daniel Farina wrote:
    > On Fri, Mar 16, 2012 at 3:42 PM, Noah Misch <noah@leadboat.com> wrote:
    > > On Thu, Mar 15, 2012 at 04:14:03PM -0700, Daniel Farina wrote:
    > >> I imagine the problem is a race condition whereby a pid might be
    > >> reused by another process owned by another user (doesn't that also
    > >> affect pg_cancel_backend?). ?Shall we just do everything using the
    > >> MyCancelKey (which I think could just be called "SessionKey",
    > >> "SessionSecret", or even just "Session") as to ensure we have no case
    > >> of mistaken identity? Or does that end up being problematic?
    > >
    > > No, I think the hazard you identify here is orthogonal to the question of when
    > > to authorize pg_terminate_backend(). ?As you note downthread, protocol-level
    > > cancellations available in released versions already exhibit this hazard. ?I
    > > wouldn't mind a clean fix for this, but it's an independent subject.
    > 
    > Hmm. Well, here's a patch that implements exactly that, I think,
    > similar to the one posted to this thread, but not using BackendIds,
    > but rather the newly-introduced "SessionId".  Would appreciate
    > comments.  Because an out-of-band signaling method has been integrated
    > more complex behaviors -- such as closing the
    > TERM-against-SECURITY-DEFINER-FUNCTION hazard -- can be addressed.
    > For now I've only attempted to solve the problem of backend ambiguity,
    > which basically necessitated out-of-line information transfer as per
    > the usual means.
    
    This patch still changes the policy for pg_terminate_backend(), and it does
    not fix other SIGINT senders like processCancelRequest() and ProcSleep().  If
    you're concerned about PID-reuse races, audit all backend signalling.  Either
    fix all such problems or propose a plan to get there eventually.  Any further
    discussion of this topic needs a new subject line; mixing its consideration
    with proposals to change the policy behind pg_terminate_backend() reduces the
    chances of the right people commenting on these distinct questions.
    
    Currently, when pg_terminate_backend() follows a pg_cancel_backend() on which
    the target has yet to act, the eventual outcome is a terminated process.  With
    this patch, the pg_terminate_backend() becomes a no-op with this warning:
    
    > ! 			 ereport(WARNING,
    > ! 					 (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
    > ! 					  (errmsg("process is busy responding to administrative "
    > ! 							  "request")),
    > ! 					  (errhint("This is temporary, and may be retried."))));
    
    That's less useful than the current behavior.
    
    
    That being said, I can't get too excited about closing PID-reuse races.  I've
    yet to see another program do so.  I've never seen a trouble report around
    this race for any software.  Every OS I have used assigns PIDs so as to
    maximize the reuse interval, which seems like an important POLA measure given
    typical admin formulae like "kill `ps | grep ...`".  This defense can only
    matter in fork-bomb conditions, at which point a stray signal is minor.
    
    I do think it's worth keeping this idea in a back pocket for achieving those
    "more complex behaviors," should we ever desire them.
    
    > > Here I discussed a hazard specific to allowing pg_terminate_backend():
    > > http://archives.postgresql.org/message-id/20110602045955.GC8246@tornado.gateway.2wire.net
    > >
    > > To summarize, user code can trap SIGINT cancellations, but it cannot trap
    > > SIGTERM terminations. ?If a backend is executing a SECURITY DEFINER function
    > > when another backend of the same role calls pg_terminate_backend() thereon,
    > > the pg_terminate_backend() caller could achieve something he cannot achieve in
    > > PostgreSQL 9.1. ?I vote that this is an acceptable loss.
    > 
    > I'll throw out a patch that just lets this hazard exist and see what
    > happens, although it is obsoleted/incompatible with the one already
    > attached.
    
    +1.  Has anyone actually said that the PID-reuse race or the thing I mention
    above should block such a patch?  I poked back through the threads I could
    remember and found nothing.
    
    
  13. Re: pg_terminate_backend for same-role

    Daniel Farina <daniel@heroku.com> — 2012-03-20T08:38:52Z

    On Thu, Mar 15, 2012 at 9:39 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
    > On Fri, Mar 16, 2012 at 8:14 AM, Daniel Farina <daniel@heroku.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.
    
    -- 
    fdr
    
  14. Re: pg_terminate_backend for same-role

    Jeff Davis <pgsql@j-davis.com> — 2012-03-26T07:14:36Z

    On Tue, 2012-03-20 at 01:38 -0700, Daniel Farina wrote:
    > On Thu, Mar 15, 2012 at 9:39 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
    > > On Fri, Mar 16, 2012 at 8:14 AM, Daniel Farina <daniel@heroku.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.
    
    Regards,
    	Jeff Davis
    
    
    
    
  15. Re: pg_terminate_backend for same-role

    Daniel Farina <daniel@heroku.com> — 2012-03-29T07:04:20Z

    On Mon, Mar 26, 2012 at 12:14 AM, Jeff Davis <pgsql@j-davis.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.fujii@gmail.com> wrote:
    >> > On Fri, Mar 16, 2012 at 8:14 AM, Daniel Farina <daniel@heroku.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
    
  16. Re: pg_terminate_backend for same-role

    Robert Haas <robertmhaas@gmail.com> — 2012-06-26T20:58:44Z

    On Thu, Mar 29, 2012 at 3:04 AM, Daniel Farina <daniel@heroku.com> wrote:
    > On Mon, Mar 26, 2012 at 12:14 AM, Jeff Davis <pgsql@j-davis.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.fujii@gmail.com> wrote:
    >>> > On Fri, Mar 16, 2012 at 8:14 AM, Daniel Farina <daniel@heroku.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
    
    
  17. Re: pg_terminate_backend for same-role

    Daniel Farina <daniel@heroku.com> — 2012-06-26T21:45:52Z

    On Tue, Jun 26, 2012 at 1:58 PM, Robert Haas <robertmhaas@gmail.com> wrote:
    > On Thu, Mar 29, 2012 at 3:04 AM, Daniel Farina <daniel@heroku.com> wrote:
    >> On Mon, Mar 26, 2012 at 12:14 AM, Jeff Davis <pgsql@j-davis.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.fujii@gmail.com> wrote:
    >>>> > On Fri, Mar 16, 2012 at 8:14 AM, Daniel Farina <daniel@heroku.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?
    
    I think that would be good.
    
    It is at the level of pain whereas I would backpatch from day one, but
    I think it would be a welcome change to people who couldn't justify
    gnashing their teeth and distributing a tweaked Postgres like I would.
     It saves me some effort, too.
    
    -- 
    fdr
    
    
  18. Re: pg_terminate_backend for same-role

    Magnus Hagander <magnus@hagander.net> — 2012-06-27T12:13:22Z

    On Tue, Jun 26, 2012 at 10:58 PM, Robert Haas <robertmhaas@gmail.com> wrote:
    > On Thu, Mar 29, 2012 at 3:04 AM, Daniel Farina <daniel@heroku.com> wrote:
    >> On Mon, Mar 26, 2012 at 12:14 AM, Jeff Davis <pgsql@j-davis.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.fujii@gmail.com> wrote:
    >>>> > On Fri, Mar 16, 2012 at 8:14 AM, Daniel Farina <daniel@heroku.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/
    
    
  19. Re: pg_terminate_backend for same-role

    Robert Haas <robertmhaas@gmail.com> — 2012-06-27T12:47:33Z

    On Wed, Jun 27, 2012 at 8:13 AM, Magnus Hagander <magnus@hagander.net> wrote:
    >> 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.
    
    Three votes with no objections is good enough for me, so, done.
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company