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