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

Robert Haas wrote:

So, one of the problems in this patch as committed is that for any
process that doesn't show up in pg_stat_activity, there's no way to
see the wait event information. That sucks. I think there are
basically two ways to fix this:

1. Show all processes that have a PGPROC in pg_stat_activity,
including auxiliary processes and whatnot, and use some new field in
pg_stat_activity to indicate the process type.

2. Add a second view, say pg_stat_system_activity, to show the
processes that don't appear in pg_stat_activity. A bunch of columns
could likely be omitted, but there would be some duplication, too.

Preferences?

I vote 1.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Robert Haas wrote:

So, one of the problems in this patch as committed is that for any
process that doesn't show up in pg_stat_activity, there's no way to
see the wait event information. That sucks. I think there are
basically two ways to fix this:

1. Show all processes that have a PGPROC in pg_stat_activity,
including auxiliary processes and whatnot, and use some new field in
pg_stat_activity to indicate the process type.

2. Add a second view, say pg_stat_system_activity, to show the
processes that don't appear in pg_stat_activity. A bunch of columns
could likely be omitted, but there would be some duplication, too.

Preferences?

I vote 1.

If we go with #2, there would immediately be a need for a union view,
which would end up being exactly the same thing as the expanded display
proposed in #1. Seems like the hard way, so I agree with Alvaro.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2016-12-12 13:26:32 -0300, Alvaro Herrera wrote:

Robert Haas wrote:

So, one of the problems in this patch as committed is that for any
process that doesn't show up in pg_stat_activity, there's no way to
see the wait event information. That sucks. I think there are
basically two ways to fix this:

1. Show all processes that have a PGPROC in pg_stat_activity,
including auxiliary processes and whatnot, and use some new field in
pg_stat_activity to indicate the process type.

2. Add a second view, say pg_stat_system_activity, to show the
processes that don't appear in pg_stat_activity. A bunch of columns
could likely be omitted, but there would be some duplication, too.

Preferences?

I vote 1.

+1

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Dec 12, 2016 at 9:19 AM, Robert Haas <robertmhaas@gmail.com> wrote:

So, one of the problems in this patch as committed is that for any
process that doesn't show up in pg_stat_activity, there's no way to
see the wait event information. That sucks. I think there are
basically two ways to fix this:

1. Show all processes that have a PGPROC in pg_stat_activity,
including auxiliary processes and whatnot, and use some new field in
pg_stat_activity to indicate the process type.

2. Add a second view, say pg_stat_system_activity, to show the
processes that don't appear in pg_stat_activity. A bunch of columns
could likely be omitted, but there would be some duplication, too.

Preferences?

​I'm inclined toward option 2.

A view over both that involves just the shared columns would give you the
benefits from option 1.

Question: is there a parent-child relationship involved here? Given a
record in today's pg_stat_activity is it useful/possible to link in all of
the pg_stat_system_activity​ process that are working to fulfill the
client-initiated task?

Even with "system" we'd probably want to distinguish between background
workers and true system maintenance processes. Or am I mis-interpreting
the scope of this feature?

David J.

On Mon, Dec 12, 2016 at 11:19 AM, Robert Haas <robertmhaas@gmail.com> wrote:

So, one of the problems in this patch as committed is that for any
process that doesn't show up in pg_stat_activity, there's no way to
see the wait event information. That sucks. I think there are
basically two ways to fix this:

1. Show all processes that have a PGPROC in pg_stat_activity,
including auxiliary processes and whatnot, and use some new field in
pg_stat_activity to indicate the process type.

2. Add a second view, say pg_stat_system_activity, to show the
processes that don't appear in pg_stat_activity. A bunch of columns
could likely be omitted, but there would be some duplication, too.

Preferences?

And now I'm noticing that Michael Paquier previously started a thread
on this problem which I failed to note before starting this one:

/messages/by-id/CAB7nPqSYN05rGsYCTahxTz+2hBikh7=m+hr2JTXaZv_Ei=qJAg@mail.gmail.com

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Dec 12, 2016 at 10:33 AM, Andres Freund <andres@anarazel.de> wrote:

On 2016-12-12 13:26:32 -0300, Alvaro Herrera wrote:

Robert Haas wrote:

1. Show all processes that have a PGPROC in pg_stat_activity,

2. Add a second view, say pg_stat_system_activity,

I vote 1.

+1

+1

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Dec 13, 2016 at 1:45 AM, Robert Haas <robertmhaas@gmail.com> wrote:

And now I'm noticing that Michael Paquier previously started a thread
on this problem which I failed to note before starting this one:

/messages/by-id/CAB7nPqSYN05rGsYCTahxTz+2hBikh7=m+hr2JTXaZv_Ei=qJAg@mail.gmail.com

Yes. I already had a look at what could be done to expose the
auxiliary processes in a system view with a set of proposals, though I
am not sure what would be better. It would be better to move the
discussion on this thread in my opinion, we are tracking down a
solution for a new problem.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 13 December 2016 at 01:45, Kevin Grittner <kgrittn@gmail.com> wrote:

On Mon, Dec 12, 2016 at 10:33 AM, Andres Freund <andres@anarazel.de> wrote:

On 2016-12-12 13:26:32 -0300, Alvaro Herrera wrote:

Robert Haas wrote:

1. Show all processes that have a PGPROC in pg_stat_activity,

2. Add a second view, say pg_stat_system_activity,

I vote 1.

+1

+1

I've long wanted the ability to see auxillary process state in
pg_stat_activity, so +1.

Right now pg_stat_replication is a join over pg_stat_get_activity()
and pg_stat_get_wal_senders() filtered for walsenders, and
pg_stat_activity is a view over pg_stat_get_activity() filtered for
processes with a user id. I'd really like to see walsenders in
pg_stat_activity now that logical decoding makes them more than dumb
I/O channels, as well as other auxillary processes.

We should probably expose a proc_type or something, with types:

* client_backend
* bgworker
* walsender
* autovacuum
* checkpointer
* bgwriter

for simpler filtering.

I don't think existing user code is likely to get upset by more
processes appearing in pg_stat_activity, and it'll be very handy.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Dec 13, 2016 at 10:05 AM, Craig Ringer <craig@2ndquadrant.com> wrote:

We should probably expose a proc_type or something, with types:

* client_backend
* bgworker
* walsender
* autovacuum
* checkpointer
* bgwriter

A text field is adapted then, more than a single character.

for simpler filtering.

I don't think existing user code is likely to get upset by more
processes appearing in pg_stat_activity, and it'll be very handy.

Indeed, for WAL senders now abusing of the query field is definitely
not consistent. Even if having this information is useful, adding such
a column would make sense. Still, one thing that is important to keep
with pg_stat_activity is the ability to count the number of
connections that are part of max_connections for monitoring purposes.
The docs definitely would need an example of such a query counting
only client_backend and WAL senders and tell users that this can be
used to count how many active connections there are.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 13 December 2016 at 09:13, Michael Paquier <michael.paquier@gmail.com> wrote:

On Tue, Dec 13, 2016 at 10:05 AM, Craig Ringer <craig@2ndquadrant.com> wrote:

We should probably expose a proc_type or something, with types:

* client_backend
* bgworker
* walsender
* autovacuum
* checkpointer
* bgwriter

A text field is adapted then, more than a single character.

for simpler filtering.

I don't think existing user code is likely to get upset by more
processes appearing in pg_stat_activity, and it'll be very handy.

Indeed, for WAL senders now abusing of the query field is definitely
not consistent. Even if having this information is useful, adding such
a column would make sense. Still, one thing that is important to keep
with pg_stat_activity is the ability to count the number of
connections that are part of max_connections for monitoring purposes.
The docs definitely would need an example of such a query counting
only client_backend and WAL senders and tell users that this can be
used to count how many active connections there are.

Good point.

No need for a new field, since a non-null client_port should be
sufficient. But definitely documented.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Dec 12, 2016 at 8:13 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Tue, Dec 13, 2016 at 10:05 AM, Craig Ringer <craig@2ndquadrant.com> wrote:

We should probably expose a proc_type or something, with types:

* client_backend
* bgworker
* walsender
* autovacuum
* checkpointer
* bgwriter

A text field is adapted then, more than a single character.

Sure.

for simpler filtering.

I don't think existing user code is likely to get upset by more
processes appearing in pg_stat_activity, and it'll be very handy.

Indeed, for WAL senders now abusing of the query field is definitely
not consistent. Even if having this information is useful, adding such
a column would make sense. Still, one thing that is important to keep
with pg_stat_activity is the ability to count the number of
connections that are part of max_connections for monitoring purposes.
The docs definitely would need an example of such a query counting
only client_backend and WAL senders and tell users that this can be
used to count how many active connections there are.

Let's confine ourselves to fixing one problem at a time. I think we
can get where we want to be in this case by adding one new column and
some new rows to pg_stat_activity. Michael, is that something you're
going to do? If not, one of my esteemed colleagues here at
EnterpriseDB will have a try.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Dec 12, 2016 at 6:45 PM, Kevin Grittner <kgrittn@gmail.com> wrote:

On Mon, Dec 12, 2016 at 10:33 AM, Andres Freund <andres@anarazel.de>
wrote:

On 2016-12-12 13:26:32 -0300, Alvaro Herrera wrote:

Robert Haas wrote:

1. Show all processes that have a PGPROC in pg_stat_activity,

2. Add a second view, say pg_stat_system_activity,

I vote 1.

+1

+1

+1 more, if we're still counting.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Tue, Dec 13, 2016 at 11:40:54AM -0500, Robert Haas wrote:

Let's confine ourselves to fixing one problem at a time. I think we
can get where we want to be in this case by adding one new column and
some new rows to pg_stat_activity.

Agreed. Let's also remove the abuse of WAL senders with the query field
at the same time.

Michael, is that something you're
going to do? If not, one of my esteemed colleagues here at
EnterpriseDB will have a try.

If I had received feedback on the other thread, I would have coded a
proposal of patch already. But as long as SCRAM is not done I will
restrain from taking an extra project. I am fine to do reviews as I already
looked at ways to solve the problem though. So if anybody has room to do
it please be my guest.

Regarding the way to solve things, I think that having in ProcGlobal an
array of PGPROC entries for each auxiliary process is the way to go, the
position of each entry in the array defining what the process type is.
That would waste some shared memory, but we are not talking about that
much here. That would as well remove the need of having checkpointerLatch,
walWriteLatch and the startup fields in ProcGlobal.
--
Michael

Hello everyone,

As discussed in this thread, I've attached a set of patches to show
auxiliary processes, autovacuum launcher and bgworker along with other
backends in pg_stat_activity. For now, I've extended
BackendStatusArray to store auxiliary processes. Backends use slots
indexed in the range from 1 to MaxBackends (inclusive), so we use
MaxBackends + AuxProcType + 1 as the index of the slot for an
auxiliary process. However, BackendStatusArray should be renamed to
something like 'ProcStatusArray' along with many others in pgstat.c
and pgstatfuncs.c(Ex: LocalPgBackendStatus etc). But, that needs a
major code refactoring. I can do the changes if we agree with that.

I've also kept a local array, named localBackendStatusIndex, which
stores the index of currently active backends from BackendStatusArray.
It assigns ids to currently active backend from 1 to the number of
active backends.(It is required in some pgstat_* functions, for
example: pg_stat_get_backend_idset). Hence, we are not affecting the
outputs of other sql functions apart from pg_stat_activity and
pg_stat_get_activity.

I've also added an extra column, named proc_type (suggested by Craig
and Robert), to indicate the type of process in pg_stat_activity view.
proc_type includes:

* client backend
* autovacuum launcher
* wal sender
* bgworker
* writer
* checkpointer
* wal writer
* wal receiver

Here is the present output with the relevant columns. (Didn't show
backend_start since it takes too long space)

postgres=# select pid, usesysid, application_name, wait_event_type,
wait_event, state, proc_type from pg_stat_activity;
pid | usesysid | application_name | wait_event_type |
wait_event | state | proc_type
--------+----------+------------------------------+-----------------+---------------------+--------+---------------------
109945 | | | Activity |
AutoVacuumMain | idle | autovacuum launcher
109947 | | logical replication launcher | Activity |
LogicalLauncherMain | idle | bgworker
109962 | 10 | walreceiver | Activity |
WalSenderMain | idle | wal sender
109976 | 10 | psql | |
| active | client backend
109943 | | | Activity |
BgWriterMain | idle | writer
109942 | | | Activity |
CheckpointerMain | idle | checkpointer
109944 | | | Activity |
WalWriterMain | idle | wal writer
(7 rows)

Whereas, the output of other pgstat_* functions remains unchanged. For example,

postgres=# SELECT pg_stat_get_backend_pid(s.backendid) AS procpid,
pg_stat_get_backend_activity(s.backendid) AS current_query
FROM (SELECT pg_stat_get_backend_idset() AS backendid) AS s;
procpid | current_query
---------+-------------------------------------------------------------------
120713 | SELECT pg_stat_get_backend_pid(s.backendid) AS procpid, +
| pg_stat_get_backend_activity(s.backendid) AS current_query+
| FROM (SELECT pg_stat_get_backend_idset() AS backendid) AS s;

Thoughts?

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

Hi Kuntal,

Patches apply and compile fine. Works as advertised.

Some minor comments on the patches themselves.

