Re: Patch to allow users to kill their own queries

On Tue, Dec 13, 2011 at 11:59, Greg Smith <greg(at)2ndquadrant(dot)com> wrote:
> The submission from Edward Muller I'm replying to is quite similar to what
> the other raging discussion here decided was the right level to target.
>  There was one last year from Josh Kupershmidt with similar goals:
>  http://archives.postgresql.org/pgsql-admin/2010-02/msg00052.php  A good
> place to start is the concise summary of the new specification goal that Tom
> made in the other thread:
>
>> If allowing same-user cancels is enough to solve 95% or 99% of the
>> real-world use cases, let's just do that.
>
> Same-user cancels, but not termination.  Only this, and nothing more.

Good.

> Appropriate credits here would go Josh Kupershmidt, Edward Muller, and then
> myself; everyone did an equally useful chunk of this in that order.  It's
> all packaged up for useful gitsumption at
> https://github.com/greg2ndQuadrant/postgres/tree/cancel_backend too.  I
> attached it to the next CommitFest:
>  https://commitfest.postgresql.org/action/patch_view?id=722 but would enjoy
> seeing a stake finally put through its evil heart before then, as I don't
> think there's much left to do now.
>
> test=> select pg_terminate_backend(28154);
> ERROR:  must be superuser to terminate other server processes
> HINT:  you can use pg_cancel_backend() on your own processes

That HINT sounds a bit weird to me. "you can cancel your own queries
using pg_cancel_backend() instead" or something like that?

> Victory over the evil sleeping backend is complete, without a superuser in
> sight.

\o/

> There's one obvious and questionable design decision I made to highlight.
>  Right now the only consumers of pg_signal_backend are the cancel and
> terminate calls.  What I did was make pg_signal_backend more permissive,
> adding the idea that role equivalence = allowed, and therefore granting that
> to anything else that might call it.  And then I put a stricter check on
> termination.  This results in a redundant check of superuser on the
> termination check, and the potential for mis-using pg_signal_backend.  I
> documented all that and liked the result; it feels better to me to have
> pg_signal_backend provide an API that is more flexible here.  Pushback to
> structure this differently is certainly possible though, and I'm happy to
> iterate the patch to address that.  It might drift back toward something
> closer to Josh's original design.

How about passing a parameter to pg_signal_backend? Making
pg_signal_backend(int pid, int sig, bool allow_samerole)?

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