Re: pgstat: Flush some statistics within running transactions, take 2

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>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Lukas Fittl <lukas(at)fittl(dot)com>
Subject: Re: pgstat: Flush some statistics within running transactions, take 2
Date: 2026-08-27 05:21:16
Message-ID: ao/JTOS3gK1gtWwp@bdtpg
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Mon, Aug 24, 2026 at 08:34:50PM -0500, Sami Imseih wrote:
> Hi,
>
> > Yeah, that would address my concern and keep memcmp(), so +1.
>
> Thanks. I went ahead and did that in v8-0002 (previously v7-0001).

Thanks for the new version!

> That issue is already present in HEAD and is not introduced by this
> patch set. pgstat_prep_database_pending() can already ERROR on OOM while
> setting up the database pending entry, so this is not new here.
>
> That said, preparing the database pending entry before updating any
> stats is probably a better idea just for consistency during such cases,
> so I split that out as a standalone fix in v8-0001.

=== 1

Thanks! Looking closer, I think there is still another window though:

pgstat_unlock_entry() calls LWLockRelease(), which can process a pending
interrupt. At that point, the shared relation stats have been updated, while the
database pending stats and flushed baseline have not. A later retry could then
apply the same delta again.

That is also already possible in HEAD and should be very rare in practice. That
said, I think it is worth mentioning and deciding whether we want to handle this
case too.

=== 2

+ else if (entry_ref->flushed_this_pass)
+ {
+ /*
+ * The entry is already pending and was already visited in the current
+ * flush pass. Move it to the tail so the data just accumulated into
+ * it is flushed again before the pass ends.
+ */
+ dlist_move_tail(&pgStatPending, &entry_ref->pending_node);

IIUC, flushed_this_pass remains set after the flush pass ends. So, the next
pgstat_update_dbstats() moves the database entry to the tail before the new pass
starts.

Should we clear this flag between passes, or at least check pgStatFlushInProgress
here?

I did not review the tests in detail yet.

Regards,

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2026-08-27 05:30:24 Re: tablecmds: fix bug where index rebuild loses replica identity on partitions
Previous Message Ewan Young 2026-08-27 05:17:12 Re: right() returns the whole string for the most negative n