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-

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 495cbf2006008c3d8f4df021a2286351281a0b1c..5ab91c2c581bd5e0fb2a257401ac3a47439d2c96 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/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);
    }
@@ -4050,7 +4053,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);
@@ -4427,13 +4429,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");
@@ -4469,10 +4489,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.
@@ -4487,11 +4506,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.

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 0d9d09c4922608d0e8ca2f98b92db8206019e408..88e5dd1b2b72b2f81718d9b65d32c324462196d2 100644 (file)
--- a/src/backend/utils/activity/pgstat.c
+++ b/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)