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