On Thu, Mar 06, 2025 at 10:33:52AM +0000, Bertrand Drouvot wrote: > Indeed, there is no reason for pgstat_backend_have_pending_cb() to return > true if > pgstat_tracks_backend_bktype() is not satisfied. > > So it deserves a dedicated patch to fix this already existing issue: > 0001 attached.
pgstat_backend_have_pending_cb(void)
{
- return (!pg_memory_is_all_zeros(&PendingBackendStats,
- sizeof(struct PgStat_BackendPending)));
+ if (!pgstat_tracks_backend_bktype(MyBackendType))
+ return false;
+ else
+ return (!pg_memory_is_all_zeros(&PendingBackendStats,
+ sizeof(struct PgStat_BackendPending)));
So, if I understand your point correctly, it is not a problem on HEAD
because we are never going to update PendingBackendStats in the
checkpointer as pgstat_count_backend_io_op[_time]() blocks any attempt
to do so. However, it becomes a problem with the WAL portion patch
because of the dependency to pgWalUsage which may be updated by the
checkpointer and the pending callback would happily report true in
this case. It would also become a problem if we add in backend stats
a different portion that depends on something external.
An extra check based on pgstat_tracks_backend_bktype() makes sense in
pgstat_backend_have_pending_cb(), yes, forcing the hand of the stats
reports to not do stupid things in a process where we should not
report stats. Good catch from the sanity check in
pgstat_report_stat(), even if this is only a problem once we begin to
use something else than PendingBackendStats for the pending stats.
This has also the merit of making pgstat_report_stat() do less work.
--
Michael
signature.asc
Description: PGP signature
