Re: Fix pg_stat_get_backend_wait_event() for aux processes - Mailing list pgsql-hackers

Hi,

> pg_stat_get_backend_wait_event() and
> pg_stat_get_backend_wait_event_type() functions don't work for aux
> processes:

> We added aux processes to pg_stat_activity in commit fc70a4b0df, but
> apparently forgot to do the same for those functions.

Yes, this looks like an oversight. It has probably gone unreported all
this time because pg_stat_activity is the more popular choice
for retrieving this information.

> With the attached fix:
>
> > postgres=# SELECT pg_stat_get_backend_pid(backendid) AS pid,
> >        pg_stat_get_backend_wait_event_type(backendid) as wait_event_type,
> >        pg_stat_get_backend_wait_event(backendid) as wait_event
> > FROM pg_stat_get_backend_idset() AS backendid;
> >    pid   | wait_event_type |     wait_event
> > ---------+-----------------+---------------------
> >  3667552 |                 |
> >  3667545 | Activity        | AutovacuumMain
> >  3667546 | Activity        | LogicalLauncherMain
> >  3667538 | Activity        | IoWorkerMain
> >  3667539 | Activity        | IoWorkerMain
> >  3667540 | Activity        | IoWorkerMain
> >  3667541 | Activity        | CheckpointerMain
> >  3667542 | Activity        | BgwriterMain
> >  3667544 | Activity        | WalWriterMain
> > (9 rows)
>
> While looking at this, I noticed that pg_stat_activity has a
> "backend_type" field, but there's no corresponding
> "pg_stat_get_backend_type(backend_id)" function, similar to
> "pg_stat_get_backend_wait_event(backend_id)" et al. I wonder if that was
> on purpose, or we just forgot to add it when we added it to
> pg_stat_activity?

Looks like other fields from pg_stat_activity are missing corresponding
pg_stat_get_backend_ functions as well. i.e., query_id, client_hostname,
application_name, state_change, backend_xmin, backend_xmax. Not sure
what the reason these were left out either.

It also should be noted that the information from pg_stat_get_backend_subxact
cannot be retrieved from pg_stat_activity.

> 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);
"

--
Sami Imseih
Amazon Web Services (AWS)