Hi Ashutosh,
Thank you for continuing to review the patch. Attached is v9,
incorporating the feedback. Please see my responses inline below.
On Fri, Sep 12, 2025 at 5:34 AM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
> + Returns statistics about current multixact usage:
> + <literal>num_mxids</literal> is the total number of multixact IDs
> assigned since startup,
> + <literal>num_members</literal> is the total number of multixact
> member entries created since startup,
>
> GetMultiXactInfo() returns following
> *members = nextOffset - *oldestOffset;
> *multixacts = nextMultiXactId - *oldestMultiXactId;
> They seem to be the numbers that exist in the system at the time of
> the call and not since the startup. Am I missing something?
You are right, these counts reflect the numbers currently present in
the system, not cumulative totals since startup. I have reworded the
docs to say “currently present”.
> + up to approximately 2^32 entries(occupying roughly 20GB in the
>
> space between s and (
Fixed.
> + proallargtypes => '{int4,int8,int8,xid,int8}',
>
> I think the first parameter should also be int8 since uint32 won't fit
> into int4.
Updated.
> > > + See <xref linkend="vacuum-for-multixact-wraparound"/> for further
> > > details on multixact wraparound.
> > >
> > > I don't think we need this reference here. Reference back from that
> > > section is enough.
> > I kept the cross-reference for now since other multixact function docs
> > (such as pg_get_multixact_members()) already use this style, and it helps
> > readers who land directly on the function reference page. Please let me
> > know if you would prefer that I remove it.
>
> None of the write up there talks about multixact wraparound so the
> reference seems arbitrary to me. I would remove it.
Removed.
> Thanks for updating the documentation. But the comment in prologue of
> pg_get_multixact_stats is not completely correct as mentioned in my
> previous reply. I would just say "Returns NULL if the oldest
> referenced offset is unknown". Anybody who wants to know when can that
> happen, may search relevant code by looking at GetMultiXactInfo().
Simplified the prologue comment as suggested.
> I still find the comment at the start of the isolation test a bit
> verbose. But I think it's best to leave that to a committer's
> judgement. However, the word "locker" is unusual. You want to say the
> session that locks a row (or something similar). We may leave exact
> words to a committer's judgement.
Reworded to remove "locker" and simplified.
> I still find think that the specific usage scenarios described in the
> documentation are not required. And the computation in
> pg_get_multixact_stats() should use macros instead of bare numbers.
> But we can leave that for a committer to decide. Once you address
> above comments, we may mark the CF entry as RFC.
Sounds good!
With these updates in v9, I believe the patch is now in good shape to
be marked RFC. I’ll go ahead and update the CommitFest entry.
Thank you again for your thorough reviews and thoughtful guidance on
this patch — it has been very helpful.
Best regards,
Naga
