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);
> "
This discussion got a little side-tracked by the idea of moving
wait_event to PgBackendStatus. I still think we should do that, but that
doesn't seem appropriate to backpatch, and we still need to fix this in
stable branches somehow.
So for now, I just adopted the most straightforward fix, and copied the
above pattern to pg_stat_get_backend_wait_event() and
pg_stat_get_backend_wait_event_type().
Committed and backpatched that.
On 05/02/2026 13:15, Rahila Syed wrote:
>> [Introduce "PGPROC *PidGetPGProc(pid_t)" function]
>
> +1 for the idea, do you also intend to remove AuxiliaryPidGetProc() as
> part of this change, given that all the occurrences of it are coupled with
> BackendPidGetProc() ?
Yeah, I think that'd make sense. (I haven't written that patch yet, but
I think we should do it)
- Heikki