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-16 08:27:56
Message-ID: aZLVDJLTiFlxsWZi@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 Fri, Feb 13, 2026 at 08:23:01PM -0600, Sami Imseih wrote:
> > PFA attached v6, addressing the reviews comments.
>
> v6 is getting closer IMO. Here are some comments I have.

Thanks!

> v6-0001 looks solid, but some minor comments:
> 1/
>
> + pgstat_flush_backend(nowait, PGSTAT_BACKEND_FLUSH_WAL, true);
>
> let's also use explicit (void) cast here

Yeah. The patch did not introduce this inconsistency but let's fix it in passing.

> 2/
>
> + * Timeout handler for flushing non-transactional stats.
>
> I also noticed in v6-0005, we refer to "anytime" stats as
> "non-transactional". It's better to just refer to them as "anytime"
> anywhere instead of "non-transactional:.

Done.

> v6-0002:
>
> The logic here should not try to acquire the lock twice.

Yeah, changed to match the pgstat_wal_flush_cb() logic.

> + -- Force anytime flush (inside transaction!)
> + select pg_stat_force_anytime_flush();
>
> Not sure why we need pg_stat_force_anytime_flush.
> A pg_sleep is sufficient, like below. right?

Right, done.

>
> v6-0003:
>
> 1/
> Suggested doc changes:

Done.

> 2/
> I don't see we have tests for other timeout based GUCs, but it would nice
> to ensure that this woks correctly. Maybe as a custom_stats test where we
> SET stats_flush_interval inside the transaction and make sure the stats flush
> only after the new timeout has passed. Maybe?

Not sure I follow, that's what 0002 is doing.

> v6-0004:
>
> 1/
>
> NIT:
>
> +$node_primary->append_conf('postgresql.conf', "stats_flush_interval= '1s'");
>
> +$node_publisher->append_conf('postgresql.conf', "stats_flush_interval= '1s'");
>
> missing space before the equal sign.

Done.

>
> v6-0005:
>
> 1/
>
> /* Partial flush only happens in anytime mode for FLUSH_ANYTIME stats */
> is_partial_flush = (anytime_only && kind_info->flush_mode ==
> FLUSH_ANYTIME);
>
> Will this be tue at all time? Let's imagine a Kind that flushes all the fields
> ANYTIME, would we not want to delete the pending entry?
>
> + /* if successfull non-partial flush, remove entry */
> + if (did_flush && !is_partial_flush)
> pgstat_delete_pending_entry(entry_ref);
>

Right, and that's why the MIXED flush mode was useful, i.e to be able to distinguish
here. So, in the attached, instead of re-introducing the MIXED flush mode, I added
a "is_partial" bool paramater to the flush_pending_cb().

Regards,

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

Attachment Content-Type Size
v7-0001-Add-pgstat_report_anytime_stat-for-periodic-stats.patch text/x-diff 41.9 KB
v7-0002-Add-anytime-flush-tests-for-custom-stats.patch text/x-diff 9.0 KB
v7-0003-Add-GUC-to-specify-non-transactional-statistics-f.patch text/x-diff 8.9 KB
v7-0004-Remove-useless-calls-to-flush-some-stats.patch text/x-diff 7.7 KB
v7-0005-Change-RELATION-and-DATABASE-stats-to-anytime-flu.patch text/x-diff 34.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hayato Kuroda (Fujitsu) 2026-02-16 08:33:47 RE: Improve docs syntax checking and enable it in the meson build
Previous Message Hunaid Sohail 2026-02-16 08:20:58 Re: Proposal: SELECT * EXCLUDE (...) command