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: Xuneng Zhou <xunengzhou(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Michael Paquier <michael(at)paquier(dot)xyz>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: Fwd: [BUG]: the walsender does not update its IO statistics until it exits
Date: 2025-03-13 13:18:45
Message-ID: Z9LbNVAZrMinORG7@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, Mar 13, 2025 at 07:33:24PM +0800, Xuneng Zhou wrote:
> Regarding patch 0001, the optimization in pgstat_backend_have_pending_cb
> looks good:

Thanks for looking at it!

> bool
> pgstat_backend_have_pending_cb(void)
> {
> - return (!pg_memory_is_all_zeros(&PendingBackendStats,
> - sizeof(struct PgStat_BackendPending)));
> + return backend_has_iostats;
> }
>
> Additionally, the function pgstat_flush_backend includes the check:
>
> + if (!pgstat_backend_have_pending_cb())
> return false;
>
> However, I think we might need to revise the comment (and possibly the
> function name) for clarity:
>
> /*
> * Check if there are any backend stats waiting to be flushed.
> */

The comment is not exactly this one on the current HEAD, it looks like that you're
looking at a previous version of the core code.

> Originally, this function was intended to check multiple types of backend
> statistics, which made sense when PendingBackendStats was the centralized
> structure for various pending backend stats. However, since
> PgStat_PendingWalStats was removed from PendingBackendStats earlier, and
> now this patch introduces the backend_has_iostats variable, the scope of
> this function appears even narrower. This narrowed functionality no longer
> aligns closely with the original function name and its associated comment.

I don't think so, as since 76def4cdd7c, a pgstat_backend_wal_have_pending() check
has been added to pgstat_backend_have_pending_cb(). You're probably looking at
a version prior to 76def4cdd7c.

This particular sub-patch needs a rebase though, done in the attached. 0001
remains unchanged as compared to the v4 one just shared up-thread. If 0001 goes
in, merging 0002 would be less beneficial (as compared to v3).

Regards,

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

Attachment Content-Type Size
v4-0001-Flush-the-IO-statistics-of-active-walsenders.patch text/x-diff 6.2 KB
v4-0002-Add-a-new-backend_has_iostats-global-variable.patch text/x-diff 2.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2025-03-13 13:19:51 Re: Fwd: [BUG]: the walsender does not update its IO statistics until it exits
Previous Message Tatsuo Ishii 2025-03-13 12:51:29 Re: Row pattern recognition