Hi, Heikki! On Tue, 28 Nov 2023 at 13:13, Heikki Linnakangas <hlinn...@iki.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.