On Fri, 22 Oct 2021 at 10:22, Rafia Sabih <[email protected]> wrote:
>
> Hello there hackers,
>
> We at Zalando have faced some issues around long running idle
> transactions and were thinking about increasing the visibility of
> pg_stat_* views to capture them easily. What I found is that currently
> in pg_stat_activity there is a lot of good information about the
> current state of the process, but it is lacking the cumulative
> information on how much time the connection spent being idle, idle in
> transaction or active, we would like to see cumulative values for each
> of these per connection. I believe it would be helpful for us and more
> people out there if we could have total connection active and idle
> time displayed in pg_stat_activity.
>
> To provide this information I was digging into how the statistics
> collector is working and found out there is already information like
> total time that a connection is active as well as idle computed in
> pgstat_report_activity[1]. Ideally, this would be the values we would
> like to see per process in pg_stat_activity.
>
> Curious to know your thoughts on this.
>
> [1]https://github.com/postgres/postgres/blob/cd3f429d9565b2e5caf0980ea7c707e37bc3b317/src/backend/utils/activity/backend_status.c#L593
>
>
>
> --
> Regards,
> Rafia Sabih
Please find the attached patch for the idea of our intentions.
It basically adds three attributes for idle, idle_in_transaction, and
active time respectively.
Please let me know your views on this and I shall add this to the
upcoming commitfest for better tracking.
--
Regards,
Rafia Sabih
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 55f6e3711d..c721dbc0c5 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -835,7 +835,10 @@ CREATE VIEW pg_stat_activity AS
s.backend_xmin,
S.query_id,
S.query,
- S.backend_type
+ S.backend_type,
+ S.active_time,
+ S.idle_in_transaction_time,
+ S.idle_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/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index b7d0fbaefd..3e0eb963b3 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -249,6 +249,7 @@ PgStat_Counter pgStatBlockWriteTime = 0;
static PgStat_Counter pgLastSessionReportTime = 0;
PgStat_Counter pgStatActiveTime = 0;
PgStat_Counter pgStatTransactionIdleTime = 0;
+PgStat_Counter pgStatTransactionIdleInTxnTime = 0;
SessionEndType pgStatSessionEndCause = DISCONNECT_NORMAL;
/* Record that's written to 2PC state file when pgstat state is persisted */
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index 7229598822..7ced0f738b 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -590,9 +590,20 @@ pgstat_report_activity(BackendState state, const char *cmd_str)
if (beentry->st_state == STATE_RUNNING ||
beentry->st_state == STATE_FASTPATH)
- pgstat_count_conn_active_time((PgStat_Counter) secs * 1000000 + usecs);
+ {
+ pgstat_count_conn_active_time((PgStat_Counter) secs * 1000000 + usecs);
+ beentry->st_active_time = pgStatActiveTime;
+ }
+ else if (beentry->st_state == STATE_IDLEINTRANSACTION)
+ {
+ pgstat_count_conn_txn_idle_in_txn_time((PgStat_Counter) secs * 1000000 + usecs);
+ beentry->st_idle_in_transaction_time = pgStatTransactionIdleInTxnTime;
+ }
else
+ {
pgstat_count_conn_txn_idle_time((PgStat_Counter) secs * 1000000 + usecs);
+ beentry->st_idle_time = pgStatTransactionIdleTime;
+ }
}
/*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index ff5aedc99c..8044318533 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -567,7 +567,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 33
int num_backends = pgstat_fetch_stat_numbackends();
int curr_backend;
int pid = PG_ARGISNULL(0) ? -1 : PG_GETARG_INT32(0);
@@ -916,6 +916,10 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
nulls[29] = true;
else
values[29] = UInt64GetDatum(beentry->st_query_id);
+
+ values[30] = Int64GetDatum(beentry->st_active_time);
+ values[31] = Int64GetDatum(beentry->st_idle_in_transaction_time);
+ values[32] = Int64GetDatum(beentry->st_idle_time);
}
else
{
@@ -944,6 +948,9 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
nulls[27] = true;
nulls[28] = true;
nulls[29] = true;
+ values[30] = true;
+ values[31] = true;
+ values[32] = true;
}
tuplestore_putvalues(tupstore, tupdesc, values, nulls);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index d068d6532e..6f2b1a8dbd 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5348,9 +5348,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,int4,int4},int4',
+ 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,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,active_time,idle_in_transaction_time,idle_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 bcd3588ea2..916764a02a 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -985,6 +985,7 @@ extern PgStat_Counter pgStatBlockWriteTime;
*/
extern PgStat_Counter pgStatActiveTime;
extern PgStat_Counter pgStatTransactionIdleTime;
+extern PgStat_Counter pgStatTransactionIdleInTxnTime;
/*
@@ -1092,6 +1093,8 @@ extern void pgstat_initstats(Relation rel);
(pgStatActiveTime += (n))
#define pgstat_count_conn_txn_idle_time(n) \
(pgStatTransactionIdleTime += (n))
+#define pgstat_count_conn_txn_idle_in_txn_time(n) \
+ (pgStatTransactionIdleInTxnTime += (n))
extern void pgstat_count_heap_insert(Relation rel, PgStat_Counter n);
extern void pgstat_count_heap_update(Relation rel, bool hot);
diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h
index 8042b817df..063262e6ae 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;
+
+ int64 st_active_time;
+ int64 st_idle_in_transaction_time;
+ int64 st_idle_time;
} PgBackendStatus;
