On Sat, May 28, 2011 at 01:44:20PM -0400, Josh Kupershmidt wrote:
> Anssi and I posted some initial feedback on the patch's goals earlier.
> I would like to ultimately see users have the capability to
> pg_cancel_backend() their own queries. But I could at least conceive
> of others not wanting this behavior enabled by default. So perhaps
> this patch's approach of granting extra privs to the database owner
> could work as a first attempt. And maybe a later version could
> introduce a GUC allowing the DBA to control whether users can
> cancel/terminate their backends, or we could instead have an option
> flag to CREATE/ALTER ROLE, allowing per-user configuration.
What risks arise from unconditionally allowing these calls for the same user's
backends? `pg_cancel_backend' ought to be safe enough; the user always has
access to the standard cancellation protocol, making the SQL interface a mere
convenience (albeit a compelling one). `pg_terminate_backend' does open up
access to a new behavior, but no concrete risks come to mind.
On the other hand, this *would* be substantial new authority for database
owners. Seems like a reasonable authority to grant, though.
> It would be helpful to hear from others whether this patch's goals
> would work as a first pass at this problem, so that Torello doesn't
> waste time on a doomed approach. Also, it might be helpful to add an
> entry on the Todo list for 'allow non-superusers to use
> pg_cancel_backend()', in case this patch gets sunk.
>
> Now, a few technical comments about the patch:
> 1.) This bit looks dangerous:
> + backend = pgstat_fetch_stat_beentry(i);
> + if (backend->st_procpid == pid) {
>
> Since pgstat_fetch_stat_beentry() might return NULL.
I think you want BackendPidGetProc().
Thanks,
nm