v2-0001-performance-improvements-of-reporting-wal-stats.patch

diff --git a/src/backend/executor/instrument.c b/src/backend/executor/instrument.c
index 237e13361b..095d8d0970 100644
--- a/src/backend/executor/instrument.c
+++ b/src/backend/executor/instrument.c
@@ -19,6 +19,17 @@
 
 BufferUsage pgBufferUsage;
 static BufferUsage save_pgBufferUsage;
+
+/*
+ * generated WAL usage counters.
+ *
+ * Be careful that the counters are cleared after reporting them to
+ * the stats collector although you can use WalUsageAccumDiff()
+ * to computing differences to previous values. For backends,
+ * the counters may be reset after a transaction is finished and
+ * pgstat_report_wal() is invoked, so you can compute the difference
+ * in the same transaction only.
+ */
 WalUsage	pgWalUsage;
 static WalUsage save_pgWalUsage;
 
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 60f45ccc4e..1ffedb2a24 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -144,14 +144,6 @@ char	   *pgstat_stat_tmpname = NULL;
 PgStat_MsgBgWriter BgWriterStats;
 PgStat_MsgWal WalStats;
 
-/*
- * WAL usage counters saved from pgWALUsage at the previous call to
- * pgstat_report_wal(). This is used to calculate how much WAL usage
- * happens between pgstat_report_wal() calls, by substracting
- * the previous counters from the current ones.
- */
-static WalUsage prevWalUsage;
-
 /*
  * List of SLRU names that we keep stats for.  There is no central registry of
  * SLRUs, so we use this fixed list instead.  The "other" entry is used for
@@ -3117,13 +3109,6 @@ pgstat_initialize(void)
 		MyBEEntry = &BackendStatusArray[MaxBackends + MyAuxProcType];
 	}
 
-	/*
-	 * Initialize prevWalUsage with pgWalUsage so that pgstat_report_wal() can
-	 * calculate how much pgWalUsage counters are increased by substracting
-	 * prevWalUsage from pgWalUsage.
-	 */
-	prevWalUsage = pgWalUsage;
-
 	/* Set up a process-exit hook to clean up */
 	on_shmem_exit(pgstat_beshutdown_hook, 0);
 }
@@ -4674,8 +4659,7 @@ pgstat_send_bgwriter(void)
 /* ----------
  * pgstat_report_wal() -
  *
- * Calculate how much WAL usage counters are increased and send
- * WAL statistics to the collector.
+ * Send WAL statistics to the collector and clear the counters.
  *
  * Must be called by processes that generate WAL.
  * ----------
@@ -4683,19 +4667,36 @@ pgstat_send_bgwriter(void)
 void
 pgstat_report_wal(void)
 {
-	WalUsage	walusage;
-
 	/*
-	 * Calculate how much WAL usage counters are increased by substracting the
-	 * previous counters from the current ones. Fill the results in WAL stats
-	 * message.
+	 * Skip if the WAL statistics counters were not updated.
+	 *
+	 * We can know whether the counters were updated to check only two
+	 * records. So, for performance, we don't allocate another memory spaces
+	 * and check the all stats like pgstat_send_slru().
+	 *
+	 * It's not enough to check only the generated wal records, because there
+	 * is a case that a backend generates no wal records, but just writes and
+	 * syncs the wal data. When to flush buffers, for example, to select
+	 * buffer replacement victim, it requires a WAL flush if the buffer is
+	 * dirty. This happens even if executing read-only transaction.
+	 *
+	 * In this case, if to write is happend, to sync is happend too. although
+	 * there is a case to write is not happend but to sync is happened. For
+	 * example, when to flush buffers just after another process only write
+	 * and don't sync because the wal buffer is no space.
+	 *
+	 * So, it's ok to check the generated wal records and the counter of
+	 * syncing only.
 	 */
-	MemSet(&walusage, 0, sizeof(WalUsage));
-	WalUsageAccumDiff(&walusage, &pgWalUsage, &prevWalUsage);
+	if (pgWalUsage.wal_records == 0 && WalStats.m_wal_sync == 0)
+		return;
 
-	WalStats.m_wal_records = walusage.wal_records;
-	WalStats.m_wal_fpi = walusage.wal_fpi;
-	WalStats.m_wal_bytes = walusage.wal_bytes;
+	/*
+	 * set the counters related to generated WAL data.
+	 */
+	WalStats.m_wal_records = pgWalUsage.wal_records;
+	WalStats.m_wal_fpi = pgWalUsage.wal_fpi;
+	WalStats.m_wal_bytes = pgWalUsage.wal_bytes;
 
 	/*
 	 * Send WAL stats message to the collector.
@@ -4704,18 +4705,26 @@ pgstat_report_wal(void)
 		return;
 
 	/*
-	 * Save the current counters for the subsequent calculation of WAL usage.
+	 * Clear out the statistics buffer for generated WAL data, so it can be
+	 * re-used.
+	 *
+	 * It's ok to clear out because no one takes difference crossing
+	 * pgstat_report_wal() calls although these counters are used in another
+	 * places, for example in pg_stat_statements.c.
 	 */
-	prevWalUsage = pgWalUsage;
+	MemSet(&pgWalUsage, 0, sizeof(WalUsage));
 }
 
 /* ----------
  * pgstat_send_wal() -
  *
- *	Send WAL statistics to the collector.
+ * Send WAL statistics to the collector.
  *
- * If 'force' is not set, WAL stats message is only sent if enough time has
- * passed since last one was sent to reach PGSTAT_STAT_INTERVAL.
+ * If the processes that generate WAL data must call pgstat_report_wal() instead.
+ *
+ * If 'force' is not set, the WAL stats message is only sent if the statistics
+ * counters except for the counters related to generated WAL data were updated
+ * and enough time has passed since last one was sent to reach PGSTAT_STAT_INTERVAL.
  *
  * Return true if the message is sent, and false otherwise.
  * ----------
@@ -4723,26 +4732,32 @@ pgstat_report_wal(void)
 bool
 pgstat_send_wal(bool force)
 {
-	/* We assume this initializes to zeroes */
-	static const PgStat_MsgWal all_zeroes;
 	static TimestampTz sendTime = 0;
 
-	/*
-	 * This function can be called even if nothing at all has happened. In
-	 * this case, avoid sending a completely empty message to the stats
-	 * collector.
-	 */
-	if (memcmp(&WalStats, &all_zeroes, sizeof(PgStat_MsgWal)) == 0)
-		return false;
-
 	if (!force)
 	{
-		TimestampTz now = GetCurrentTimestamp();
+		TimestampTz now;
+
+		/*
+		 * Don't expend a clock check if the WAL statistics counters were not
+		 * updated.
+		 *
+		 * We don't consider the WAL stats related to the generated WAL data
+		 * because pgstat_report_wal() is called instead in the case.
+		 *
+		 * We can know whether the counters were updated to check only two
+		 * records. So, for performance, we don't allocate another memory
+		 * spaces and check the all stats like pgstat_send_slru().
+		 */
+		if (walStats.wal_write == 0 && walStats.wal_sync == 0)
+			return false;
 
 		/*
 		 * Don't send a message unless it's been at least PGSTAT_STAT_INTERVAL
-		 * msec since we last sent one.
+		 * msec since we last sent one to avoid overloading the stats
+		 * collector.
 		 */
+		now = GetCurrentTimestamp();
 		if (!TimestampDifferenceExceeds(sendTime, now, PGSTAT_STAT_INTERVAL))
 			return false;
 		sendTime = now;