Re: Add 64-bit XIDs into PostgreSQL 15

At Mon, 14 Mar 2022 19:43:40 +0400, Pavel Borisov <pashkin.elfe@gmail.com> wrote in 
> > I'd like to add a few quick notes on what's been done in v17.

I have some commens by a quick look-through.  Apologize in advance for
wrong comments from the lack of the knowledge of the whole patch-set.

> > Patches 0001 and 0002 that are planned to be committed to PG15 are almost
> > unchanged with the exception of one unnecessary cast in 0002 removed.

0001:

 The XID_FMT has quite bad impact on the translatability of error
 messages.  3286065651 has removed INT64_FORMAT from translatable
 texts for the reason.  This re-introduces that in several places.
 0001 itself does not harm but 0005 replaces XID_FMT with
 INT64_FORMAT.  Other patches have the same issue, too.

> > We've also addressed several issues in patch 0005 (which is planned for
> > PG16):
> > - The bug with frozen xids after pg_upgrade, reported by Justin [1]
> > - Added proper processing of double xmax pages in
> > HeapPageSetPruneXidInternal()
> > - Fixed xids comparison. Initially in the patch it was changed to simple <
> > <= => > for 64 bit values. Now v17 patch has returned this to the way
> > similar to what is used in STABLE for 32-bit xids, but using modulus-64
> > numeric ring. The main goal of this change was to fix SRLU tests that were mentioned

If IIUC, the following part in 0002 doesn't consider wraparound.

-asyncQueuePagePrecedes(int p, int q)
+asyncQueuePagePrecedes(int64 p, int64 q)
 {
-	return asyncQueuePageDiff(p, q) < 0;
+	return p < q;
 }

> > by Alexander to have been disabled. We've fixed and enabled most of them,
> > but some of them are still need to be fixed and enabled.
> >
> > Also, we've pgindent-ed all the patches.

0005 has "new blank line at EOF".

> > As patches that are planned to be delivered to PG15 are almost unchanged,
> > I completely agree with Alexander's plan to consider these patches (0001
> > and 0002) as RfC.
> >
> > All activity, improvement, review, etc. related to the whole patchset is
> > also very much appreciated. Big thanks to Alexander for working on the
> > patch set!
> >
> > [1]
> > https://www.postgresql.org/message-id/20220115063925.GS14051%40telsasoft.com
> >
> Also, the patch v17 (0005) returns SLRU_PAGES_PER_SEGMENT to the previous
> value of 32.

0002 re-introduces pg_strtouint64, which have been recently removed by
3c6f8c011f.
>     Simplify the general-purpose 64-bit integer parsing APIs
>     
>     pg_strtouint64() is a wrapper around strtoull/strtoul/_strtoui64, but
>     it seems no longer necessary to have this indirection.
..
>    type definition int64/uint64.  For that, add new macros strtoi64() and
>    strtou64() in c.h as thin wrappers around strtol()/strtoul() or
>    strtoll()/stroull().  This makes these functions available everywhere

regards.


-- 
Kyotaro Horiguchi
NTT Open Source Software Center