Re: [PATCH] Refactor SLRU to always use long file names

Hi Michael,

Thanks for your feedback!

> Your patch is just doing a rename() of the files from short to long
> names. How about adding a new TAP script in pg_upgrade that creates a
> couple of empty files with short files names in each path that needs
> to do the transfer? Then the test could run one pg_upgrade command
> and check that the new names are in place. You could use a array of
> objects, with the base path, the old name and the new name, then loop
> over it. With the check in check_slru_segment_filenames() based on
> SLRU_SEG_FILENAMES_CHANGE_CAT_VER, that should work.

OK I gave it a try and discovered that the test becomes very ugly very
fast. Attached is the draft of the test to give you an idea of how
it's going to look like.

In order to trigger renaming of SLRU segments first we have to
downgrade the catalog version in the pg_control file. Otherwise the
check in check_slru_segment_filenames() is not going to pass and
pg_upgrade will do nothing (*). This per se is easily done with
binmode() and pack() however the file has a CRC. Calculating it is not
difficult since we have pg_read_binary_file() and crc32c() SQL
functions, although personally I don't find a need for starting a
cluster for this quite satisfactory. The CRC is stored by the offset
`sizeof(ControlData)-4` and unless I'm wrong is platform-dependent.

I see several solutions for this problem:

* We could add sizeof(ControlData) to the output of `pg_controldata`
so we could use it from Perl
* We could teach `initdb` to override the catalog version
* We could implement a new tool for editing pg_control file

On top of that I should add that the test takes about 7 seconds on my
laptop. Apparently executing two initdb's and one pg_upgrade is not
very cheap. This makes me wonder if instead of writing a new test we
should modify 002_pg_upgrade.pl. This however will make the test even
less readable and maintainable.

What do you think?

(*) BTW I noticed a mistake in the commented code. The condition
should be `>=`, not `<`, i.e:

```
if(new_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER)
return;
```

--
Best regards,
Aleksander Alekseev