On 03/02/2026 14:46, Sami Imseih wrote:
>> Another thing I didn't do in this patch yet: I feel we should replace
>> BackendPidGetProc() with a function like "PGPROC *PidGetPGProc(pid_t)",
>> that would work for backends and aux processes alike. It's a common
>> pattern to call BackendPidGetProc() followed by AuxiliaryPidGetProc()
>> currently. Even for the callers that specifically want to only check
>> backend processes, I think it would be more natural to call
>> PidGetPGProc(), and then check the process type.
>
> +1 for such a function, and it could replace 6 different places ( if I counted
> correctly ) in code where this pattern is used. At minimum, shouldn't
> the fix for pg_stat_get_backend_wait_event() and
> pg_stat_get_backend_wait_event_type() follow the same pattern?
>
> "
> proc = BackendPidGetProc(pid);
> if (proc == NULL)
> proc = AuxiliaryPidGetProc(pid);
> "
Yeah, that would be the most straightforward fix. But it feels silly to
call BackendPidGetProc(pid), when we already have the ProcNumber at hand.
Come to think of it, why is wait_event_info stored in PGPROC in the
first place, rather than in PgBackendStatus? All the other
pg_stat_get_backend_*() functions just read the local PgBackendStatus copy.
That point was debated when the wait events were introduced [1] [2].
AFAICS the main motivation was that aux processes didn't have
PgBackendStatus entries, and we wanted to expose wait events for aux
processes too. That has changed since then, aux processes do have
PgBackendStatus entries now, so that argument is moot.
Because wait_event_info is fetched from PGPROC, it's not part of the
"activity snapshot". So when you run "select * pg_stat_activity"
repeatedly in the same transaction, the wait_events will change, even
though the other fields are fetched once and frozen for the duration of
the transaction. Tom pointed this out back then [1], but looks like that
point was then forgotten, as we haven't documented that exception either.
There might be a performance argument too, although I haven't done any
benchmarking and it's probably not really significant: PgBackendStatus
is accessed less frequently by other backends than PGPROC, so you might
get less cache line bouncing if wait_event_info is in PgBackendStatus
instead of PGPROC.
So how about moving wait_event_info to PgBackendStatus, per attached?
[1] https://www.postgresql.org/message-id/4067.1439561494%40sss.pgh.pa.us
[2]
https://www.postgresql.org/message-id/CA%2BTgmoZ-8ZpoUM9BGtBUP1u4dUQhC-9EpEDLzyK0dG4pKMDUwQ%40mail.gmail.com
- Heikki
