Re: exposing wait events for non-backends (was: Tracking wait event for latches)

From: Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Kevin Grittner <kgrittn(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: exposing wait events for non-backends (was: Tracking wait event for latches)
Date: 2017-03-15 12:14:37
Message-ID: CAGz5QCLK_SeSp0j6bcZLAvknEf3toDzqVBMciuR1Fr_L19ayyA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 14, 2017 at 1:50 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> "writer" would be better if defined as "background writer" instead?
> You are forgetting in this list autovacuum workers and the startup
> process, the latter is important for nodes in recovery.
>
Modified "writer" as "background writer". Included autovacuum workers
and startup process.

> + <entry>Type of the current server process. Possible types are:
> + autovacuum launcher, bgworker, checkpointer, client backend,
> + wal receiver, wal sender, wal writer and writer.
> + </entry>
> There should be markups around those terms. Shouldn't "wal writer" and
> "wal sender" and "wal receiver" not use any space? On HEAD a WAL
> sender is defined as "walsender".
Enclosed each type in <literal/>. Removed space from "wal sender" and
"wal receiver".

> For WAL senders, the "query" field is still filled with "walsender".
> This should be removed.
Fixed.

> @@ -365,6 +368,8 @@ CheckpointerMain(void)
> */
> AbsorbFsyncRequests();
>
> + pgstat_report_activity(STATE_RUNNING, NULL);
> +
> if (got_SIGHUP)
> It seems to me that what we want to know here are only the wait
> events, so I think that you had better drop this portion of the patch.
> It is hard to decide if an auxiliary process is idle or running, and
> this routine is a concept that applies to database backends running
> queries.
Removed this part as suggested.

I've attached the updated patches.
In 0001-Infra-to-expose-all-backend-processes-in-pg_stat_get.patch,
I've extended BackendStatusArray to to store auxiliary processes.
Backends
use slots indexed in the range from 1 to MaxBackends (inclusive), so
we can use MaxBackends + AuxBackendType + 1 as the index of the slot
for an auxiliary process. Also, I've added a backend_type to describe
the type of the backend process. The type includes:
* autovacuum launcher
* autovacuum worker
* background writer
* bgworker
* client backend
* checkpointer
* startup
* walreceiver
* walsender
* walwriter
In 0002-Expose-stats-for-all-backends.patch, I've added the required
code for reporting activity of different auxiliary processes,
autovacuum launcher and bgworker processes.
In 0003-Add-backend_type-column-in-pg_stat_get_activity.patch, I've
added a column named backend_type in pg_stat_get_activity to show the
type of the process to user.

There are some pg_stat_* functions where showing all the backends
doesn't make much sense. For example,
postgres=# SELECT pg_stat_get_backend_pid(s.backendid) AS pid,
pg_stat_get_backend_activity(s.backendid) AS query
FROM (SELECT pg_stat_get_backend_idset() AS backendid) AS s;
pid | query
-------+------------------------------------------------------------------
17300 | SELECT pg_stat_get_backend_pid(s.backendid) AS pid, +
| pg_stat_get_backend_activity(s.backendid) AS query +
| FROM (SELECT pg_stat_get_backend_idset() AS backendid) AS s;
16925 | <command string not enabled>
16927 | <command string not enabled>
16926 | <command string not enabled>
16929 | <command string not enabled>
IMHO, this scenario can be easily avoided by filtering backends using
backend_type. I'm not sure whether we should add any logic in the code
for handling such cases. Thoughts?

--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com

Attachment Content-Type Size
0003-Add-backend_type-column-in-pg_stat_get_activity.patch binary/octet-stream 8.5 KB
0002-Expose-stats-for-all-backends.patch binary/octet-stream 6.0 KB
0001-Infra-to-expose-all-backend-processes-in-pg_stat_get.patch binary/octet-stream 16.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2017-03-15 12:20:32 Re: GSOC - TOAST'ing in slices
Previous Message Sachin Kotwal 2017-03-15 11:38:51 Re: Allow pg_dumpall to work without pg_authid