Re: XID formatting and SLRU refactorings (was: Add 64-bit XIDs into PostgreSQL 15)

On 09/03/2023 17:21, Aleksander Alekseev wrote:
> v57-0001-Index-SLRUs-by-64-bit-integers-rather-than-by-32.patch

Reviewing this now. I think it's almost ready to be committed.

There's another big effort going on to move SLRUs to the regular buffer
cache (https://commitfest.postgresql.org/43/3514/). I wonder how moving
to 64 bit page numbers will affect that. BlockNumber is still 32 bits,
after all.

> +/*
> + * An internal function used by SlruScanDirectory().
> + *
> + * Returns true if a file with a name of a given length may be a correct
> + * SLRU segment.
> + */
> +static inline bool
> +SlruCorrectSegmentFilenameLength(SlruCtl ctl, size_t len)
> +{
> + if (ctl->long_segment_names)
> + return (len == 12);
> + else
> +
> + /*
> + * XXX Should we still consider 5 and 6 to be a correct length? It
> + * looks like it was previously allowed but now SlruFileName() can't
> + * return such a name.
> + */
> + return (len == 4 || len == 5 || len == 6);
> +}

Huh, I didn't realize that 5 and 6 character SLRU segment names are
possible. But indeed they are. Commit 638cf09e76d allowed 5-character
lengths:

> commit 638cf09e76d70dd83d8123e7079be6c0532102d2
> Author: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
> Date: Thu Jan 2 18:17:29 2014 -0300
>
> Handle 5-char filenames in SlruScanDirectory
>
> Original users of slru.c were all producing 4-digit filenames, so that
> was all that that code was prepared to handle. Changes to multixact.c
> in the course of commit 0ac5ad5134f made pg_multixact/members create
> 5-digit filenames once a certain threshold was reached, which
> SlruScanDirectory wasn't prepared to deal with; in particular,
> 5-digit-name files were not removed during truncation. Change that
> routine to make it aware of those files, and have it process them just
> like any others.
>
> Right now, some pg_multixact/members directories will contain a mixture
> of 4-char and 5-char filenames. A future commit is expected fix things
> so that each slru.c user declares the correct maximum width for the
> files it produces, to avoid such unsightly mixtures.
>
> Noticed while investigating bug #8673 reported by Serge Negodyuck.
>

And later commit 73c986adde5, which introduced commit_ts, allowed 6
characters. With 8192 block size, pg_commit_ts segments are indeed 5
chars wide, and with 512 block size, 6 chars are needed.

This patch makes the filenames always 12 characters wide (for SLRUs that
opt-in to the new naming). That's actually not enough for the full range
that a 64 bit page number can represent. Should we make it 16 characters
now, to avoid repeating the same mistake we made earlier? Or make it
more configurable, on a per-SLRU basis. One reason I don't want to just
make it 16 characters is that WAL segment filenames are also 16 hex
characters, which could cause confusion.

--
Heikki Linnakangas
Neon (https://neon.tech)