Re: Allow non-superuser to cancel superuser tasks.

[Nathan Bossart]

On Fri, Nov 22, 2024 at 06:13:16PM -0500, Andres Freund wrote:
>> -            if (procStatus && procStatus->st_backendType == 
>> B_AUTOVAC_WORKER)
>> +            if (pgstat_get_backend_type(pid) == B_AUTOVAC_WORKER)
> 
> Because we already mapped the pid to a ProcNumber, it'd be cheaper to access
> the backend status via procnumber.

D'oh, I missed that ProcNumber could be used as an index for the
BackendStatusArray.  Is the attached more like what you are imagining?

> We don't need the pgstat_begin_read_activity() protocol when just accessing a
> single 4 byte value - we assume in lots of places that can be read in a
> non-tearable way.
> 
>> +                    if (pgstat_read_activity_complete(before_changecount,
>> +                                                                            
>>           after_changecount))
>> +                            break;
>> +
>> +                    CHECK_FOR_INTERRUPTS();
>> +            }
>> +
>> +            if (found)
>> +                    return beentry->st_backendType;
> 
> But if we were to follow it, we'd certainly need to use it here too.

I see.

-- 
nathan

>From 50220e60b8f1e15a63bdfec22cdaef2ebce549d6 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Fri, 22 Nov 2024 20:39:35 -0600
Subject: [PATCH v2 1/1] avoid calling pgstat_read_current_status() in
 pg_signal_backend()

---
 src/backend/storage/ipc/signalfuncs.c       |  4 ++--
 src/backend/utils/activity/backend_status.c | 11 +++++++++++
 src/include/utils/backend_status.h          |  1 +
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/backend/storage/ipc/signalfuncs.c 
b/src/backend/storage/ipc/signalfuncs.c
index aa729a36e3..d835327600 100644
--- a/src/backend/storage/ipc/signalfuncs.c
+++ b/src/backend/storage/ipc/signalfuncs.c
@@ -88,9 +88,9 @@ pg_signal_backend(int pid, int sig)
        if (!OidIsValid(proc->roleId) || superuser_arg(proc->roleId))
        {
                ProcNumber      procNumber = GetNumberFromPGProc(proc);
-               PgBackendStatus *procStatus = 
pgstat_get_beentry_by_proc_number(procNumber);
+               BackendType backendType = 
pgstat_get_backend_type_by_proc_number(procNumber);
 
-               if (procStatus && procStatus->st_backendType == 
B_AUTOVAC_WORKER)
+               if (backendType == B_AUTOVAC_WORKER)
                {
                        if (!has_privs_of_role(GetUserId(), 
ROLE_PG_SIGNAL_AUTOVACUUM_WORKER))
                                return SIGNAL_BACKEND_NOAUTOVAC;
diff --git a/src/backend/utils/activity/backend_status.c 
b/src/backend/utils/activity/backend_status.c
index bdb3a296ca..19796909e7 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -1134,6 +1134,17 @@ pgstat_get_local_beentry_by_index(int idx)
 }
 
 
+BackendType
+pgstat_get_backend_type_by_proc_number(ProcNumber procNumber)
+{
+       volatile PgBackendStatus *backendStatus;
+
+       backendStatus = &BackendStatusArray[procNumber];
+
+       return backendStatus->st_backendType;
+}
+
+
 /* ----------
  * pgstat_fetch_stat_numbackends() -
  *
diff --git a/src/include/utils/backend_status.h 
b/src/include/utils/backend_status.h
index 97874300c3..9640820edd 100644
--- a/src/include/utils/backend_status.h
+++ b/src/include/utils/backend_status.h
@@ -334,6 +334,7 @@ extern int  pgstat_fetch_stat_numbackends(void);
 extern PgBackendStatus *pgstat_get_beentry_by_proc_number(ProcNumber 
procNumber);
 extern LocalPgBackendStatus 
*pgstat_get_local_beentry_by_proc_number(ProcNumber procNumber);
 extern LocalPgBackendStatus *pgstat_get_local_beentry_by_index(int idx);
+extern BackendType pgstat_get_backend_type_by_proc_number(ProcNumber 
procNumber);
 extern char *pgstat_clip_activity(const char *raw_activity);
 
 
-- 
2.39.5 (Apple Git-154)