Date: Thu, 13 Mar 2025 09:31:29 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: [BUG]: the walsender does not update its IO statistics until it
 exits
Message-ID: <Z9Kl8UfC/GNExHDN@ip-10-97-1-34.eu-west-3.compute.internal>
References: <Z73IsKBceoVd4t55@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z762lusJXp_wNF_z@paquier.xyz>
 <Z77jgvhwOu9S0a5r@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z8FMjlyNpNicucGa@paquier.xyz>
 <Z8GSY8mk+0qVHzsW@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z8ULFzNY24_SMCdu@paquier.xyz>
 <Z8WYf1jyy4MwOveQ@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z84xFrbmnW7Knb1Q@paquier.xyz>
 <Z878uhEaxYxRoXj7@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z8/T2DU208vq3rFc@ip-10-97-1-34.eu-west-3.compute.internal>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="jnHzDq0jK9SJt44i"
Content-Disposition: inline
In-Reply-To: <Z8/T2DU208vq3rFc@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: 
 <https://www.postgresql.org/message-id/Z9Kl8UfC/GNExHDN%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk


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

Hi,

On Tue, Mar 11, 2025 at 06:10:32AM +0000, Bertrand Drouvot wrote:
> WalSndWaitForWal() is being used only for logical walsender. So we'd need to 
> find another location for the physical walsender case. One option is to keep the
> WalSndLoop() location and control the reports frequency.

I ended up with an idea in the same vein as your third one, but let's do it
in WalSndLoop() instead for both logical and physical walsenders. The idea
is to report the stats when the walsender is caught up or has pending data to
send. Also, we do flush only periodically (based on PGSTAT_MIN_INTERVAL).

Remarks/Questions:

1. maybe relying on PGSTAT_IDLE_INTERVAL would make more sense? In both case
PGSTAT_MIN_INTERVAL or PGSTAT_MIN_INTERVAL, I'm not sure there is a need to 
update the related doc.

2. bonus point to make it here is that we don't need an extra call to
GetCurrentTimestamp(). We simply assign the one that is already done to "now".

3. add to use poll_query_until() has I've seen failures on the CI whith this
new approach. Also moving the test far away from the stats reset to to minimize
the risk of polling for too long.

With the attached in place, the number of times the stats are being flushed
reduce drastically but still seems enough to get an accurate idea of the walsender
IO activity. Thoughts?

Regards,

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

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