On Mon, Aug 04, 2025 at 04:51:30PM -0500, Naga Appani wrote:
> The rest of the patch remains the same as v2, which incorporated
> feedback from Ashutosh and Michael (see my previous email for
> details).
>
> Please find v3 attached.
I am reading again what you have here, and I really think that we
should move the SQL function parts of multixact.c into their own new
file, exposing ReadMultiXactCounts() in multixact.h, because I also
suspect that this can become really useful for extensions that aim at
doing things similar to your proposal in terms of data monitoring for
autovacuum wraparound. This means two refactoring patches:
- One to expose the new routine in multixact.h.
- One to move the existing SQL code to its new file.
ReadMultiXactCounts() is also incorrectly named with your proposal to
expose oldestMultiXactId in the information returned to the caller,
where the key point is to make sure that the information retrieved is
consistent across a single LWLock acquisition. So perhaps this should
be named GetMultiXactInformation() or something similar?
The top of ReadMultiXactCounts() (or whatever its new name) should
also document the information returned across a single call. It looks
inconsistent to return oldestMultiXactId if the oldestOffsetKnown is
false. What about oldestOffset itself? Should it be returned for
consistency with the counts and oldestMultiXactId?
--
Michael
