Hello,

I have addressed the reviews.

@Aleksander Alekseev thanks for reporting the issue. I have altered
the patch to respect the behavior of pg_stat_activity, specifically
[1]

> Another important point is that when a server process is asked to display any 
> of these statistics,
> it first fetches the most recent report emitted by the collector process and 
> then continues to use this snapshot
> for all statistical views and functions until the end of its current 
> transaction.
> So the statistics will show static information as long as you continue the 
> current transaction.

For the patch it means no computing of real-time values of
total_*_time. Here is an example to illustrate the new behavior:

=# begin;

=*# select total_active_time, total_idle_in_transaction_time from
pg_stat_activity where pid = pg_backend_pid();
 total_active_time | total_idle_in_transaction_time
-------------------+--------------------------------
             0.124 |                      10505.098

postgres=*# select pg_sleep(10);

postgres=*# select total_active_time, total_idle_in_transaction_time
from pg_stat_activity where pid = pg_backend_pid();
 total_active_time | total_idle_in_transaction_time
-------------------+--------------------------------
             0.124 |                      10505.098

postgres=*# commit;

postgres=# select total_active_time, total_idle_in_transaction_time
from pg_stat_activity where pid = pg_backend_pid();
 total_active_time | total_idle_in_transaction_time
-------------------+--------------------------------
         10015.796 |                      29322.831


[1] 
https://www.postgresql.org/docs/14/monitoring-stats.html#MONITORING-STATS-VIEWS

Regards,
Sergey
From b5298301a3f5223bd78c519ddcddbd1bec9cf000 Mon Sep 17 00:00:00 2001
From: Sergey Dudoladov <sergey.dudola...@gmail.com>
Date: Wed, 20 Apr 2022 23:47:37 +0200
Subject: [PATCH] pg_stat_activity: add 'total_active_time' and
 'total_idle_in_transaction_time'

catversion bump because of the change in the contents of pg_stat_activity

Author: Sergey Dudoladov, based on the initial version by Rafia Sabih

Reviewed-by: Aleksander Alekseev, Bertrand Drouvot, and Atsushi Torikoshi

Discussion: https://www.postgresql.org/message-id/flat/CA%2BFpmFcJF0vwi-SWW0wYO-c-FbhyawLq4tCpRDCJJ8Bq%3Dja-gA%40mail.gmail.com
---
 doc/src/sgml/monitoring.sgml                | 20 +++++++++++++
 src/backend/catalog/system_views.sql        |  4 ++-
 src/backend/utils/activity/backend_status.c | 33 ++++++++++++++++-----
 src/backend/utils/adt/pgstatfuncs.c         |  8 ++++-
 src/include/catalog/pg_proc.dat             |  6 ++--
 src/include/pgstat.h                        | 10 -------
 src/include/utils/backend_status.h          | 10 +++++++
 src/test/regress/expected/rules.out         | 12 ++++----
 8 files changed, 75 insertions(+), 28 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 7dbbab6f5c..943927fe34 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -979,6 +979,26 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   11:34   0:00 postgres: ser
        additional types.
       </para></entry>
      </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>total_active_time</structfield> <type>double precision</type>
+      </para>
+      <para>
+       Time in milliseconds this backend spent in <literal>active</literal> and
+       <literal>fastpath function call</literal> states.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>total_idle_in_transaction_time</structfield> <type>double precision</type>
+      </para>
+      <para>
+       Time in milliseconds this backend spent in <literal>idle in transaction</literal>
+       and <literal>idle in transaction (aborted)</literal> states.
+      </para></entry>
+     </row>
     </tbody>
    </tgroup>
   </table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index f369b1fc14..2ec6ea2304 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -864,7 +864,9 @@ CREATE VIEW pg_stat_activity AS
             s.backend_xmin,
             S.query_id,
             S.query,
-            S.backend_type
+            S.backend_type,
+            S.total_active_time,
+            S.total_idle_in_transaction_time
     FROM pg_stat_get_activity(NULL) AS S
         LEFT JOIN pg_database AS D ON (S.datid = D.oid)
         LEFT JOIN pg_authid AS U ON (S.usesysid = U.oid);
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index c7ed1e6d7a..8fe2929fba 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -336,6 +336,8 @@ pgstat_bestart(void)
 	lbeentry.st_activity_start_timestamp = 0;
 	lbeentry.st_state_start_timestamp = 0;
 	lbeentry.st_xact_start_timestamp = 0;
+	lbeentry.st_total_active_time = 0;
+	lbeentry.st_total_transaction_idle_time = 0;
 	lbeentry.st_databaseid = MyDatabaseId;
 
 	/* We have userid for client-backends, wal-sender and bgworker processes */
@@ -524,6 +526,8 @@ pgstat_report_activity(BackendState state, const char *cmd_str)
 	TimestampTz start_timestamp;
 	TimestampTz current_timestamp;
 	int			len = 0;