In 0001:

- * pgstat_bestart() -
+ * pgstat_procstart() -
+ *
+ *  Initialize this process's entry in the PgBackendStatus array.
+ *  Called from InitPostgres and AuxiliaryProcessMain.

Not being called from AuxiliaryProcessMain(). Maybe leftover comment from
a previous version. Actually I see that in patch 0002, Main() functions
of various auxiliary processes call pgstat_procstart, not
AuxiliaryProcessMain.

+ * user-defined functions which expects ids of backends starting from
1 to

s/expects/expect/g

+/*
+ * AuxiliaryPidGetProc -- get PGPROC for an auxiliary process
+ * given its PID
+ *
+ * Returns NULL if not found.
+ */
+PGPROC *
+AuxiliaryPidGetProc(int pid)
+{
+    PGPROC     *result;

Initialize to NULL so that the comment above is true. :)

In 0002:

@@ -248,6 +248,9 @@ BackgroundWriterMain(void)
*/
prev_hibernate = false;

+    /* report walwriter process in the PgBackendStatus array */
+    pgstat_procstart();
+

s/walwriter/writer/g

Patch 0004 should update monitoring.sgml.

Thanks,
Amit

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello Amit,

On Tue, Mar 7, 2017 at 4:24 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Hi Kuntal,

Patches apply and compile fine. Works as advertised.

Some minor comments on the patches themselves.

Thanks for the review.

In 0001:

- * pgstat_bestart() -
+ * pgstat_procstart() -
+ *
+ *  Initialize this process's entry in the PgBackendStatus array.
+ *  Called from InitPostgres and AuxiliaryProcessMain.

Not being called from AuxiliaryProcessMain(). Maybe leftover comment from
a previous version. Actually I see that in patch 0002, Main() functions
of various auxiliary processes call pgstat_procstart, not
AuxiliaryProcessMain.

Fixed.

+ * user-defined functions which expects ids of backends starting from
1 to

s/expects/expect/g

Fixed.

+/*
+ * AuxiliaryPidGetProc -- get PGPROC for an auxiliary process
+ * given its PID
+ *
+ * Returns NULL if not found.
+ */
+PGPROC *
+AuxiliaryPidGetProc(int pid)
+{
+    PGPROC     *result;

Initialize to NULL so that the comment above is true. :)

Fixed.

In 0002:

@@ -248,6 +248,9 @@ BackgroundWriterMain(void)
*/
prev_hibernate = false;

+    /* report walwriter process in the PgBackendStatus array */
+    pgstat_procstart();
+

s/walwriter/writer/g

Fixed.

Patch 0004 should update monitoring.sgml.

Added.

I've attached the updated patches. PFA.

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

Perhaps I'm confused by the title of this thread/CF entry, but
background workers already do show up in pg_stat_activity. (You can
verify that by testing the background sessions patch.) So which
additional things are we aiming to see with this?

In practice, I think it's common to do a quick select * from
pg_stat_activity to determine whether a database instance is in use.
(You always see your own session, but that's easy to eyeball.) If we
add all the various background processes by default, that will make
things harder, especially if there is no straightforward way to filter
them out.

Perhaps a pg_stat_user_* and pg_stat_system_* split like we have for
some of the statistics tables would be useful?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Mar 9, 2017 at 2:30 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

Perhaps I'm confused by the title of this thread/CF entry, but
background workers already do show up in pg_stat_activity. (You can
verify that by testing the background sessions patch.) So which
additional things are we aiming to see with this?

All the processes that don't normally show up in pg_stat_activity,
such as auxiliary processes.

In practice, I think it's common to do a quick select * from
pg_stat_activity to determine whether a database instance is in use.
(You always see your own session, but that's easy to eyeball.) If we
add all the various background processes by default, that will make
things harder, especially if there is no straightforward way to filter
them out.

Perhaps a pg_stat_user_* and pg_stat_system_* split like we have for
some of the statistics tables would be useful?

I thought of the same kind of thing, and it was discussed upthread.
There seemed to be more votes for keeping it all in one view, but that
could change if more people vote.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

On 2017-03-09 14:30:21 -0500, Peter Eisentraut wrote:

In practice, I think it's common to do a quick select * from
pg_stat_activity to determine whether a database instance is in use.

(You always see your own session, but that's easy to eyeball.) If we
add all the various background processes by default, that will make
things harder, especially if there is no straightforward way to filter
them out.

A good chunk of those still apply to database attached background
workers (say dropping a database, using it as a template) - so I'm not
really convinced that's an issue.

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers