Обсуждение: Re: pg_get_multixact_members not documented

On Fri, May 30, 2025 at 02:23:30PM -0500, Sami Imseih wrote:
> While looking into another multixact related topic, I realised that
> pg_get_multixact_members
> is not documented. I saw the lack of documentation was mentioned here [0], but
> this was never committed.
> 
> Any reason it should not be?

It looks to me like Álvaro just never got any feedback on the last patch he
sent.  I noticed that there are no in-tree uses, but I do see a couple of
blog posts that recommend it.  In any case, I can't think of a reason it
ought to remain undocumented.  Want to put together a patch?

-- 
nathan

On Fri, May 30, 2025 at 04:24:43PM -0500, Sami Imseih wrote:
>> Want to put together a patch?
> 
> Yes, will do

For extra credit, maybe we could add a test or two, too...

-- 
nathan

On Mon, Jun 02, 2025 at 12:46:51PM -0500, Sami Imseih wrote:
> v1-0001 is the documentation only patch. I improved upon the description
> suggested in [0]

Your patch adds an entry to the "Transaction ID and Snapshot Information
Functions" table, while Álvaro's introduced a new "Multixact Functions"
table.  His also added a note to maintenance.sgml.  Any reason for the
differences?

> A simple test will be a regress/sql which ensure the XID and lock mode
> of a transaction using a savepoint, something like the below. To do anything
> fancier with concurrency, we will need an isolation test.
> 
> ```
> drop table if exists t;
> create table t (v int);  insert into t values (1);
> begin;
> select from t for update ;
> savepoint s1;
> update t set v = v;
> select pg_get_multixact_members(a.relminmxid), a.relminmxid from
> (select relminmxid from pg_class where relname = 't') a;
> commit;
> ```

That seems reasonable to me.

-- 
nathan

v2 addresses the comments.

Adds a new section called "Multixact Information Functions" and a reference
to pg_get_multixact_members after the description of what multixact members
are in maintenance.sgml.

As I spent some time looking into this, I still think we should document this
function because of its use in blogs and examples that describe multixact.

However, this function does not appear to be very practical to use, because
the only visible MXID to the user is the oldest one, via pg_database.datminmxid
or pg_class.relfrozenxid, or with the help of the contrib extension pgrowlocks.

To actually find all multixact IDs and their associated XIDs that are yet to
be vacuumed, I could write a query like the following:

SELECT
b as mxid,
a.*
FROM
(SELECT min(datminmxid::text::int) min_datminmxid FROM pg_database) d,
generate_series(d.min_datminmxid, mxid_age(d.min_datminmxid::text::xid)) as b,
pg_get_multixact_members(b::text::xid) a

Another thing is in [0], we mention two things:

Running transactions and prepared transactions can be ignored if there is
no chance that they might appear in a multixact.

MXID information is not directly visible in system views such as
pg_stat_activity; however, looking for old XIDs is still a good way of
determining which transactions are causing MXID wraparound problems.

I really think this is an area that needs improvement. We tell users to ignore
transactions that have "no chance" of appearing in a multixact, but what that
really means is the user must somehow figure this out on their own. I don't
think it would be unrealistic to expose real-time information about backends
with transactions involved in multixacts.

Looking at a recent public discussion about multixact [1], it lists
"lack of direct visibility" as one of the areas for improvement. This
will be the
subject of a different thread, but I thought it would be good to mention it
here first as it's relevant.

[0] https://www.postgresql.org/docs/current/routine-vacuuming.html
[1] https://metronome.com/blog/root-cause-analysis-postgresql-multixact-member-exhaustion-incidents-may-2025

--
Sami Imseih
Amazon Web Services (AWS)

Thanks for the review!

> +1. PFA diff of some edits. Please incorporate them in
> your patch if you find them correct.

sure, the diff looks fine to me. will fix.

> We developers may understand the mode text "sh", "keysh" etc. But it may not be user friendly.

yes, I can see that being a point of confusion.

> It would have been better if the function would have reported standard modes as described in [1]
> but we can't change that now.

Really, the section this relates to is row-level locks [0], and for
multixact, there are 2 additional
modes "nokeyupd" which refer to locks takes by updated in no key mode,
and "upd" which
are other types of updates and deletes.

> It might help if we add mapping in the documentation.

I am not too thrilled with a new table mapping, because of the maintenance
aspect of keeping it up-to-date ( not that locks change often or ever
). I rather
describe that the modes are abbreviated names for locks referenced in
[1] and that
there are also some additional modes specific to multixact.

> "" ... each member of the multixact identified by the given mxid' sounds more
> accurate compared to " ... each member of a given multixact ID". That document
> page seems to differentiate between a transaction and transaction ID. I think we
> should try to do the same for multixact.

I like my wording more. It fits the style of the way we describe
functions better. Also,
we don't use "mxid" in the docs, but do use "Transaction ID" and "multixact ID"

