Hi,
> > 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 ?
>
> Exactly. The attached first diff is the schetch of that.
This diff actually adds more code than it removes and somewhat bloats the patch.
I decided to incorporate it anyway because the diff explicitly shows
that time differences since the last report
are send to the statistics collector,which is not immediately evident
from the existing PgStat*Time counters.
That point may be worth further discussion though.
> And, it seems like I forgot to mention this, but as Kuntal suggested
> (in a different context and objective, though) upthraed, I think that
> we can show realtime values in the two time fields by adding the time
> of the current state. See the attached second diff.
That is exactly what we need in our infra, also included into the patch.
@Kyotaro Horiguchi
Thank you for the contribution. I included both of your diffs with
minor changes.
Should I add you to the authors of the patch given that now half of it
is basically your code ?
Regards,
Sergey
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 62f2a3332b..bc76016834 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -967,6 +967,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>active_time</structfield> <type>double precision</type>
+ </para>
+ <para>
+ Time in milliseconds this backend spent in <literal>active</literal> and
+ <literal>fastpath</literal> states.
+ </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.
+ </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/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 0646f53098..8b84533953 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -249,8 +249,8 @@ static int pgStatXactRollback = 0;
PgStat_Counter pgStatBlockReadTime = 0;
PgStat_Counter pgStatBlockWriteTime = 0;
static PgStat_Counter pgLastSessionReportTime = 0;
-PgStat_Counter pgStatActiveTime = 0;
-PgStat_Counter pgStatTransactionIdleTime = 0;
+PgStat_Counter pgStatLastActiveTime = 0;
+PgStat_Counter pgStatLastTransactionIdleTime = 0;
SessionEndType pgStatSessionEndCause = DISCONNECT_NORMAL;
/* Record that's written to 2PC state file when pgstat state is persisted */
@@ -1026,8 +1026,12 @@ pgstat_send_tabstat(PgStat_MsgTabstat *tsmsg, TimestampTz now)
TimestampDifference(pgLastSessionReportTime, now, &secs, &usecs);
pgLastSessionReportTime = now;
tsmsg->m_session_time = (PgStat_Counter) secs * 1000000 + usecs;
- tsmsg->m_active_time = pgStatActiveTime;
- tsmsg->m_idle_in_xact_time = pgStatTransactionIdleTime;
+
+ /* send the difference since the last report */
+ tsmsg->m_active_time =
+ pgstat_get_my_active_time() - pgStatLastActiveTime;
+ tsmsg->m_idle_in_xact_time =
+ pgstat_get_my_transaction_idle_time() - pgStatLastTransactionIdleTime;
}
else
{
@@ -1039,8 +1043,8 @@ pgstat_send_tabstat(PgStat_MsgTabstat *tsmsg, TimestampTz now)
pgStatXactRollback = 0;
pgStatBlockReadTime = 0;
pgStatBlockWriteTime = 0;
- pgStatActiveTime = 0;
- pgStatTransactionIdleTime = 0;
+ pgStatLastActiveTime = pgstat_get_my_active_time();
+ pgStatLastTransactionIdleTime = pgstat_get_my_transaction_idle_time();
}
else
{
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index 2fecf26a2c..ccea8e3325 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;
@@ -575,24 +587,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 +637,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);
}
@@ -1046,6 +1068,48 @@ pgstat_get_my_query_id(void)
}
+/* ----------
+ * pgstat_get_my_active_time() -
+ *
+ * Return current backend's accumulated active time.
+ */
+uint64
+pgstat_get_my_active_time(void)
+{
+ if (!MyBEEntry)
+ return 0;
+
+ /*
+ * There's no need for a lock around pgstat_begin_read_activity /
+ * pgstat_end_read_activity here as it's only called from
+ * pg_stat_get_activity which is already protected, or from the same
+ * backend which means that there won't be concurrent writes.
+ */
+ return MyBEEntry->st_total_active_time;
+}
+
+
+/* ----------
+ * pgstat_get_my_transaction_idle_time() -
+ *
+ * Return current backend's accumulated in-transaction idle time.
+ */
+uint64
+pgstat_get_my_transaction_idle_time(void)
+{
+ if (!MyBEEntry)
+ return 0;
+
+ /*
+ * There's no need for a lock around pgstat_begin_read_activity /
+ * pgstat_end_read_activity here as it's only called from
+ * pg_stat_get_activity which is already protected, or from the same
+ * backend which means that there won't be concurrent writes.
+ */
+ return MyBEEntry->st_total_transaction_idle_time;
+}
+
+
/* ----------
* pgstat_fetch_stat_beentry() -
*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 15cb17ace4..076fa1f049 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);
@@ -675,6 +675,7 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
{
SockAddr zero_clientaddr;
char *clipped_activity;
+ int64 time_to_report;
switch (beentry->st_state)
{
@@ -916,6 +917,23 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
nulls[29] = true;
else
values[29] = UInt64GetDatum(beentry->st_query_id);
+
+ time_to_report = beentry->st_total_active_time;
+ /* add the realtime value to the counter if needed */
+ if (PGSTAT_IS_ACTIVE(beentry))
+ time_to_report +=
+ GetCurrentTimestamp() - beentry->st_state_start_timestamp;
+ /* convert it to msec */
+ values[30] = Float8GetDatum(time_to_report / 1000.0) ;
+
+ time_to_report = beentry->st_total_transaction_idle_time;
+ /* add the realtime value to the counter if needed */
+ if (PGSTAT_IS_IDLEINTRANSACTION(beentry))
+ time_to_report +=
+ GetCurrentTimestamp() - beentry->st_state_start_timestamp;
+
+ /* convert it to msec */
+ values[31] = Float8GetDatum(time_to_report / 1000.0);
}
else
{
@@ -944,6 +962,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/pgstat.h b/src/include/pgstat.h
index e10d20222a..2f168cbdab 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -1071,13 +1071,6 @@ extern PgStat_MsgWal WalStats;
extern PgStat_Counter pgStatBlockReadTime;
extern PgStat_Counter pgStatBlockWriteTime;
-/*
- * Updated by pgstat_count_conn_*_time macros, called by
- * pgstat_report_activity().
- */
-extern PgStat_Counter pgStatActiveTime;
-extern PgStat_Counter pgStatTransactionIdleTime;
-
/*
* Updated by the traffic cop and in errfinish()
@@ -1185,10 +1178,6 @@ extern void pgstat_initstats(Relation rel);
(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 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 8217d0cb6b..e06f2c4022 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
@@ -305,6 +315,8 @@ extern const char *pgstat_get_backend_current_activity(int pid, bool checkUser);
extern const char *pgstat_get_crashed_backend_activity(int pid, char *buffer,
int buflen);
extern uint64 pgstat_get_my_query_id(void);
+extern uint64 pgstat_get_my_active_time(void);
+extern uint64 pgstat_get_my_transaction_idle_time(void);
/* ----------
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,
