Re: [Proposal] Expose internal MultiXact member count function for efficient monitoring - Mailing list pgsql-hackers

Hi Ashutosh,

Thank you for continuing to review the patch. Attached is v8,
incorporating the feedback. Please see my responses inline below.

On Fri, Sep 5, 2025 at 6:27 AM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> This one is remaining.
> + up to approximately 2^32 entries before reaching wraparound.
>
> ... 2^32 entries (occupying roughly 20GB in the
> <literal>pg_multixact/members</literal> directory) before reaching
> wraparound. ...
Done.

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

> + * Returns NULL if the oldest referenced offset is unknown, which
> happens during
> + * system startup or when no MultiXact references exist in any relation.
>
> If no MultiXact references exist, and GetMultiXactInfo() returns
> false, MultiXactMemberFreezeThreshold() will assume the worst, which I
> take as meaning that it will trigger aggressive autovacuum. No
> MultiXact references existing is a common case which shouldn't be
> assumed as the worst case. The comment I quoted means "the oldest
> value of the offset referenced by any multi-xact referenced by a
> relation *may not be always known". You seem to have interpreted "may
> not be known" as "does not exist" That's not right. I would write this
> as "Returns NULL if the oldest referenced offset is unknown which
> happens during system startup".
>
> Similarly I would rephrase the following docs as
> + <para>
> + The function returns <literal>NULL</literal> when multixact
> statistics are unavailable.
> + For example, during startup before multixact initialization completes or when
> + the oldest member offset cannot be determined.
>
> "The function returns <literal>NULL</literal> when multixact
> statistics when the oldest multixact offset corresponding to a
> multixact referenced by a relation is not known after starting the
> system."
>
Updated.

> > >
> > > @@ -0,0 +1,127 @@
> > > +# High-signal invariants for pg_get_multixact_stats()
>
> What does "High-signal" mean here? Is that term defined somewhere?
> Using terms that most of the contributors are familiar with improves
> readability. If a new term is required, it needs to be defined first.
> But I doubt something here requires defining a new term.
Dropped that wording and simplified the isolation test.

> > > What's a driver transaction?
> > A driver transaction is simply the controlling session that stays open
> > while snapshots are taken.
>
> I still don't understand the purpose of this transaction.
> pg_get_multixact_stats() isn't transactional so the driver transaction
> isn't holding any "snapshot" of the stats. It's also not creating any
> multixact and hence does not contribute to testing the output of
> pg_get_multixact_stats. Whatever this session is doing, can be done
> outside a transaction too. Which step in this session requires an
> outer transaction?
Removed this mention; the test now only checks monotonicity without extra
transaction scaffolding.

> Some more comments
> + Returns statistics about current multixact usage:
> + <literal>num_mxids</literal> is the number of multixact IDs assigned,
>
> Is this the number of multixact IDs assigned till now (since whatever
> time) or the number of multixact IDs currently in the system?
>
> + <literal>num_members</literal> is the number of multixact member
> entries created,
Updated.

> + Returns statistics about current multixact usage:
> + <literal>num_mxids</literal> is the number of multixact IDs assigned,
>
> Is this the number of multixact IDs assigned till now (since whatever
> time) or the number of multixact IDs currently in the system?
>
> + <literal>num_members</literal> is the number of multixact member
> entries created,
Updated.


> + multixact allocation and usage patterns in real time. For example:
>
> suggestion: ... real time, for example: ... Otherwise the sentence
> started by "For example" is not a complete sentence.
Updated.

> + values[0] = Int32GetDatum(multixacts);
>
> This should be UInt32GetDatum() multixacts is uint32.
>
> + values[1] = Int64GetDatum(members);
>
> Similarly this since MultiXactOffset is uint32.
>
> + values[4] = Int64GetDatum(oldestOffset);
>
> Similarly this since MultiXactOffset is uint32.
Thanks for pointing this out. I had originally followed the existing
types but drifted, fixed now.

> +# Get MultiXact state
> +{
> + oid => '9001',
> + descr => 'get current multixact member and multixact ID counts and
> oldest values',
>
> suggestion: get current multixact usage statistics.
Updated

> + proname => 'pg_get_multixact_stats',
> + prorettype => 'record',
> + proargtypes => '',
> + proallargtypes => '{int4,int8,int8,xid,int8}',
> + proargmodes => '{o,o,o,o,o}',
> + proargnames =>
> '{num_mxids,num_members,members_size,oldest_multixact,oldest_offset}',
> + provolatile => 'v',
> + proparallel => 's',
> + prosrc => 'pg_get_multixact_stats'
> +},
>
> I like the way you have formatted the new entry, but other entries in
> this file are not formatted this way. It would be good to format it
> like other entries but if other reviewers prefer this way, we can go
> with this too.
I reformatted the pg_proc.dat entry to match the surrounding style.

Best regards,
Naga