I did change " for each " to "of each", which is more accurate.

>> However, this function does not appear to be very practical to use, because
>> the only visible MXID to the user is the oldest one, via pg_database.datminmxid
>> or pg_class.relfrozenxid, or with the help of the contrib extension pgrowlocks.
>
>
> If somebody starts from a row itself e.g. select xmax, * from t1 ... ,
> this function gives the transactions that have locked a given row. So not that useless.

I did not mean "useless", but I did say "not practical", as it
requires a bit of understanding
of the multixact system and multixact ID generation to use. The
example usage you mention
and the example I mentioned earlier [1] demonstrates this.

[0] https://www.postgresql.org/docs/current/explicit-locking.html#LOCKING-ROWS
[1] https://www.postgresql.org/message-id/CAA5RZ0vJat3-VumsTGagxzY35T%3DrCSNyH9GXwkdB3-FFY4as8A%40mail.gmail.com

See v3 addressing the comments.

-- 
Sami

> Attached patch removing extra comma. Otherwise LGTM.

Thanks! For v4, the final comma in the list is grammatically correct,
and it’s the style we use throughout the docs.

I marked the patch RFC.

--
Sami

On Thu, Jun 12, 2025 at 12:15:03AM -0500, Sami Imseih wrote:
>> Attached patch removing extra comma. Otherwise LGTM.
> 
> Thanks! For v4, the final comma in the list is grammatically correct,
> and it´s the style we use throughout the docs.

+1

> I marked the patch RFC.

Barring comments/objections, I'll plan on committing/back-patching this in
the near future.

-- 
nathan

On Sat, Jun 28, 2025 at 4:02 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Thu, Jun 12, 2025 at 03:10:56PM -0500, Nathan Bossart wrote:
> > Barring comments/objections, I'll plan on committing/back-patching this in
> > the near future.
>
> Here is what I have staged for commit.  I ended up moving it to the
> "Transaction ID and Snapshot Information Functions" table, which is what I
> think you had originally proposed.  Creating a new section for this seemed
> unnecessary, and this table already has one multixact-related function.

WFM.


--
Best Wishes,
Ashutosh Bapat

On Mon, Jun 30, 2025 at 7:29 AM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> On Sat, Jun 28, 2025 at 4:02 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
> >
> > On Thu, Jun 12, 2025 at 03:10:56PM -0500, Nathan Bossart wrote:
> > > Barring comments/objections, I'll plan on committing/back-patching this in
> > > the near future.
> >
> > Here is what I have staged for commit.  I ended up moving it to the
> > "Transaction ID and Snapshot Information Functions" table, which is what I
> > think you had originally proposed.  Creating a new section for this seemed
> > unnecessary, and this table already has one multixact-related function.
>
> WFM.

Correct, I originally proposed the "Transaction ID and Snapshot
Information Functions"
section, but as stated in [0],
pg_get_multixact_members does not fit the description of the section as it
is described as  "The main use of these functions is to determine
which transactions
were committed between two snapshots."

IMO, it makes more sense to keep this function in a new section as we
have in v4.

[0] https://www.postgresql.org/message-id/CAA5RZ0vtSPzx%2B8HW1gohcWPgURX2VST5j3Nqh2OL6h9dSk0HSg%40mail.gmail.com

--
Sami

On Mon, Jun 30, 2025 at 04:08:04PM +0300, Sami Imseih wrote:
> Correct, I originally proposed the "Transaction ID and Snapshot
> Information Functions" section, but as stated in [0],
> pg_get_multixact_members does not fit the description of the section as
> it is described as  "The main use of these functions is to determine
> which transactions were committed between two snapshots."

Well, is that description correct?  I've used several of these functions,
but I'm not sure I've ever used them for this purpose.

Looking again, pg_get_multixact_members() might need to be added to this
list of exceptions:

    However, the functions shown in Table 9.84, except age and mxid_age,
    use a 64-bit type xid8 that does not wrap around during the life of an
    installation and can be converted to xid by casting if required; see
    Section 67.1 for details.

I'm also a little wary of adding a new section on the back-branches because
it will change some section numbers.

-- 
nathan

> On Mon, Jun 30, 2025 at 04:08:04PM +0300, Sami Imseih wrote:
> > Correct, I originally proposed the "Transaction ID and Snapshot
> > Information Functions" section, but as stated in [0],
> > pg_get_multixact_members does not fit the description of the section as
> > it is described as  "The main use of these functions is to determine
> > which transactions were committed between two snapshots."
>
> Well, is that description correct?  I've used several of these functions,
> but I'm not sure I've ever used them for this purpose.

I suppose the description is not applicable to all of these functions;
for example
pg_current_xact_id_if_assigned()  returns the current transactions xid, so the
description does not apply. Neither does it apply to pg_snapshot_xip().

