public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Ryoga Yoshida <[email protected]>
Cc: [email protected]
Subject: Re: Doesn't pgstat_report_wal() handle the argument "force" incorrectly
Date: Mon, 25 Sep 2023 09:56:22 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

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


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (8+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Doesn't pgstat_report_wal() handle the argument "force" incorrectly
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox