public inbox for [email protected]  
help / color / mirror / Atom feed
From: Ian Lawrence Barwick <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: pg_stat_get_backend_subxact() and backend IDs?
Date: Thu, 24 Aug 2023 10:22:49 +0900
Message-ID: <CAB8KJ=j-ACb3H4L9a_b3ZG3iCYDW5aEu3WsPAzkm2S7JzS1Few@mail.gmail.com> (raw)

Hi

I was playing around with "pg_stat_get_backend_subxact()" (commit 10ea0f924)
and see it emits NULL values for some backends, e.g.:

    postgres=# \pset null NULL
    Null display is "NULL".

    postgres=#  SELECT id, pg_stat_get_backend_pid(id), s.*,
                       pg_stat_get_backend_activity (id)
                  FROM pg_stat_get_backend_idset() id
               JOIN LATERAL pg_stat_get_backend_subxact(id) AS s ON TRUE;
     id  | pg_stat_get_backend_pid | subxact_count |
subxact_overflowed |                pg_stat_get_backend_activity
    -----+-------------------------+---------------+--------------------+------------------------------------------------------------
       1 |                 3175972 |             0 | f
 | <command string not enabled>
       2 |                 3175973 |             0 | f
 | <command string not enabled>
       3 |                 3177889 |             0 | f
 | SELECT id, pg_stat_get_backend_pid(id), s.*,              +
         |                         |               |
 |         pg_stat_get_backend_activity (id)                 +
         |                         |               |
 |    FROM pg_stat_get_backend_idset() id                    +
         |                         |               |
 | JOIN LATERAL pg_stat_get_backend_subxact(id) AS s ON TRUE;
       4 |                 3176027 |             5 | f
 | savepoint s4;
     256 |                 3175969 |          NULL | NULL
 | <command string not enabled>
     258 |                 3175968 |          NULL | NULL
 | <command string not enabled>
     259 |                 3175971 |          NULL | NULL
 | <command string not enabled>
    (7 rows)

Reading through the thread [1], it looks like 0/false are intended to be
returned for non-backend processes too [2], so it seems odd that NULL/NULL is
getting returned in some cases, especially as that's what's returned if a
non-existent backend ID is provided.

[1] https://www.postgresql.org/message-id/flat/CAFiTN-uvYAofNRaGF4R%2Bu6_OrABdkqNRoX7V6%2BPP3H_0HuYMwg%4...
[2] https://www.postgresql.org/message-id/flat/CAFiTN-ut0uwkRJDQJeDPXpVyTWD46m3gt3JDToE02hTfONEN%3DQ%40m...

Looking at the code, this is happening because
"pgstat_fetch_stat_local_beentry()"
expects to be passed the backend ID as an integer representing a 1-based index
referring to "localBackendStatusTable", but "pg_stat_get_backend_subxact()"
is presumably intended to take the actual BackendId , as per other
"pg_stat_get_XXX()"
functions.

Also, the comment for "pgstat_fetch_stat_local_beentry()" says:

   Returns NULL if the argument is out of range (no current caller does that).

so the last part is currently incorrect.

Assuming I am not misunderstanding something here (always a
possibility, apologies
in advance if this is merely noise), what is actually needed is a function which
accepts a BackendId (as per "pgstat_fetch_stat_beentry()"), but returns a
LocalPgBackendStatus (as per "pgstat_fetch_stat_local_beentry()") like the
attached, clumsily named "pgstat_fetch_stat_backend_local_beentry()".

Regards

Ian Barwick


Attachments:

  [text/x-patch] pg_stat_get_backend_subxact-fix.v1.patch (2.5K, ../CAB8KJ=j-ACb3H4L9a_b3ZG3iCYDW5aEu3WsPAzkm2S7JzS1Few@mail.gmail.com/2-pg_stat_get_backend_subxact-fix.v1.patch)
  download | inline diff:
commit d4292d3347658f61855fd84827954f587838a324
Author: Ian Barwick <[email protected]>
Date:   Thu Aug 24 10:16:51 2023 +0900

    pg_stat_get_backend_subxact(): handle backend ID correctly

diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index 38f91a495b..bce7836264 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -1136,6 +1136,39 @@ pgstat_fetch_stat_local_beentry(int beid)
 }
 
 
+/* ----------
+ * pgstat_fetch_stat_backend_local_beentry() -
+ *
+ *	Like pgstat_fetch_stat_local_beentry() but takes the BackendId of the
+ *  desired session.
+ *
+ *	Returns NULL if the given beid doesn't identify any known session.
+ *
+ *	NB: caller is responsible for a check if the user is permitted to see
+ *	this info (especially the querystring).
+ * ----------
+ */
+
+LocalPgBackendStatus *
+pgstat_fetch_stat_backend_local_beentry(BackendId beid)
+{
+	LocalPgBackendStatus key;
+
+	pgstat_read_current_status();
+
+	/*
+	 * Since the localBackendStatusTable is in order by backend_id, we can use
+	 * bsearch() to search it efficiently.
+	 */
+	key.backend_id = beid;
+
+	return (LocalPgBackendStatus *) bsearch(&key, localBackendStatusTable,
+											localNumBackends,
+											sizeof(LocalPgBackendStatus),
+											cmp_lbestatus);
+}
+
+
 /* ----------
  * pgstat_fetch_stat_numbackends() -
  *
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 2b9742ad21..8479ea130b 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -727,7 +727,7 @@ pg_stat_get_backend_subxact(PG_FUNCTION_ARGS)
 
 	BlessTupleDesc(tupdesc);
 
-	if ((local_beentry = pgstat_fetch_stat_local_beentry(beid)) != NULL)
+	if ((local_beentry = pgstat_fetch_stat_backend_local_beentry(beid)) != NULL)
 	{
 		/* Fill values and NULLs */
 		values[0] = Int32GetDatum(local_beentry->backend_subxact_count);
diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h
index 77939a0aed..760ddc48ae 100644
--- a/src/include/utils/backend_status.h
+++ b/src/include/utils/backend_status.h
@@ -334,6 +334,7 @@ extern uint64 pgstat_get_my_query_id(void);
  */
 extern int	pgstat_fetch_stat_numbackends(void);
 extern PgBackendStatus *pgstat_fetch_stat_beentry(BackendId beid);
+extern LocalPgBackendStatus *pgstat_fetch_stat_backend_local_beentry(BackendId beid);
 extern LocalPgBackendStatus *pgstat_fetch_stat_local_beentry(int beid);
 extern char *pgstat_clip_activity(const char *raw_activity);
 


view thread (3+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: pg_stat_get_backend_subxact() and backend IDs?
  In-Reply-To: <CAB8KJ=j-ACb3H4L9a_b3ZG3iCYDW5aEu3WsPAzkm2S7JzS1Few@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox