On Tue, 2 Nov 2021 at 09:00, Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Tue, Oct 26, 2021 at 5:17 PM Rafia Sabih <rafia.pghack...@gmail.com> wrote: > > > > > > > > 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 for the idea > Thanks!
> > 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. > > About the patch, IIUC earlier all the idle time was accumulated in the > "pgStatTransactionIdleTime" counter, now with your patch you have > introduced one more counter which specifically tracks the > STATE_IDLEINTRANSACTION state. But my concern is that the > STATE_IDLEINTRANSACTION_ABORTED is still computed under STATE_IDLE and > that looks odd to me. Either STATE_IDLEINTRANSACTION_ABORTED should > be accumulated in the "pgStatTransactionIdleInTxnTime" counter or > there should be a separate counter for that. But after your patch we > can not accumulate this in the "pgStatTransactionIdleTime" counter. > As per your comments I have added it in pgStatTransactionIdleInTxnTime. Please let me know if there are any further comments. -- Regards, Rafia Sabih
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index eb560955cd..4dfa33ffa9 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -839,7 +839,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..805cf7ae1e 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -590,9 +590,21 @@ 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 || + beentry->st_state == STATE_IDLEINTRANSACTION_ABORTED) + { + 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;