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 |
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 |