pgstat: add pgstat_prep_pending() for entry ref pending setup
Hi,
pgstat_prep_pending_entry() bundles three operations in a single call:
lookup, entry creation, and pending data setup. This is convenient for
the common case, but some callers need to do some other things in
between these steps. For example, looking at what needs to be done
for moving pg_stat_statements [1]/messages/by-id/CAA5RZ0vZwR_dSK6fo0P2-EnskUVN0NjLHnGnJMFDPC8-kEW3sQ@mail.gmail.com to the cumulative statistics collector,
it needs to:
1. Look up the entry (pgstat_get_entry_ref with create=false)
2. If not found, check capacity and possibly evict before creating
3. Create the entry (pgstat_get_entry_ref with create=true)
4. Attach pending data
Steps 1-3 are already possible with pgstat_get_entry_ref(), but step 4
has no API.
The attached adds pgstat_prep_pending() which exposes step 4 as a
function. Together with pgstat_get_entry_ref(), callers can now perform
the same work as pgstat_prep_pending_entry() in individual steps.
pgstat_prep_pending_entry() itself is refactored to use the new
function internally.
[1]: /messages/by-id/CAA5RZ0vZwR_dSK6fo0P2-EnskUVN0NjLHnGnJMFDPC8-kEW3sQ@mail.gmail.com
--
Sami Imseih
Amazon Web Services (AWS)
Attachments:
v1-0001-pgstat-add-pgstat_prep_pending-for-entry-ref-pend.patchapplication/octet-stream; name=v1-0001-pgstat-add-pgstat_prep_pending-for-entry-ref-pend.patchDownload+34-21
On Wed, Jul 15, 2026 at 03:50:19PM -0500, Sami Imseih wrote:
pgstat_prep_pending_entry() bundles three operations in a single call:
lookup, entry creation, and pending data setup. This is convenient for
the common case, but some callers need to do some other things in
between these steps. For example, looking at what needs to be done
for moving pg_stat_statements [1] to the cumulative statistics collector,
it needs to:1. Look up the entry (pgstat_get_entry_ref with create=false)
2. If not found, check capacity and possibly evict before creating
3. Create the entry (pgstat_get_entry_ref with create=true)
4. Attach pending dataSteps 1-3 are already possible with pgstat_get_entry_ref(), but step 4
has no API.The attached adds pgstat_prep_pending() which exposes step 4 as a
function. Together with pgstat_get_entry_ref(), callers can now perform
the same work as pgstat_prep_pending_entry() in individual steps.
pgstat_prep_pending_entry() itself is refactored to use the new
function internally.
Hmm. So we have the APIs to do steps 1 to 3, with an eviction cleanup
happening in-between. Your argument is that if you would like to do
something between steps 3 and 4, as we don't have an API to attach
pending data. If there is nothing to do between steps 3 and 4, we can
just:
- use pgstat_get_entry_ref(create=false)
- eviction
- pgstat_prep_pending_entry() (has pgstat_get_entry_ref(create=true))
So my question is: what do you intend to do between steps 3 and 4?
More waiting for some of parallel eviction done in step 2? I was
looking at v4-0004 on the other thread, and it does the combo of
pgstat_get_entry_ref -> eviction -> pgstat_prep_pending_entry(), so I
am wondering what the actual reason for this new separation is. I
don't object to this change, but you are not mentioning why you want
it, as far as I can see?
--
Michael
On Wed, Jul 15, 2026 at 03:50:19PM -0500, Sami Imseih wrote:
pgstat_prep_pending_entry() bundles three operations in a single call:
lookup, entry creation, and pending data setup. This is convenient for
the common case, but some callers need to do some other things in
between these steps. For example, looking at what needs to be done
for moving pg_stat_statements [1] to the cumulative statistics collector,
it needs to:1. Look up the entry (pgstat_get_entry_ref with create=false)
2. If not found, check capacity and possibly evict before creating
3. Create the entry (pgstat_get_entry_ref with create=true)
4. Attach pending dataSteps 1-3 are already possible with pgstat_get_entry_ref(), but step 4
has no API.The attached adds pgstat_prep_pending() which exposes step 4 as a
function. Together with pgstat_get_entry_ref(), callers can now perform
the same work as pgstat_prep_pending_entry() in individual steps.
pgstat_prep_pending_entry() itself is refactored to use the new
function internally.Hmm. So we have the APIs to do steps 1 to 3, with an eviction cleanup
happening in-between. Your argument is that if you would like to do
something between steps 3 and 4, as we don't have an API to attach
pending data. If there is nothing to do between steps 3 and 4, we can
just:
- use pgstat_get_entry_ref(create=false)
- eviction
- pgstat_prep_pending_entry() (has pgstat_get_entry_ref(create=true))So my question is: what do you intend to do between steps 3 and 4?
More waiting for some of parallel eviction done in step 2?
What I am saying is in order to perform steps 1-4 with individual steps
rather than the single pgstat_prep_pending_entry(), the API for step 4,
to prepare the pending entry, is still missing.
What I am saying is that in order to perform steps 1-4 with individual
steps rather than the single pgstat_prep_pending_entry(), the API for
step 4, to prepare the pending entry, is missing.
In the common fast path case (step 1 succeeds, entry already exists), I have
a valid entry_ref from pgstat_get_entry_ref(create=false). To attach
pending data to it without the proposed API, I would have to call
pgstat_prep_pending_entry(), which internally calls pgstat_get_entry_ref
(create=true) again. That repeats all the work that was already done in
step 1, key initialization, setup checks, the GC scan
(pgstat_need_entry_refs_gc),
and the local cache lookup, all to get back the same entry I already hold.
pgstat_prep_pending() avoids that: it attaches pending data directly
to the entry_ref I already have, with none of the redundant overhead.
--
Sami
On Wed, Jul 15, 2026 at 08:40:21PM -0500, Sami Imseih wrote:
What I am saying is that in order to perform steps 1-4 with individual
steps rather than the single pgstat_prep_pending_entry(), the API for
step 4, to prepare the pending entry, is missing.In the common fast path case (step 1 succeeds, entry already exists), I have
a valid entry_ref from pgstat_get_entry_ref(create=false). To attach
pending data to it without the proposed API, I would have to call
pgstat_prep_pending_entry(), which internally calls pgstat_get_entry_ref
(create=true) again. That repeats all the work that was already done in
step 1, key initialization, setup checks, the GC scan
(pgstat_need_entry_refs_gc),
and the local cache lookup, all to get back the same entry I already hold.pgstat_prep_pending() avoids that: it attaches pending data directly
to the entry_ref I already have, with none of the redundant overhead.
Ah, OK. So your argument is about making even the fast path where an
entry already exists. Indeed that looks like a waste to not do a
separation. I initially thought that this was no big deal, but now I
see your point. Your change makes sense, then.
--
Michael
At Thu, 16 Jul 2026 11:11:49 +0900, Michael Paquier <michael@paquier.xyz> wrote in
Ah, OK. So your argument is about making even the fast path where an
entry already exists. Indeed that looks like a waste to not do a
separation. I initially thought that this was no big deal, but now I
see your point. Your change makes sense, then.
I agree that this change makes sense.
One concern I have is the name of the new function.
pgstat_prep_pending() sounds a bit incomplete. Would a name such as
pgstat_prep_pending_for_entry_ref() be clearer?
Regarding the new function's comment, perhaps it could be phrased
similarly to that of pgstat_prep_pending_entry(), for example:
Prepare the given entry to receive pending stats, if not already done.
Finally, pgstat_get_entry_ref() does not return NULL when create is
true, but the new function itself does not enforce that assumption. It
might be worth adding an Assert(entry_ref != NULL), in addition to the
assertion on flush_pending_cb.
Regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Thu, Jul 16, 2026 at 01:04:38PM +0900, Kyotaro Horiguchi wrote:
One concern I have is the name of the new function.
pgstat_prep_pending() sounds a bit incomplete. Would a name such as
pgstat_prep_pending_for_entry_ref() be clearer?
Hmm. pgstat_prep_pending_for_entry_ref() would work here for the
name.
--
Michael