(unnamed)

text/plain

Filename: (unnamed)
Type: text/plain
Part: 0
Message: Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 7fcc3f6ded..47973f1e30 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -244,8 +244,6 @@ static int	pgStatXactCommit = 0;
 static int	pgStatXactRollback = 0;
 PgStat_Counter pgStatBlockReadTime = 0;
 PgStat_Counter pgStatBlockWriteTime = 0;
-PgStat_Counter pgStatActiveTime = 0;
-PgStat_Counter pgStatTransactionIdleTime = 0;
 SessionEndType pgStatSessionEndCause = DISCONNECT_NORMAL;
 
 /* Record that's written to 2PC state file when pgstat state is persisted */
@@ -323,7 +321,7 @@ static void pgstat_send_tabstat(PgStat_MsgTabstat *tsmsg);
 static void pgstat_send_funcstats(void);
 static void pgstat_send_slru(void);
 static HTAB *pgstat_collect_oids(Oid catalogid, AttrNumber anum_oid);
-static void pgstat_send_connstats(bool disconnect, TimestampTz last_report);
+static void pgstat_send_connstats(TimestampTz now);
 
 static PgStat_TableStatus *get_tabstat_entry(Oid rel_id, bool isshared);
 
@@ -876,10 +874,8 @@ pgstat_report_stat(bool disconnect)
 		return;
 
 	/* for backends, send connection statistics */
-	if (MyBackendType == B_BACKEND)
-		pgstat_send_connstats(disconnect, last_report);
-
-	last_report = now;
+	if (MyBackendType == B_BACKEND && disconnect)
+		pgstat_send_connstats(now);
 
 	/*
 	 * Destroy pgStatTabHash before we start invalidating PgStat_TableEntry
@@ -1368,39 +1364,41 @@ pgstat_drop_relation(Oid relid)
  * pgstat_send_connstats() -
  *
  *	Tell the collector about session statistics.
- *	The parameter "disconnect" will be true when the backend exits.
- *	"last_report" is the last time we were called (0 if never).
  * ----------
  */
 static void
-pgstat_send_connstats(bool disconnect, TimestampTz last_report)
+pgstat_send_connstats(TimestampTz now)
 {
 	PgStat_MsgConn msg;
-	long		secs;
-	int			usecs;
+	volatile PgBackendStatus *beentry = MyBEEntry;
 
 	if (pgStatSock == PGINVALID_SOCKET || !pgstat_track_counts)
 		return;
 
 	pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_CONNECTION);
 	msg.m_databaseid = MyDatabaseId;
+	msg.m_disconnect = pgStatSessionEndCause;
 
-	/* session time since the last report */
-	TimestampDifference(((last_report == 0) ? MyStartTimestamp : last_report),
-						GetCurrentTimestamp(),
-						&secs, &usecs);
-	msg.m_session_time = secs * 1000000 + usecs;
+	msg.m_count = 1;
 
-	msg.m_disconnect = disconnect ? pgStatSessionEndCause : DISCONNECT_NOT_YET;
+	if (pgstat_track_activities && beentry)
+	{
+		msg.m_session_time =
+			(PgStat_Counter)(now - beentry->st_proc_start_timestamp) * 1000;
+		msg.m_active_time = beentry->st_session_active_time;
+		msg.m_idle_in_xact_time = beentry->st_session_idle_time;
 
-	msg.m_active_time = pgStatActiveTime;
-	pgStatActiveTime = 0;
-
-	msg.m_idle_in_xact_time = pgStatTransactionIdleTime;
-	pgStatTransactionIdleTime = 0;
-
-	/* report a new session only the first time */
-	msg.m_count = (last_report == 0) ? 1 : 0;
+		PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
+		beentry->st_session_active_time = 0;
+		beentry->st_session_idle_time = 0;
+		PGSTAT_END_WRITE_ACTIVITY(beentry);
+	}
+	else
+	{
+		msg.m_session_time = 0;
+		msg.m_active_time = 0;
+		msg.m_idle_in_xact_time = 0;
+	}
 
 	pgstat_send(&msg, sizeof(PgStat_MsgConn));
 }
@@ -2877,7 +2875,7 @@ pgstat_initialize(void)
 	prevWalUsage = pgWalUsage;
 
 	/* Set up a process-exit hook to clean up */
-	on_shmem_exit(pgstat_shutdown_hook, 0);
+	before_shmem_exit(pgstat_shutdown_hook, 0);
 }
 
 /* ------------------------------------------------------------
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index e19c4506ef..0f0eecc3b4 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_session_active_time = 0;
+	lbeentry.st_session_idle_time = 0;
 	lbeentry.st_databaseid = MyDatabaseId;
 
 	/* We have userid for client-backends, wal-sender and bgworker processes */
@@ -590,9 +592,9 @@ 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(secs * 1000000 + usecs);
+			beentry->st_session_active_time += secs * 1000000 + usecs;
 		else
-			pgstat_count_conn_txn_idle_time(secs * 1000000 + usecs);
+			beentry->st_session_idle_time += secs * 1000000 + usecs;
 	}
 
 	/*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index f0e09eae4d..62b60077c1 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1635,6 +1635,56 @@ pg_stat_get_db_blk_write_time(PG_FUNCTION_ARGS)
 	PG_RETURN_FLOAT8(result);
 }
 
+/* XXXXX */
+static double
+_pg_stat_active_session_stats(Oid dbid, int type)
+{
+	int			num_backends = pgstat_fetch_stat_numbackends();
+	int			curr_backend;
+	TimestampTz	now = GetCurrentTimestamp();
+	double		result = 0.0;
+	int			nsessions = 0;
+
+
+	/* Add session time of active backends */
+	/* 1-based index */
+	for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
+	{
+		LocalPgBackendStatus *local_beentry;
+		PgBackendStatus *beentry;
+
+		local_beentry = pgstat_fetch_stat_local_beentry(curr_backend);
+		if (!local_beentry)
+			continue;
+
+		beentry = &local_beentry->backendStatus;
+
+		if (beentry->st_databaseid != dbid)
+			continue;
+
+		switch (type) /* define enum !*/
+		{
+			case 0: /* SESSION TIME */
+				result +=
+					(double)(now - beentry->st_proc_start_timestamp) / 1000;
+				break;
+			case 1: /* SESSION ACTIVE TIME */
+				result += (double)beentry->st_session_active_time / 1000;
+				break;
+			case 2: /* SESSION IDLE TIME */
+				result += (double)beentry->st_session_idle_time / 1000;
+				break;
+			case 3: /* SESSION NUMBER */
+				nsessions++;
+		}
+	}
+
+	if (type < 3)
+		return result;
+
+	return (double)nsessions;
+}
+
 Datum
 pg_stat_get_db_session_time(PG_FUNCTION_ARGS)
 {
@@ -1646,6 +1696,8 @@ pg_stat_get_db_session_time(PG_FUNCTION_ARGS)
 	if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) != NULL)
 		result = ((double) dbentry->total_session_time) / 1000.0;
 
+	result += _pg_stat_active_session_stats(dbid, 0);
+
 	PG_RETURN_FLOAT8(result);
 }
 
@@ -1660,6 +1712,8 @@ pg_stat_get_db_active_time(PG_FUNCTION_ARGS)
 	if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) != NULL)
 		result = ((double) dbentry->total_active_time) / 1000.0;
 
+	result += _pg_stat_active_session_stats(dbid, 1);
+
 	PG_RETURN_FLOAT8(result);
 }
 
@@ -1674,6 +1728,8 @@ pg_stat_get_db_idle_in_transaction_time(PG_FUNCTION_ARGS)
 	if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) != NULL)
 		result = ((double) dbentry->total_idle_in_xact_time) / 1000.0;
 
+	result += _pg_stat_active_session_stats(dbid, 2);
+
 	PG_RETURN_FLOAT8(result);
 }
 
@@ -1687,6 +1743,7 @@ pg_stat_get_db_sessions(PG_FUNCTION_ARGS)
 	if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) != NULL)
 		result = (int64) (dbentry->n_sessions);
 
+	result += (int64)_pg_stat_active_session_stats(dbid, 3);
 	PG_RETURN_INT64(result);
 }
 
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index f779b48b8c..9311111690 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -931,13 +931,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()
@@ -1039,10 +1032,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 8042b817df..ebd5755247 100644
--- a/src/include/utils/backend_status.h
+++ b/src/include/utils/backend_status.h
@@ -124,6 +124,10 @@ typedef struct PgBackendStatus
 	TimestampTz st_activity_start_timestamp;
 	TimestampTz st_state_start_timestamp;
 
+	/* Session active/idle time in microsecnods */
+	int64		st_session_active_time;
+	int64		st_session_idle_time;
+
 	/* Database OID, owning user's OID, connection client address */
 	Oid			st_databaseid;
 	Oid			st_userid;