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