Re: Flush some statistics within running transactions

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Sami Imseih <samimseih(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, 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-02 17:16:30
Message-ID: aYDb7g5hXnaP+5BU@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 Sat, Jan 31, 2026 at 11:16:03AM -0600, Sami Imseih wrote:
> > This makes things confusing because instead of just relying on
> > dlist_is_empty(&pgStatPending) to check if we need to flush anything,
> > the responsibility now moves to the callback, which now also has to
> > account for all the ANYTIME fields.
> >
>
> Thinking about this a bit more, it seems the FLUSH_MIXED may
> not be needed. Instead can we just introduce a new global variable
> called pgstat_report_variable_anytime which acts like
> pgstat_report_fixed, except it's set to true whenever we update
> anytime variable-numbered stats
>
> With this approach, we will not enter pgstat_flush_pending_entries
> inside pgstat_report_anytime_stat unless we have anytime
> variable stats to report.

Thanks for looking at it!

In v5 attached I changed the design so that we don't re-enable the timeout
after each stats flush in ProcessInterrupts(). Instead the timeout is enabled
when we set pgstat_report_fixed to true or in pgstat_prep_pending_entry() when
appropriate. So that we know that when we enter pgstat_report_anytime_stat() that's
for good reasons and we don't need extra checks in it.

> ```
> + /*
> + * Check if there are any non-transactional stats to flush. Avoid
> + * unnecessarily locking the entry if nothing accumulated.
> + */
> + if (!(lstats->counts.numscans > 0 ||
> + lstats->counts.tuples_returned > 0 ||
> + lstats->counts.tuples_fetched > 0 ||
> + lstats->counts.blocks_fetched > 0 ||
> + lstats->counts.blocks_hit > 0))
> + return true;
> ```

Yeah, with the new design in place then those are not needed anymore.

> This feels like an easier approach to reason about and we don't
> need to add a third flush mode.

I do think we still need it. Indeed in 0004 that helps distinguish between
anytime flush or mixed flush (with the help of the new pgstat_report_mixed_anytime
global variable) in pgstat_prep_pending_entry().

Thoughts?

Regards,

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

Attachment Content-Type Size
v5-0001-Add-pgstat_report_anytime_stat-for-periodic-stats.patch text/x-diff 28.5 KB
v5-0002-Add-GUC-to-specify-non-transactional-statistics-f.patch text/x-diff 10.5 KB
v5-0003-Remove-useless-calls-to-flush-some-stats.patch text/x-diff 7.6 KB
v5-0004-Add-FLUSH_MIXED-support-and-implement-it-for-RELA.patch text/x-diff 20.9 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2026-02-02 17:19:43 Re: Flush some statistics within running transactions
Previous Message Nathan Bossart 2026-02-02 17:04:03 Re: Pasword expiration warning