| From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | Michael Paquier <michael(at)paquier(dot)xyz> |
| Cc: | Sami Imseih <samimseih(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Subject: | Re: Flush some statistics within running transactions |
| Date: | 2026-02-24 13:55:48 |
| Message-ID: | aZ2t5IZ2c7HBuDS3@ip-10-97-1-34.eu-west-3.compute.internal |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Tue, Feb 24, 2026 at 08:48:31AM +0900, Michael Paquier wrote:
> On Thu, Feb 19, 2026 at 08:01:36AM +0000, Bertrand Drouvot wrote:
> > On Thu, Feb 19, 2026 at 12:58:12PM +0900, Michael Paquier wrote:
> >> 2) The timeout requirement itself, relying on a timeout threshold
> >> controlled by a backend-side configuration.
> >
> > What are you concerns with this?
>
> I am concerned about the three additional points/requirements:
> 1) The need for all processes who want to flush non-transactional
> stats to set up timeouts, unconditionally, which is what the patch
> shows with the new InitializeTimeouts() calls added for example for
> auxiliary processes. This forces the use of SIGALRM in these
> processes,
Right but they all already call pqsignal(SIGALRM, SIG_IGN), so I'm not sure
to get the point.
> This requires an extra unconditional RegisterTimeout(), as well.
Yes, I'm not clear why that's an issue though.
> 2) The need for all the stats to call pgstat_schedule_anytime_update()
> in strategical places. This is less of a burden compared to 1), but
> this leads to more complications in these code paths with the coding
> requirements, especially for custom stats kinds.
I think that's solved with Sami's proposal for variable stats kind (to flush or
schedule when the session is idle).
> 3) Enforcing a flush timeout unconditionally.
What do you mean? It's done only if there is things to flush.
> My main worries are mainly around 1), I guess, with the new SIGALRM
> handler requirements for all auxiliary processes. Using a procsignal
> path would allow us to rely on a solution that has the same
> flexibility, combined with strategic additional flush calls that we
> could spread depending on requirements we want to enforce in some
> processes, like in the WAL sender, or perhaps the checkpointer.
I see, but we know they'll have to flush IO or WAL stats at some point so
enabling the timeout for those look ok.
> The addition of the property to track if a
> stats kind of OK to flush outside a transaction boundary is also
> critical to have, of course. I am sold to the point point of the
> design about this new property tracked in the stats kind meta-data.
Great!
> 039549d70f6 goes in line with the "client" prospective, where I would
> like to think that strategic flush calls are more flexible.
>
> > Yeah, after our off-list discussion yesterday, I tried to implement the same
> > trick that f1e251be80a has done with injection points (nice trick by the way!),
> > but that led to:
>
> In this case, avoiding an injpoint allocation in a critical section
> would be a two-step process:
> - INJECTION_POINT_LOAD(), before the critical section, to warm up the
> cache and do all the allocations.
> - INJECTION_POINT_CACHED() with IS_INJECTION_POINT_ATTACHED()
> (optional), to run the point, in the critical section.
Oh okay, thanks for the explanation!
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nazir Bilal Yavuz | 2026-02-24 13:57:21 | Re: Speed up COPY FROM text/CSV parsing using SIMD |
| Previous Message | Corey Huinker | 2026-02-24 13:45:26 | Re: Proposal: ANALYZE (MODIFIED_STATS) using autoanalyze thresholds |