From e3cb1a586cef746326eeabf36d103ea1136607f9 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 12 Jun 2023 15:06:12 -0700 Subject: [PATCH] Report stats when replaying XLOG_RUNNING_XACTS Previously stats in the startup process would only get reported during shutdown of the startup process. It has been that way for a long time, but became a lot more noticeable with the new pg_stat_io view, which separates out IO done by different backend types... While replaying after every XLOG_RUNNING_XACTS isn't the prettiest approach, it has the advantage of being quite easy. Given that we're well past feature freeze... It's not a problem that we don't report stats more frequently with wal_level=minimal, in that case stats can't be read before the stats process has shut down. Besides the above, this commit also changes pgstat_report_stat() to acquire the timestamp with GetCurrentTimestamp() instead of GetCurrentTransactionStopTimestamp(). Thanks to Melih Mutlu, Kyotaro Horiguchi for prototypes of other approaches to solving this issue. Reported-by: Fujii Masao Discussion: https://postgr.es/m/5315aedc-fbca-1556-c5de-dc2e00b23a14@oss.nttdata.com --- src/backend/storage/ipc/standby.c | 9 +++++++++ src/backend/utils/activity/pgstat.c | 17 ++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c index ffe5e1563f5..70b0da50fc1 100644 --- a/src/backend/storage/ipc/standby.c +++ b/src/backend/storage/ipc/standby.c @@ -1193,6 +1193,15 @@ standby_redo(XLogReaderState *record) running.xids = xlrec->xids; ProcArrayApplyRecoveryInfo(&running); + + /* + * The startup process currently has no convenient way to schedule + * stats to be reported. XLOG_RUNNING_XACTS records issued at a + * regular cadence, making this a convenient location to report + * stats. While these records aren't generated with wal_level=minimal, + * stats also cannot be accessed during WAL replay. + */ + pgstat_report_stat(true); } else if (info == XLOG_INVALIDATIONS) { diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index 0cdb552631e..d743fc0b289 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -615,10 +615,21 @@ pgstat_report_stat(bool force) */ Assert(!pgStatLocal.shmem->is_shutdown); - now = GetCurrentTransactionStopTimestamp(); - - if (!force) + if (force) { + /* + * Stats reports are forced either when it's been too long since stats + * have been reported or in processes that force stats reporting to + * happen at specific points (including shutdown). In the former case + * the transaction stop time might be quite old, in the latter it + * would never get cleared. + */ + now = GetCurrentTimestamp(); + } + else + { + now = GetCurrentTransactionStopTimestamp(); + if (pending_since > 0 && TimestampDifferenceExceeds(pending_since, now, PGSTAT_MAX_INTERVAL)) {