Re: Fwd: [BUG]: the walsender does not update its IO statistics until it exits

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Xuneng Zhou <xunengzhou(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Fwd: [BUG]: the walsender does not update its IO statistics until it exits
Date: 2025-04-03 07:24:46
Message-ID: Z+43vj3ouLrP13Ys@ip-10-97-1-34.eu-west-3.compute.internal
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Thu, Apr 03, 2025 at 09:25:02AM +0530, vignesh C wrote:
> On Mon, 31 Mar 2025 at 18:35, Bertrand Drouvot
> <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> >
> Couple of suggestions:

Thanks for looking at it!

> 1) I felt we can include a similar verification for one of the logical
> replication tests too:
> +# Wait for the walsender to update its IO statistics.
>
> +# Has to be done before the next restart and far enough from the
> +# pg_stat_reset_shared('io') to minimize the risk of polling for too long.
> +$node_primary->poll_query_until(
> + 'postgres',
> + qq[SELECT sum(reads) > 0
> + FROM pg_catalog.pg_stat_io
> + WHERE backend_type = 'walsender'
> + AND object = 'wal']
> + )
> + or die
> + "Timed out while waiting for the walsender to update its IO statistics";
> +

Initially ([1]) it was added in 035_standby_logical_decoding.pl. But this
test (035) is already racy enough that I felt better to move it to 001_stream_rep.pl
(see [2]) instead.

I don't think that's a big issue as the code path being changed in the patch is
shared between logical and physical walsenders. That said that would not hurt to
add a logical walsender test, but I would prefer it to be outside
035_standby_logical_decoding.pl. Do you have a suggestion for the location of
such a test?

> 2) We can comment this in a single line itself:
> + /*
> + * Report IO statistics
> + */

Right, but:

- it's already done that way in walsender.c (see "Try to flush any pending output to the client"
for example).
- the exact same comment is written that way in pgstat_bgwriter.c and
pgstat_checkpointer.c

So that I just copy/paste it.

[1]: https://www.postgresql.org/message-id/Z73IsKBceoVd4t55%40ip-10-97-1-34.eu-west-3.compute.internal
[2]: https://www.postgresql.org/message-id/Z77jgvhwOu9S0a5r%40ip-10-97-1-34.eu-west-3.compute.internal

Regards,

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2025-04-03 07:25:28 Re: pg_upgrade: Support for upgrading to checksums enabled
Previous Message Laurenz Albe 2025-04-03 07:22:04 Re: Reducing the log spam