Re: XID formatting and SLRU refactorings

From: Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Maxim Orlov <orlovmg(at)gmail(dot)com>, Aleksander Alekseev <aleksander(at)timescale(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Japin Li <japinli(at)hotmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(at)paquier(dot)xyz>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Subject: Re: XID formatting and SLRU refactorings
Date: 2023-11-28 10:14:51
Message-ID: CALT9ZEG_H_nF9thT4vi+q9PQQYUNF3g9v=UyT7w0jkDJwy59KQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi, Heikki!

On Tue, 28 Nov 2023 at 13:13, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>
> On 27/11/2023 01:43, Alexander Korotkov wrote:
> > v61 looks good to me. I'm going to push it as long as there are no objections.
> This was discussed earlier, but is still present in v61:
>
> > +/*
> > + * 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 == 15); /* see SlruFileName() */
> > + else
> > + /*
> > + * Commit 638cf09e76d allowed 5-character lengths. Later commit
> > + * 73c986adde5 allowed 6-character length.
> > + *
> > + * XXX should we still consider such names to be valid?
> > + */
> > + return (len == 4 || len == 5 || len == 6);
> > +}
> > +
>
> I think it's pretty sloppy that the "short" filenames can be 4, 5 or 6
> chars long. For pg_multixact/members, which introduced the 5-char case,
> I think we should always pad the filenames 5 characters, and for
> commit_ts which introduced the 6 char case, always pad to 6 characters.
>
> Instead of a "long_segment_names" boolean, how about an integer field,
> to specify the length.
>
> That means that we'll need pg_upgrade to copy pg_multixact/members files
> under the new names. That should be pretty straightforward.

I think what's done in patch 0001 is just an extension of existing
logic and moving it into separate function.

- if ((len == 4 || len == 5 || len == 6) &&
+ if (SlruCorrectSegmentFilenameLength(ctl, len) &&
strspn(clde->d_name, "0123456789ABCDEF") == len)
{
- segno = (int) strtol(clde->d_name, NULL, 16);
+ segno = strtoi64(clde->d_name, NULL, 16);
segpage = segno * SLRU_PAGES_PER_SEGMENT;

I'd prefer to leave it as it is as a part of 64-bit extension patch.

Regards,
Pavel.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2023-11-28 10:16:25 Re: Testing autovacuum wraparound (including failsafe)
Previous Message Daniel Verite 2023-11-28 10:12:52 Re: proposal: change behavior on collation version mismatch