On Fri, Sep 22, 2023 at 01:58:37PM +0900, Ryoga Yoshida wrote: > pgstat_report_wal() calls pgstat_flush_wal() and pgstat_flush_io(). When > calling them, pgstat_report_wal() specifies its argument "force" as the > argument of them, as follows. But according to the code of > pgstat_flush_wal() and pgstat_flush_io(), their argument is "nowait" and its > meaning seems the opposite of "force". This means that, even when > checkpointer etc calls pgstat_report_wal() with force=true to forcibly flush > the statistics, pgstat_flush_wal() and pgstat_flush_io() skip flushing the > statistics if they fail to acquire the lock immediately because they are > called with nowait=true. This seems unexpected behavior and a bug.
It seems to me that you are right here. It would make sense to me to say that force=true is equivalent to nowait=false, as in "I'm OK to wait on the lockas I want to make sure that the stats are flushed at this point". Currently force=true means nowait=true, as in "I'm OK to not have the stats flushed if I cannot take the lock". Seeing the three callers of pgstat_report_wal(), the checkpointer wants to force its way twice, and the WAL writer does not care if they are not flushed immediately at it loops forever in this path. A comment at the top of pgstat_report_wal() would be nice to document that a bit better, at least. -- Michael
signature.asc
Description: PGP signature