Re: Flush some statistics within running transactions - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Flush some statistics within running transactions
Date
Msg-id abfZ55jwCGGci6aK@paquier.xyz
Whole thread Raw
In response to Re: Flush some statistics within running transactions  (Bertrand Drouvot <bertranddrouvot.pg@gmail.com>)
List pgsql-hackers
On Mon, Mar 16, 2026 at 09:20:41AM +0000, Bertrand Drouvot wrote:
> I did not look closely at the code but did some testing too. I confirm that
> pg_stat_io and pg_stat_wal are updated when pg_stat_report(<backend_pid>) is
> triggered. But the stats update is not visible if requested through
> pg_stat_get_backend_io(<same_backend_pid>) or pg_stat_get_backend_wal(<same_backend_pid>)).
> I guess that PGSTAT_KIND_BACKEND should also get the PGSTAT_REPORT_TRANSACTION
> report_context?

Yes, I guess that the transaction-level flag should be set as well for
the backend stats, due to the fact that WAL and IO stats have it set.

>> Note that this is a WIP, which is check-world stable.  One thing that
>> sticks a bit in mind now is that perhaps we should not allow the
>> function for auxiliary processes at all.
>
> Why?

I was feeling that there would be an issue with that, and I did not
test it in details yet.  For the checkpointer with a busy activity,
the IO stats could be interesting to see in live.

>> Perhaps we should just have pgstat_flush_pending_entries() provide a
>> correct status in line with the property set in a stats kind when we
>> try a flush while in a transaction.
>
> The idea would be to avoid trying to flush stats that don't have pending
> entries?

I had to work around the assert at the end of pgstat_report_stat(), to
tell that under an xact the partial flushes were OK.  I was wondering
about keeping the end assertion based on solely "force" intact, making
the flush routines return a status based on if we are in an xact.

> That looks "simpler" that the previous proposal but who would be responsible to
> call pg_stat_report()? If that's the client responsabilty that kind of look weird
> to me. If that's the core, how would that be scheduled? I think that the
> end solution should prevent to find similar issues as 039549d70f6 fixed, without
> delegating to the client.

TBH, I don't like the requirement of setting SIGALRM in all the places
where we'd require it for the sake of this proposal, where
historically we have never done that, copying a mechanism that already
exists in the tree for procsigs, as the previous patch I posted proves
we could reuse.  It's also not clear to me what a correct frequency of
the stat updates should be, and why it would make sense to force that
in a GUC; we want to have some information from long-running
transactions, where I fear that we will want modularity.  A
user-settable GUC would fit with this picture, but the requirement of
planting the timeouts don't stick for me..

On top of that, I am not really convinced that there is a good reason
to remove the existing stat report calls we have already planted in
the tree for auxiliary processes, diverging from the stable branches
where these exist.  With more than one release already out with them,
there is more benefit with potentially planting more strategic report
calls where they could matter (as added in v18 for WAL senders as one
example), when we find a requirement for them.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: Add missing stats_reset column to pg_statio_all_sequences view
Next
From: Ashutosh Bapat
Date:
Subject: Re: Better shared data structure management and resizable shared data structures