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@alvh.no-ip.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)