Re: Patch to allow users to kill their own queries

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.

Relative to that goal, Ed's patch was too permissive for termination, 
and since he's new to this code it didn't check all the error conditions 
possible here.  Josh's patch had many of the right error checks, but it 
was more code than I liked for his slightly different permissions 
change.  And its attempts to be helpful leaked role information.  (That 
may have been just debugging debris left for review purposes)  I mashed 
the best bits of both together, tried to simplify the result, then 
commented heavily upon the race conditions and design decisions the code 
reflects.  Far as I can tell the patch is feature complete, including 
documentation.

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.

To demo I start with a limited user and a crazy, must be stopped backend:

$ createuser test
Shall the new role be a superuser? (y/n) n
Shall the new role be allowed to create databases? (y/n) n
Shall the new role be allowed to create more new roles? (y/n) n
$ psql -U test
test=> select pg_sleep(1000000);

Begin another session, find and try to terminate; get rejected with a 
hint, then follow it to cancel:

test=> select procpid,usename,current_query from pg_stat_activity;
-[ RECORD 1 ]-+------------------------------------------------------------
procpid       | 28154
usename       | test
current_query | select pg_sleep(1000000);

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
test=> select pg_cancel_backend(28154);
-[ RECORD 1 ]-----+--
pg_cancel_backend | t

And then this is shown on the first one:

test=> select pg_sleep(1000000);
ERROR:  canceling statement due to user request

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

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.

-- 
Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us