postgres.git / summary / log / commit / refs

commit    802fcb9ed16672dcc179977c9dc561fa12afae7b
Author:   Michael Paquier <michael@paquier.xyz>
Date:     Tue Sep 26 00:30:39 2023 +0000
First released in: 17.0, 15.5, 16.1

    Fix behavior of "force" in pgstat_report_wal()
    
    As implemented in 5891c7a8ed8f, setting "force" to true in
    pgstat_report_wal() causes the routine to not wait for the pgstat
    shmem lock if it cannot be acquired, in which case the WAL and I/O
    statistics finish by not being flushed.  The origin of the confusion
    comes from pgstat_flush_wal() and pgstat_flush_io(), that use "nowait"
    as sole argument.  The I/O stats are new in v16.
    
    This is the opposite behavior of what has been used in
    pgstat_report_stat(), where "force" is the opposite of "nowait".  In
    this case, when "force" is true, the routine sets "nowait" to false,
    which would cause the routine to wait for the pgstat shmem lock,
    ensuring that the stats are always flushed.  When "force" is false,
    "nowait" is set to true, and the stats would only not be flushed if the
    pgstat shmem lock can be acquired, returning immediately without
    flushing the stats if the lock cannot be acquired.
    
    This commit changes pgstat_report_wal() so as "force" has the same
    behavior as in pgstat_report_stat().  There are currently three callers
    of pgstat_report_wal():
    - Two in the checkpointer where force=true during a shutdown and the
    main checkpointer loop.  Now the code behaves so as the stats are always
    flushed.
    - One in the main loop of the bgwriter, where force=false.  Now the code
    behaves so as the stats would not be flushed if the pgstat shmem lock
    could not be acquired.
    
    Before this commit, some stats on WAL and I/O could have been lost after
    a shutdown, for example.
    
    Reported-by: Ryoga Yoshida
    Author: Ryoga Yoshida, Michael Paquier
    Discussion: https://postgr.es/m/f87a4d7be70530606b864fd1df91718c@oss.nttdata.com
    Backpatch-through: 15


 src/backend/utils/activity/pgstat_wal.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/activity/pgstat_wal.c b/src/backend/utils/activity/pgstat_wal.c
index 5a878bd1155..305a925c47a 100644
--- a/src/backend/utils/activity/pgstat_wal.c
+++ b/src/backend/utils/activity/pgstat_wal.c
@@ -38,11 +38,22 @@ static WalUsage prevWalUsage;
  *
  * Must be called by processes that generate WAL, that do not call
  * pgstat_report_stat(), like walwriter.
+ *
+ * "force" set to true ensures that the statistics are flushed; note that
+ * this needs to acquire the pgstat shmem LWLock, waiting on it.  When
+ * set to false, the statistics may not be flushed if the lock could not
+ * be acquired.
  */
 void
 pgstat_report_wal(bool force)
 {
-	pgstat_flush_wal(force);
+	bool		nowait;
+
+	/* like in pgstat.c, don't wait for lock acquisition when !force */
+	nowait = !force;
+
+	/* flush wal stats */
+	pgstat_flush_wal(nowait);
 }
 
 /*


[parent: 3c065d5a7ecd]