Re: Add 64-bit XIDs into PostgreSQL 15

At Mon, 14 Mar 2022 19:43:40 +0400, Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)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