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
# Copyright (c) 2024, PostgreSQL Global Development Group use strict; use warnings FATAL => 'all'; use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; # This test ensures that pg_upgrade renames SLRU segments. # After the upgrade all segments should have long file names. # equals SLRU_SEG_FILENAMES_CHANGE_CAT_VER in pg_upgrade.h my $slru_seg_filenames_change_cat_ver = 202411121; my @slru_dirs = ( "pg_xact", "pg_commit_ts", "pg_multixact/offsets", "pg_multixact/members", "pg_subtrans", "pg_serial", ); my $short_segment_name = "1234"; my $long_segment_name = "000000000001234"; my $oldnode = PostgreSQL::Test::Cluster->new('old_node'); $oldnode->init(); my $oldbindir = $oldnode->config_data('--bindir'); my $newnode = PostgreSQL::Test::Cluster->new('new_node'); $newnode->init(); my $newbindir = $newnode->config_data('--bindir'); # Fill data_dir of the old node with SLRU segments that use short file names. # pg_upgrade renames the files without looking at the content, so the content # is not important. foreach my $dir (@slru_dirs) { my $fname = $oldnode->data_dir."/".$dir."/".$short_segment_name; open my $fh, ">", $fname or die $!; close $fh; } # Modify pg_control of the old node to make it look like a version that needs # migration (decrease ControlData->cat_ver). Otherwise pg_upgrade will skip it. my $pg_control_fname = $oldnode->data_dir."/global/pg_control"; open my $fh, "+<", $pg_control_fname or die $!; binmode($fh); sysseek($fh, 12, 0); my $binval = pack("L!", $slru_seg_filenames_change_cat_ver - 1); syswrite($fh, $binval, 4); close($fh); # Calculate CRC of the updated file using pg_read_binary_file() and crc32c() my $fsize = -s $pg_control_fname; # WRONG! should be sizeof(ControlFile) $newnode->start; my $newcrc; $newnode->psql( "postgres", "SELECT crc32(pg_read_binary_file('".$pg_control_fname."',0,".($fsize-4)."));", stdout => \$newcrc, on_error_die => 1, ); $newnode->stop; # Update CRC open $fh, "+<", $pg_control_fname or die $!; binmode($fh); sysseek($fh, $fsize-4, 0); my $bincrc = pack("L!", $newcrc); syswrite($fh, $bincrc, 4); close($fh); $newnode->command_ok( [ 'pg_upgrade', '--old-datadir', $oldnode->data_dir, '--new-datadir', $newnode->data_dir, '--old-bindir', $oldbindir, '--new-bindir', $newbindir, ], 'run of pg_upgrade'); # Check that pg_upgrade renamed the SLRU segments we created foreach my $dir (@slru_dirs) { ok(-e $newnode->data_dir."/".$dir."/".$long_segment_name); } done_testing();