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


Reply via email to