Re: XID formatting and SLRU refactorings

On 28/11/2023 12:14, Pavel Borisov wrote:
> On Tue, 28 Nov 2023 at 13:13, Heikki Linnakangas <hlinnaka@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.

That's right. I'm arguing that now is a good time to clean it up.

I won't insist if Alexander prefers to commit it as it is, though. But 
let's at least explain how this works in the comment, instead of the XXX.

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