From: | Josh Kupershmidt <schmiddy(at)gmail(dot)com> |
---|---|
To: | Noah Misch <noah(at)leadboat(dot)com> |
Cc: | Torello Querci <tquerci(at)gmail(dot)com>, Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov>, Bruce Momjian <bruce(at)momjian(dot)us>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: pg_terminate_backend and pg_cancel_backend by not administrator user |
Date: | 2011-05-29 14:56:02 |
Message-ID: | BANLkTinoLXBjX8zOci=5cCcLL0VNAvPU+w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, May 29, 2011 at 5:04 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> 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.
Looking around, I see there were real problems[1] with sending SIGTERM
to individual backends back in 2005 or so, and pg_terminate_backend()
was only deemed safe enough to put in for 8.4 [2]. So expanding
pg_terminate_backend() privileges does make me a tad nervous.
Reading through those old threads made me realize this patch would
give database owners the ability to kill off autovacuum workers. Seems
like we'd want to restrict that power to superusers.
> On the other hand, this *would* be substantial new authority for database
> owners. Seems like a reasonable authority to grant, though.
And I also realized that this patch's approach might force us to
maintain a permissions wart if we ever want to implement fine-grained
control for this stuff, e.g. a per-role setting enabling self-kills.
It would be a bit lame to have to document "Use this CREATE/ALTER ROLE
flag. Or be the database owner. Or be a superuser."
>> 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().
Ah, thanks for the pointer.
Josh
--
[1] http://postgresql.1045698.n5.nabble.com/pg-terminate-backend-idea-td1930120.html
[2] http://postgresql.1045698.n5.nabble.com/Re-COMMITTERS-pgsql-Add-pg-terminate-backend-to-allow-terminating-only-a-single-td1983763i20.html
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2011-05-29 15:05:19 | Re: Getting a bug tracker for the Postgres project |
Previous Message | Tom Lane | 2011-05-29 14:40:27 | Re: [ADMIN] pg_class reltuples/relpages not updated by autovacuum/vacuum |