+	int64		active_time_diff = 0;
+	int64		transaction_idle_time_diff = 0;
 
 	TRACE_POSTGRESQL_STATEMENT_STATUS(cmd_str);
 
@@ -550,6 +554,9 @@ pgstat_report_activity(BackendState state, const char *cmd_str)
 			beentry->st_xact_start_timestamp = 0;
 			beentry->st_query_id = UINT64CONST(0);
 			proc->wait_event_info = 0;
+
+			beentry->st_total_active_time = 0;
+			beentry->st_total_transaction_idle_time = 0;
 			PGSTAT_END_WRITE_ACTIVITY(beentry);
 		}
 		return;
@@ -575,24 +582,31 @@ pgstat_report_activity(BackendState state, const char *cmd_str)
 	 * If the state has changed from "active" or "idle in transaction",
 	 * calculate the duration.
 	 */
-	if ((beentry->st_state == STATE_RUNNING ||
-		 beentry->st_state == STATE_FASTPATH ||
-		 beentry->st_state == STATE_IDLEINTRANSACTION ||
-		 beentry->st_state == STATE_IDLEINTRANSACTION_ABORTED) &&
+	if ((PGSTAT_IS_ACTIVE(beentry) || PGSTAT_IS_IDLEINTRANSACTION(beentry)) &&
 		state != beentry->st_state)
 	{
 		long		secs;
 		int			usecs;
+		int64		usecs_diff;
 
 		TimestampDifference(beentry->st_state_start_timestamp,
 							current_timestamp,
 							&secs, &usecs);
+		usecs_diff = secs * 1000000 + usecs;
 
-		if (beentry->st_state == STATE_RUNNING ||
-			beentry->st_state == STATE_FASTPATH)
-			pgstat_count_conn_active_time((PgStat_Counter) secs * 1000000 + usecs);
+		/*
+		 * We update per-backend st_total_active_time and st_total_transaction_idle_time
+		 * separately from pgStatActiveTime and pgStatTransactionIdleTime
+		 * used in pg_stat_database to provide per-DB statistics because
+		 * 1. Changing the former values implies modifying beentry and thus
+		 * have to be wrapped into PGSTAT_*_WRITE_ACTIVITY macros (see below).
+		 * 2. The latter values are reset to 0 once the data has been sent
+		 * to the statistics collector.
+		 */
+		if (PGSTAT_IS_ACTIVE(beentry))
+			active_time_diff = usecs_diff;
 		else
-			pgstat_count_conn_txn_idle_time((PgStat_Counter) secs * 1000000 + usecs);
+			transaction_idle_time_diff = usecs_diff;
 	}
 
 	/*
@@ -618,6 +632,9 @@ pgstat_report_activity(BackendState state, const char *cmd_str)
 		beentry->st_activity_start_timestamp = start_timestamp;
 	}
 
+	beentry->st_total_active_time += active_time_diff;
+	beentry->st_total_transaction_idle_time += transaction_idle_time_diff;
+
 	PGSTAT_END_WRITE_ACTIVITY(beentry);
 }
 
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index d9e2a79382..a213211f6a 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -536,7 +536,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
 Datum
 pg_stat_get_activity(PG_FUNCTION_ARGS)
 {
-#define PG_STAT_GET_ACTIVITY_COLS	30
+#define PG_STAT_GET_ACTIVITY_COLS	32
 	int			num_backends = pgstat_fetch_stat_numbackends();
 	int			curr_backend;
 	int			pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0);
@@ -856,6 +856,10 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 				nulls[29] = true;
 			else
 				values[29] = UInt64GetDatum(beentry->st_query_id);
+
+			/* convert to msec */
+			values[30] = Float8GetDatum(beentry->st_total_active_time / 1000.0);
+			values[31] = Float8GetDatum(beentry->st_total_transaction_idle_time / 1000.0);
 		}
 		else
 		{
@@ -884,6 +888,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 			nulls[27] = true;
 			nulls[28] = true;
 			nulls[29] = true;
+			nulls[30] = true;
+			nulls[31] = true;
 		}
 
 		tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 2e41f4d9e8..bcd106897e 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5340,9 +5340,9 @@
   proname => 'pg_stat_get_activity', prorows => '100', proisstrict => 'f',
   proretset => 't', provolatile => 's', proparallel => 'r',
   prorettype => 'record', proargtypes => 'int4',
-  proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8}',
-  proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
-  proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id}',
+  proallargtypes => '{int4,oid,int4,oid,text,text,text,text,text,timestamptz,timestamptz,timestamptz,timestamptz,inet,text,int4,xid,xid,text,bool,text,text,int4,text,numeric,text,bool,text,bool,int4,int8,float8,float8}',
+  proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
+  proargnames => '{pid,datid,pid,usesysid,application_name,state,query,wait_event_type,wait_event,xact_start,query_start,backend_start,state_change,client_addr,client_hostname,client_port,backend_xid,backend_xmin,backend_type,ssl,sslversion,sslcipher,sslbits,ssl_client_dn,ssl_client_serial,ssl_issuer_dn,gss_auth,gss_princ,gss_enc,leader_pid,query_id,total_active_time,total_idle_in_transaction_time}',
   prosrc => 'pg_stat_get_activity' },
 { oid => '3318',
   descr => 'statistics: information about progress of backends running maintenance command',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index ac28f813b4..8248e63931 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -469,10 +469,6 @@ extern void pgstat_report_connect(Oid dboid);
 	(pgStatBlockReadTime += (n))
 #define pgstat_count_buffer_write_time(n)							\
 	(pgStatBlockWriteTime += (n))
-#define pgstat_count_conn_active_time(n)							\
-	(pgStatActiveTime += (n))
-#define pgstat_count_conn_txn_idle_time(n)							\
-	(pgStatTransactionIdleTime += (n))
 
 extern PgStat_StatDBEntry *pgstat_fetch_stat_dbentry(Oid dbid);
 
@@ -673,12 +669,6 @@ extern PGDLLIMPORT PgStat_CheckpointerStats PendingCheckpointerStats;
 extern PGDLLIMPORT PgStat_Counter pgStatBlockReadTime;
 extern PGDLLIMPORT PgStat_Counter pgStatBlockWriteTime;
 
-/*
- * Updated by pgstat_count_conn_*_time macros, called by
- * pgstat_report_activity().
- */
-extern PGDLLIMPORT PgStat_Counter pgStatActiveTime;
-extern PGDLLIMPORT PgStat_Counter pgStatTransactionIdleTime;
 
 /* updated by the traffic cop and in errfinish() */
 extern PGDLLIMPORT SessionEndType pgStatSessionEndCause;
diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h
index 7403bca25e..c904714c17 100644
--- a/src/include/utils/backend_status.h
+++ b/src/include/utils/backend_status.h
@@ -168,6 +168,10 @@ typedef struct PgBackendStatus
 
 	/* query identifier, optionally computed using post_parse_analyze_hook */
 	uint64		st_query_id;
+
+	/* time spent in respective states in usec */
+	int64		st_total_active_time;
+	int64		st_total_transaction_idle_time;
 } PgBackendStatus;
 
 
@@ -231,6 +235,12 @@ typedef struct PgBackendStatus
 	((before_changecount) == (after_changecount) && \
 	 ((before_changecount) & 1) == 0)
 
+/* Macros to identify the states for time accounting */
+#define PGSTAT_IS_ACTIVE(s) \
+	((s)->st_state == STATE_RUNNING || (s)->st_state == STATE_FASTPATH)
+#define PGSTAT_IS_IDLEINTRANSACTION(s) \
+	((s)->st_state == STATE_IDLEINTRANSACTION || \
+	 (s)->st_state == STATE_IDLEINTRANSACTION_ABORTED)
 
 /* ----------
  * LocalPgBackendStatus
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 7ec3d2688f..4c9f5f8369 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1758,8 +1758,10 @@ pg_stat_activity| SELECT s.datid,
     s.backend_xmin,
     s.query_id,
     s.query,
-    s.backend_type
-   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id)
+    s.backend_type,
+    s.total_active_time,
+    s.total_idle_in_transaction_time
+   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, total_active_time, total_idle_in_transaction_time)
      LEFT JOIN pg_database d ON ((s.datid = d.oid)))
      LEFT JOIN pg_authid u ON ((s.usesysid = u.oid)));
 pg_stat_all_indexes| SELECT c.oid AS relid,
@@ -1871,7 +1873,7 @@ pg_stat_gssapi| SELECT s.pid,
     s.gss_auth AS gss_authenticated,
     s.gss_princ AS principal,
     s.gss_enc AS encrypted
-   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id)
+   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, total_active_time, total_idle_in_transaction_time)
   WHERE (s.client_port IS NOT NULL);
 pg_stat_progress_analyze| SELECT s.pid,
     s.datid,
@@ -2052,7 +2054,7 @@ pg_stat_replication| SELECT s.pid,
     w.sync_priority,
     w.sync_state,
     w.reply_time
-   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id)
+   FROM ((pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, total_active_time, total_idle_in_transaction_time)
      JOIN pg_stat_get_wal_senders() w(pid, state, sent_lsn, write_lsn, flush_lsn, replay_lsn, write_lag, flush_lag, replay_lag, sync_priority, sync_state, reply_time) ON ((s.pid = w.pid)))
      LEFT JOIN pg_authid u ON ((s.usesysid = u.oid)));
 pg_stat_replication_slots| SELECT s.slot_name,
@@ -2086,7 +2088,7 @@ pg_stat_ssl| SELECT s.pid,
     s.ssl_client_dn AS client_dn,
     s.ssl_client_serial AS client_serial,
     s.ssl_issuer_dn AS issuer_dn
-   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id)
+   FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id, total_active_time, total_idle_in_transaction_time)
   WHERE (s.client_port IS NOT NULL);
 pg_stat_subscription| SELECT su.oid AS subid,
     su.subname,
-- 
2.32.0

Reply via email to