Hi, Thank you for the reviews.
> > The write operation to beentry needs to be enclosed by > > PGSTAT_BEGIN/END_WRITE_ACTIVITY(). In that perspective, it would be > > better to move that writes to the PGSTAT_WRITE_ACTIVITY section just > > below. I have fixed it in the new version. > > 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) > > usecs_diff); > > + beentry->st_total_active_time += usecs_diff; > > + } > > > > The two lines operates exactly the same way on variables with slightly > > different behavior. pgStatActiveTime is reported at transaction end > > and reset at every tabstat reporting. st_total_active_time is reported > > immediately and reset at session end. Since we do the latter, the > > first can be omitted by remembering the last values for the local > > variables at every reporting. This needs additional two exporting > > Of course it's typo(?) of "values of the shared variables". Could you please elaborate on this idea ? So we have pgStatActiveTime and pgStatIdleInTransactionTime ultimately used to report respective metrics in pg_stat_database. Now beentry's st_total_active_time / st_total_transaction_idle_time duplicates this info, so one may get rid of pgStat*Time counters. Is the idea to report instead of them at every tabstat reporting the difference between the last memorized value of st_total_*_time and its current value ? > > This needs additional two exporting > > function in pgstatfuncs like pgstat_get_my_queryid so others might > > think differently. What would be example functions to look at ? Regards, Sergey
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 62f2a3332b..25290d1260 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -967,6 +967,28 @@ 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>active_time</structfield> <type>double precision</type> + </para> + <para> + Time in milliseconds this backend spent in <literal>active</literal> and + <literal>fastpath</literal> states. This value is + updated when a backend switches from these states to a new state. + </para></entry> + </row> + + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>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. This value is + updated when a backend switches from these states to a new state. + </para></entry> + </row> </tbody> </tgroup> </table> diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 3cb69b1f87..e349709c05 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -841,7 +841,9 @@ 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 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 2fecf26a2c..31adde5ffe 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -468,6 +468,13 @@ pgstat_beshutdown_hook(int code, Datum arg) beentry->st_procpid = 0; /* mark invalid */ + /* + * Reset per-backend counters so that accumulated values for the current + * backend are not used for future backends. + */ + beentry->st_total_active_time = 0; + beentry->st_total_transaction_idle_time = 0; + PGSTAT_END_WRITE_ACTIVITY(beentry); /* so that functions can check if backend_status.c is up via MyBEEntry */ @@ -524,6 +531,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 +559,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; @@ -583,16 +595,33 @@ pgstat_report_activity(BackendState state, const char *cmd_str) { long secs; int usecs; + int64 usecs_diff; TimestampDifference(beentry->st_state_start_timestamp, current_timestamp, &secs, &usecs); + usecs_diff = 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 (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) usecs_diff); + active_time_diff = usecs_diff; + } else - pgstat_count_conn_txn_idle_time((PgStat_Counter) secs * 1000000 + usecs); + { + pgstat_count_conn_txn_idle_time((PgStat_Counter) usecs_diff); + transaction_idle_time_diff = usecs_diff; + } } /* @@ -618,6 +647,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 15cb17ace4..7c2776c14c 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 32 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); + + /* convert to msec for display */ + values[30] = Float8GetDatum(beentry->st_total_active_time / 1000.0) ; + values[31] = Float8GetDatum(beentry->st_total_transaction_idle_time / 1000.0); } else { @@ -944,6 +948,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(tupstore, tupdesc, values, nulls); diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index b6f689e8d1..59479b8594 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5334,9 +5334,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,active_time,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/utils/backend_status.h b/src/include/utils/backend_status.h index 8217d0cb6b..96d432ce49 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; diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index d652f7b5fb..5e7ee70edd 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1764,8 +1764,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.active_time, + s.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, active_time, 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, @@ -1877,7 +1879,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, active_time, idle_in_transaction_time) WHERE (s.client_port IS NOT NULL); pg_stat_progress_analyze| SELECT s.pid, s.datid, @@ -2047,7 +2049,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, active_time, 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, @@ -2081,7 +2083,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, active_time, idle_in_transaction_time) WHERE (s.client_port IS NOT NULL); pg_stat_subscription| SELECT su.oid AS subid, su.subname,