Date: Mon, 31 Mar 2025 13:05:24 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Xuneng Zhou <xunengzhou@gmail.com>, Andres Freund <andres@anarazel.de>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Fwd: [BUG]: the walsender does not update its IO statistics
 until it exits
Message-ID: <Z+qTFGLaXG0xhqVz@ip-10-97-1-34.eu-west-3.compute.internal>
References: 
 <CABPTF7VRN_-DfrPFjVtRYuN0E8mWETtVp6tSXNsXDJPDAuHHxA@mail.gmail.com>
 <CABPTF7Vf_j7Eb1F3wuaFHr6r6m2_4imVzSKCYPkciN+Ti2eR2w@mail.gmail.com>
 <Z9Lbd9BPogBFlLOX@ip-10-97-1-34.eu-west-3.compute.internal>
 <CABPTF7XT=txv4-6SnpZGkx-CreWQ3UsOq2WCat1i64TNnuO=Cw@mail.gmail.com>
 <Z9mG+xW3JFGgyeBZ@ip-10-97-1-34.eu-west-3.compute.internal>
 <CABPTF7VaVj+BnMyW5-VpGaZPJKpKjKhU8yfmCG2KyR0+mKaYOA@mail.gmail.com>
 <Z9pVl5bF0xH8HY_9@paquier.xyz>
 <Z9pbJJgpFF9ijeWt@ip-10-97-1-34.eu-west-3.compute.internal>
 <CABPTF7WbzWU9vnQcJnJpedKTJzj36xh7uLwGLzwTqCvXkuZ5dg@mail.gmail.com>
 <Z-CcIPw78fg1FGsv@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="gmFEkRlSpLUMOhSL"
Content-Disposition: inline
In-Reply-To: <Z-CcIPw78fg1FGsv@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/Z%2BqTFGLaXG0xhqVz%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk


--gmFEkRlSpLUMOhSL
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,

On Mon, Mar 24, 2025 at 08:41:20AM +0900, Michael Paquier wrote:
> On Wed, Mar 19, 2025 at 04:00:49PM +0800, Xuneng Zhou wrote:
> > Hi,
> > Moving the other two provides a more complete view of the settings. For
> > newcomers(like me) to the codebase, seeing all three related values in one
> > place helps avoid a narrow view of the settings.
> > 
> > But I am not sure that I understand the cons of this well.
> 
> While I don't disagree with the use of a hardcoded interval of time to
> control timing the flush of the WAL sender stats, do we really need to
> rely on the timing defined by pgstat.c?

No but I thought it could make sense.

> Wouldn't it be simpler to
> assign one in walsender.c and pick up a different, perhaps higher,
> value?

I don't have a strong opinion on it so done as suggested above in the attached.

I think that the 1s value is fine because: 1. it is consistent with 
PGSTAT_MIN_INTERVAL and 2. it already needs that the sender is caught up or
has pending data to send (means it could be higher than 1s already). That said,
I don't think that would hurt if you think of a higher value.

> At the end the timestamp calculations are free because we can rely on
> the existing call of GetCurrentTimestamp() for the physical WAL
> senders to get an idea of the current time.

Yup

> For the logical WAL
> senders, perhaps we'd better do the reports in WalSndWaitForWal(),
> actually.  There is also a call to GetCurrentTimestamp() that we can
> rely on in this path.

I think it's better to flush the stats in a shared code path. I think it's
easier to maintain and that there is no differences between logical and
physical walsenders that would justify to flush the stats in specific code
paths.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

--gmFEkRlSpLUMOhSL
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v5-0001-Flush-the-IO-statistics-of-active-walsenders.patch"