public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Kirill Reshke <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Leung, Anthony <[email protected]>
Cc: Andrey M. Borodin <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Allow non-superuser to cancel superuser tasks.
Date: Thu, 11 Apr 2024 09:38:07 -0500
Message-ID: <20240411143807.GC1882158@nathanxps13> (raw)
In-Reply-To: <CALdSSPiOLADNe1ZS-1dnQXWVXgGAC6=3TBKABu9C3_97YGOoMg@mail.gmail.com>
References: <[email protected]>
<20240406010751.GA167028@nathanxps13>
<[email protected]>
<[email protected]>
<[email protected]>
<CALdSSPjVmZMcFm14dzzAGenssJAmV400wBYA=UNWMi9aJeXJMw@mail.gmail.com>
<[email protected]>
<20240410150034.GA1673069@nathanxps13>
<[email protected]>
<CALdSSPiOLADNe1ZS-1dnQXWVXgGAC6=3TBKABu9C3_97YGOoMg@mail.gmail.com>
On Thu, Apr 11, 2024 at 04:55:59PM +0500, Kirill Reshke wrote:
>> It's feeling more natural here to check that we have a superuser-owned
>> backend first, and then do a lookup of the process type.
>
>> I figured since there's no reason to rely on that behavior, we might as
>> well do a bit of future-proofing in case autovacuum workers are ever not
>> run as InvalidOid.
>
> I have combined them into this:
>
> if ((!OidIsValid(proc->roleId) || superuser_arg(proc->roleId))
> && pgstat_get_backend_type(GetNumberFromPGProc(proc)) == B_AUTOVAC_WORKER)
>
> This is both future-proofing and natural, I suppose. Downside of this
> is double checking condition (!OidIsValid(proc->roleId) ||
> superuser_arg(proc->roleId)), but i think that is ok for the sake of
> simplicity.
If we want to retain the check, IMO we might as well combine the first two
blocks like Anthony proposed:
if (!OidIsValid(proc->roleId) || superuser_arg(proc->roleId))
{
ProcNumber procNumber = GetNumberFromPGProc(proc);
PGBackendStatus procStatus = pgstat_get_beentry_by_proc_number(procNumber);
if (procStatus && procStatus->st_backendType == B_AUTOVAC_WORKER &&
!has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_AUTOVAC_WORKER))
return SIGNAL_BACKEND_NOAUTOVAC;
else if (!superuser())
return SIGNAL_BACKEND_NOSUPERUSER;
}
+ <row>
+ <entry>pg_signal_autovacuum_worker</entry>
+ <entry>Allow signaling autovacuum worker backend to cancel or terminate</entry>
+ </row>
I think we need to be more specific about what pg_cancel_backend() and
pg_terminate_backend() do for autovacuum workers. The code offers some
clues:
/*
* SIGINT is used to signal canceling the current table's vacuum; SIGTERM
* means abort and exit cleanly, and SIGQUIT means abandon ship.
*/
pqsignal(SIGINT, StatementCancelHandler);
pqsignal(SIGTERM, die);
+/* ----------
+ * pgstat_get_backend_type() -
+ *
+ * Return the backend type of the backend for the given proc number.
+ * ----------
+ */
+BackendType
+pgstat_get_backend_type(ProcNumber procNumber)
+{
+ PgBackendStatus *ret;
+
+ ret = pgstat_get_beentry_by_proc_number(procNumber);
+
+ if (!ret)
+ return B_INVALID;
+
+ return ret->st_backendType;
+}
I'm not sure we really need to introduce a new function for this. I
avoided using it in my example snippet above. But, maybe it'll come in
handy down the road...
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
view thread (19+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Allow non-superuser to cancel superuser tasks.
In-Reply-To: <20240411143807.GC1882158@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox