pgstat: reduce timer overhead by leaving timer running.

Previously the timer was enabled whenever there were any pending stats after
executing a statement, just to then be disabled again when not idle
anymore. That lead to an increase in GetCurrentTimestamp() calls from within
timeout.c compared to 14.

To avoid that increase, leave the timer enabled until stats are reported,
rather than until idle. The timer is only disabled once the pending stats have
been reported.

For me this fixes the increase in GetCurrentTimestamp() calls, there now are
fewer calls in 15 than in 14, in the previously slowed down workload.

While at it, also update assertion in pgstat_report_stat() to be more precise.

Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de
Backpatch: 15-

Author: anarazel

src/backend/tcop/postgres.c

@@ -3371,10 +3371,13 @@ ProcessInterrupts(void)
 			IdleSessionTimeoutPending = false;
 	}
 
-	if (IdleStatsUpdateTimeoutPending)
+	/*
+	 * If there are pending stats updates and we currently are truly idle
+	 * (matching the conditions in PostgresMain(), report stats now.
+	 */
+	if (IdleStatsUpdateTimeoutPending &&
+		DoingCommandRead && !IsTransactionOrTransactionBlock())
 	{
-		/* timer should have been disarmed */
-		Assert(!IsTransactionBlock());
 		IdleStatsUpdateTimeoutPending = false;
 		pgstat_report_stat(true);
 	}
@@ -4051,7 +4054,6 @@ PostgresMain(const char *dbname, const char *username)
 	volatile bool send_ready_for_query = true;
 	bool		idle_in_transaction_timeout_enabled = false;
 	bool		idle_session_timeout_enabled = false;
-	bool		idle_stats_update_timeout_enabled = false;
 
 	AssertArg(dbname != NULL);
 	AssertArg(username != NULL);
@@ -4428,13 +4430,31 @@ PostgresMain(const char *dbname, const char *username)
 				if (notifyInterruptPending)
 					ProcessNotifyInterrupt(false);
 
-				/* Start the idle-stats-update timer */
+				/*
+				 * Check if we need to report stats. If pgstat_report_stat()
+				 * decides it's too soon to flush out pending stats / lock
+				 * contention prevented reporting, it'll tell us when we
+				 * should try to report stats again (so that stats updates
+				 * aren't unduly delayed if the connection goes idle for a
+				 * long time). We only enable the timeout if we don't already
+				 * have a timeout in progress, because we don't disable the
+				 * timeout below. enable_timeout_after() needs to determine
+				 * the current timestamp, which can have a negative
+				 * performance impact. That's OK because pgstat_report_stat()
+				 * won't have us wake up sooner than a prior call.
+				 */
 				stats_timeout = pgstat_report_stat(false);
 				if (stats_timeout > 0)
 				{
-					idle_stats_update_timeout_enabled = true;
-					enable_timeout_after(IDLE_STATS_UPDATE_TIMEOUT,
-										 stats_timeout);
+					if (!get_timeout_active(IDLE_STATS_UPDATE_TIMEOUT))
+						enable_timeout_after(IDLE_STATS_UPDATE_TIMEOUT,
+											 stats_timeout);
+				}
+				else
+				{
+					/* all stats flushed, no need for the timeout */
+					if (get_timeout_active(IDLE_STATS_UPDATE_TIMEOUT))
+						disable_timeout(IDLE_STATS_UPDATE_TIMEOUT, false);
 				}
 
 				set_ps_display("idle");
@@ -4470,10 +4490,9 @@ PostgresMain(const char *dbname, const char *username)
 		firstchar = ReadCommand(&input_message);
 
 		/*
-		 * (4) turn off the idle-in-transaction, idle-session and
-		 * idle-stats-update timeouts if active.  We do this before step (5)
-		 * so that any last-moment timeout is certain to be detected in step
-		 * (5).
+		 * (4) turn off the idle-in-transaction and idle-session timeouts if
+		 * active.  We do this before step (5) so that any last-moment timeout
+		 * is certain to be detected in step (5).
 		 *
 		 * At most one of these timeouts will be active, so there's no need to
 		 * worry about combining the timeout.c calls into one.
@@ -4488,11 +4507,6 @@ PostgresMain(const char *dbname, const char *username)
 			disable_timeout(IDLE_SESSION_TIMEOUT, false);
 			idle_session_timeout_enabled = false;
 		}
-		if (idle_stats_update_timeout_enabled)
-		{
-			disable_timeout(IDLE_STATS_UPDATE_TIMEOUT, false);
-			idle_stats_update_timeout_enabled = false;
-		}
 
 		/*
 		 * (5) disable async signal conditions again.

src/backend/utils/activity/pgstat.c

@@ -571,7 +571,7 @@ pgstat_report_stat(bool force)
 	bool		nowait;
 
 	pgstat_assert_is_up();
-	Assert(!IsTransactionBlock());
+	Assert(!IsTransactionOrTransactionBlock());
 
 	/* "absorb" the forced flush even if there's nothing to flush */
 	if (pgStatForceNextFlush)