| From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | Sami Imseih <samimseih(at)gmail(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Lukas Fittl <lukas(at)fittl(dot)com> |
| Subject: | Re: pgstat: Flush some statistics within running transactions, take 2 |
| Date: | 2026-08-05 11:05:33 |
| Message-ID: | anMY/a34zFTAXeoo@bdtpg |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Tue, Aug 04, 2026 at 09:49:30PM -0500, Sami Imseih wrote:
> > We can see that after the first force, relation statistics contains 100 tuples,
> > but the database aggregate does not (while the second force adds them).
> >
> > This is because it keeps the flushed database entry in the pending list. When the
> > relation callback later adds counters to that already visited entry, it is not
> > requeued because entry_ref->pending is non-NULL. Then, those counters wait
> > until the next flush.
> >
> > That could also happen for custom stats that updates an already visited retained
> > entry.
> >
> > One option could be to keep pending entry memory allocated, but track queue
> > membership separately and requeue any entry that receives new counters after
> > being processed.
>
> I think the fix is to separate cross-kind accumulation from the
> flush itself. Rather than having flush_pending_cb call
> pgstat_prep_database_pending() while we're iterating the pending
> list, what do you think of adding a post_flush_pending_cb that
> runs after a successful flush? At a transaction boundary this
> works naturally. The flushed relation entry gets deleted, the
> callback re-creates the database entry, and is guaranteed to
> be visited.
One thing is that v3 flush_pending_cb clears the fields it publishes before
returning, so the post-flush callback would need the flushed delta to remain
available.
> For mid-transaction flushing where entries stay on the list, a
> dependent entry that was already visited won't be reached again
> in the first pass because it's not a new entry. So, if we are
> mid-transaction and still have pending data, we can take a second
> pass to handle the stats accumulated during the post flush
> callback.
I'm not sure one additional pass is enough for custom stats.
For example, say the list order is C, B, A, with A updating B and B updating C.
The first pass updates B, the second pass flushes B and updates the already-visited C,
a third pass is then needed.
Also, if "still have pending data" means "have_pending", that would not identify
newly generated work, as retained entries can keep it true. It seems that newly
generated work needs to be tracked and requeued separately.
> Adding post_flush_pending_cb can go in as a pre-requisite
> commit. This is also better in terms of separation of
> responsibilities between flushing the kinds stats and post
> flush actions.
I'm not sure a new callback is needed here. The relation callback already performs
the database accumulation only after a successful flush. If pending lifetime
and queue membership are separated, the existing pgstat_prep_database_pending()
call could requeue the database entry and preserve the current behavior.
I think that a post-flush callback might improve organization, but that seems
independent of this issue and so would need to be justified on its own.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Borodin | 2026-08-05 11:07:02 | Re: Fix archive restore race that could unlink WAL before rename |
| Previous Message | Dinesh Salve | 2026-08-05 10:47:51 | Re: explain plans for foreign servers |