this description has been around since 8.3 from what I can tell, and
when the inventory
of functions in this section was much smaller and most required a snapshot as an
argument.

Perhaps we should think about removing this description, what do you think?

> Looking again, pg_get_multixact_members() might need to be added to this
> list of exceptions:
>
>         However, the functions shown in Table 9.84, except age and mxid_age,
>         use a 64-bit type xid8 that does not wrap around during the life of an
>         installation and can be converted to xid by casting if required; see
>         Section 67.1 for details.

This function returns an xid and not an int8 such as for example
```
{ oid => '3819', descr => 'view members of a multixactid',
  proname => 'pg_get_multixact_members', prorows => '1000', proretset => 't',
  provolatile => 'v', prorettype => 'record', proargtypes => 'xid',
  proallargtypes => '{xid,xid,text}', proargmodes => '{i,o,o}',
  proargnames => '{multixid,xid,mode}', prosrc => 'pg_get_multixact_members' },
```
```
{ oid => '2943', descr => 'get current transaction ID',
  proname => 'txid_current', provolatile => 's', proparallel => 'u',
  prorettype => 'int8', proargtypes => '', prosrc => 'pg_current_xact_id' },
```
am I missing something?

> I'm also a little wary of adding a new section on the back-branches because
> it will change some section numbers.

Sure, I am not against keeping the function in an existing section, but
we should remove the description mentioned above for clarity.

--
Sami

> Sure, I am not against keeping the function in an existing section, but
> we should remove the description mentioned above for clarity.

to be clear, I am suggesting we just remove the second sentence
in the description. Therefore, instead of:

"The functions shown in Table 9.84 provide server transaction information
in an exportable form. The main use of these functions is to determine which
transactions were committed between two snapshots."

mention only:

"The functions shown in Table 9.84 provide server transaction
information in an exportable form."

I don't think we need to add anything else.

--
Sami

On Mon, Jun 30, 2025 at 06:19:10PM +0300, Sami Imseih wrote:
> Perhaps we should think about removing this description, what do you think?

I think it's a good topic for another patch/thread.  Chances are it's not
the only description that could be updated.

>> Looking again, pg_get_multixact_members() might need to be added to this
>> list of exceptions:
>>
>>         However, the functions shown in Table 9.84, except age and mxid_age,
>>         use a 64-bit type xid8 that does not wrap around during the life of an
>>         installation and can be converted to xid by casting if required; see
>>         Section 67.1 for details.
> 
> This function returns an xid and not an int8 such as for example
> ```
> { oid => '3819', descr => 'view members of a multixactid',
>   proname => 'pg_get_multixact_members', prorows => '1000', proretset => 't',
>   provolatile => 'v', prorettype => 'record', proargtypes => 'xid',
>   proallargtypes => '{xid,xid,text}', proargmodes => '{i,o,o}',
>   proargnames => '{multixid,xid,mode}', prosrc => 'pg_get_multixact_members' },
> ```
> ```
> { oid => '2943', descr => 'get current transaction ID',
>   proname => 'txid_current', provolatile => 's', proparallel => 'u',
>   prorettype => 'int8', proargtypes => '', prosrc => 'pg_current_xact_id' },
> ```
> am I missing something?

That's what I mean.  I think it should say

    However, the functions shown in Table 9.84, except age, mxid_age, and
    pg_get_multixact_members, use a 64-bit type xid8 that...

I noticed that this list of exceptions doesn't exist on v13-v15, presumably
because the docs for age() and mxid_age() were only back-patched to v16
(see commits 48b5aa3 and 15afb7d), which is strange because I believe those
functions are much older.  I don't see any discussion about this choice,
either.  We should probably back-patch those commits to v13 as a
prerequisite to applying this patch.

-- 
nathan

On Mon, Jun 30, 2025 at 02:54:28PM -0500, Nathan Bossart wrote:
> I noticed that this list of exceptions doesn't exist on v13-v15, presumably
> because the docs for age() and mxid_age() were only back-patched to v16
> (see commits 48b5aa3 and 15afb7d), which is strange because I believe those
> functions are much older.  I don't see any discussion about this choice,
> either.  We should probably back-patch those commits to v13 as a
> prerequisite to applying this patch.

Spun off a new thread for this:

    https://postgr.es/m/aGMCxHxLfeMdQk8m%40nathan

-- 
nathan

>> Perhaps we should think about removing this description, what do you think?

> I think it's a good topic for another patch/thread.  Chances are it's not
> the only description that could be updated.

Sure, that could be taken up after this patch.

> That's what I mean.  I think it should say
>
>         However, the functions shown in Table 9.84, except age, mxid_age, and
>         pg_get_multixact_members, use a 64-bit type xid8 that...

I agree. v7 includes this change.

--

Sami

Committed.

-- 
nathan