Harden TAP tests that intentionally corrupt page checksums.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 25 Mar 2022 18:23:26 +0000 (14:23 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 25 Mar 2022 18:23:26 +0000 (14:23 -0400)
The previous method for doing that was to write zeroes into a
predetermined set of page locations.  However, there's a roughly
1-in-64K chance that the existing checksum will match by chance,
and yesterday several buildfarm animals started to reproducibly
see that, resulting in test failures because no checksum mismatch
was reported.

Since the checksum includes the page LSN, test success depends on
the length of the installation's WAL history, which is affected by
(at least) the initial catalog contents, the set of locales installed
on the system, and the length of the pathname of the test directory.
Sooner or later we were going to hit a chance match, and today is
that day.

Harden these tests by specifically inverting the checksum field and
leaving all else alone, thereby guaranteeing that the checksum is
incorrect.

In passing, fix places that were using seek() to set up for syswrite(),
a combination that the Perl docs very explicitly warn against.  We've
probably escaped problems because no regular buffered I/O is done on
these filehandles; but if it ever breaks, we wouldn't deserve or get
much sympathy.

Although we've only seen problems in HEAD, now that we recognize the
environmental dependencies it seems like it might be just a matter
of time until someone manages to hit this in back-branch testing.
Hence, back-patch to v11 where we started doing this kind of test.

Discussion: https://postgr.es/m/3192026.1648185780@sss.pgh.pa.us

contrib/amcheck/t/001_verify_heapam.pl
src/bin/pg_amcheck/t/003_check.pl
src/bin/pg_amcheck/t/004_verify_heapam.pl
src/bin/pg_basebackup/t/010_pg_basebackup.pl
src/bin/pg_checksums/t/002_actions.pl
src/test/perl/PostgreSQL/Test/Cluster.pm

index bb0997cd5d95f431f1ffd27b577f03393c65360c..019eed33a0c85bdf1b4de281ff4c3f3705becafa 100644 (file)
@@ -7,7 +7,6 @@ use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 
-use Fcntl qw(:seek);
 use Test::More;
 
 my ($node, $result);
@@ -193,8 +192,8 @@ sub corrupt_first_page
    # Corrupt some line pointers.  The values are chosen to hit the
    # various line-pointer-corruption checks in verify_heapam.c
    # on both little-endian and big-endian architectures.
-   seek($fh, 32, SEEK_SET)
-     or BAIL_OUT("seek failed: $!");
+   sysseek($fh, 32, 0)
+     or BAIL_OUT("sysseek failed: $!");
    syswrite(
        $fh,
        pack("L*",
index d984eacb24f2326d3ef80f6d52e4567247370913..0cf67065d6b6b5addfe573f33008cc10c3fc78b4 100644 (file)
@@ -7,7 +7,6 @@ use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 
-use Fcntl qw(:seek);
 use Test::More;
 
 my ($node, $port, %corrupt_page, %remove_relation);
@@ -90,8 +89,8 @@ sub corrupt_first_page
    # Corrupt some line pointers.  The values are chosen to hit the
    # various line-pointer-corruption checks in verify_heapam.c
    # on both little-endian and big-endian architectures.
-   seek($fh, 32, SEEK_SET)
-     or BAIL_OUT("seek failed: $!");
+   sysseek($fh, 32, 0)
+     or BAIL_OUT("sysseek failed: $!");
    syswrite(
        $fh,
        pack("L*",
index 94d691a614d4b9c21e940e6e5f3b1cb16be3d1c3..bbada168f09e490a23e09d5ab40910c9635434b0 100644 (file)
@@ -7,7 +7,6 @@ use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 
-use Fcntl qw(:seek);
 use Test::More;
 
 # This regression test demonstrates that the pg_amcheck binary correctly
@@ -99,8 +98,8 @@ sub read_tuple
 {
    my ($fh, $offset) = @_;
    my ($buffer, %tup);
-   seek($fh, $offset, SEEK_SET)
-     or BAIL_OUT("seek failed: $!");
+   sysseek($fh, $offset, 0)
+     or BAIL_OUT("sysseek failed: $!");
    defined(sysread($fh, $buffer, HEAPTUPLE_PACK_LENGTH))
      or BAIL_OUT("sysread failed: $!");
 
@@ -165,8 +164,8 @@ sub write_tuple
        $tup->{c_va_header},  $tup->{c_va_vartag},
        $tup->{c_va_rawsize}, $tup->{c_va_extinfo},
        $tup->{c_va_valueid}, $tup->{c_va_toastrelid});
-   seek($fh, $offset, SEEK_SET)
-     or BAIL_OUT("seek failed: $!");
+   sysseek($fh, $offset, 0)
+     or BAIL_OUT("sysseek failed: $!");
    defined(syswrite($fh, $buffer, HEAPTUPLE_PACK_LENGTH))
      or BAIL_OUT("syswrite failed: $!");
    return;
index 9aa35a16a45dd4c154d6c4aec0eaee600e698f56..47f3d00ac454e84bbe0a479bd6bfe97af4f6f5e2 100644 (file)
@@ -5,7 +5,6 @@ use strict;
 use warnings;
 use File::Basename qw(basename dirname);
 use File::Path qw(rmtree);
-use Fcntl qw(:seek);
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 use Test::More;
@@ -56,7 +55,7 @@ if (open my $badchars, '>>', "$tempdir/pgdata/FOO\xe0\xe0\xe0BAR")
 }
 
 $node->set_replication_conf();
-system_or_bail 'pg_ctl', '-D', $pgdata, 'reload';
+$node->reload;
 
 $node->command_fails(
    [ @pg_basebackup_defs, '-D', "$tempdir/backup" ],
@@ -706,17 +705,13 @@ my $file_corrupt2 = $node->safe_psql('postgres',
    q{CREATE TABLE corrupt2 AS SELECT b FROM generate_series(1,2) AS b; ALTER TABLE corrupt2 SET (autovacuum_enabled=false); SELECT pg_relation_filepath('corrupt2')}
 );
 
-# set page header and block sizes
-my $pageheader_size = 24;
+# get block size for corruption steps
 my $block_size = $node->safe_psql('postgres', 'SHOW block_size;');
 
 # induce corruption
-system_or_bail 'pg_ctl', '-D', $pgdata, 'stop';
-open $file, '+<', "$pgdata/$file_corrupt1";
-seek($file, $pageheader_size, SEEK_SET);
-syswrite($file, "\0\0\0\0\0\0\0\0\0");
-close $file;
-system_or_bail 'pg_ctl', '-D', $pgdata, 'start';
+$node->stop;
+$node->corrupt_page_checksum($file_corrupt1, 0);
+$node->start;
 
 $node->command_checks_all(
    [ @pg_basebackup_defs, '-D', "$tempdir/backup_corrupt" ],
@@ -727,16 +722,12 @@ $node->command_checks_all(
 rmtree("$tempdir/backup_corrupt");
 
 # induce further corruption in 5 more blocks
-system_or_bail 'pg_ctl', '-D', $pgdata, 'stop';
-open $file, '+<', "$pgdata/$file_corrupt1";
+$node->stop;
 for my $i (1 .. 5)
 {
-   my $offset = $pageheader_size + $i * $block_size;
-   seek($file, $offset, SEEK_SET);
-   syswrite($file, "\0\0\0\0\0\0\0\0\0");
+   $node->corrupt_page_checksum($file_corrupt1, $i * $block_size);
 }
-close $file;
-system_or_bail 'pg_ctl', '-D', $pgdata, 'start';
+$node->start;
 
 $node->command_checks_all(
    [ @pg_basebackup_defs, '-D', "$tempdir/backup_corrupt2" ],
@@ -747,12 +738,9 @@ $node->command_checks_all(
 rmtree("$tempdir/backup_corrupt2");
 
 # induce corruption in a second file
-system_or_bail 'pg_ctl', '-D', $pgdata, 'stop';
-open $file, '+<', "$pgdata/$file_corrupt2";
-seek($file, $pageheader_size, SEEK_SET);
-syswrite($file, "\0\0\0\0\0\0\0\0\0");
-close $file;
-system_or_bail 'pg_ctl', '-D', $pgdata, 'start';
+$node->stop;
+$node->corrupt_page_checksum($file_corrupt2, 0);
+$node->start;
 
 $node->command_checks_all(
    [ @pg_basebackup_defs, '-D', "$tempdir/backup_corrupt3" ],
index 62c608eaf6cc0932f6a131933a761695aa092bc5..0309cbbaa330fdd673940618a12743b230a57338 100644 (file)
@@ -9,7 +9,6 @@ use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 
-use Fcntl qw(:seek);
 use Test::More;
 
 
@@ -24,6 +23,7 @@ sub check_relation_corruption
    my $tablespace = shift;
    my $pgdata     = $node->data_dir;
 
+   # Create table and discover its filesystem location.
    $node->safe_psql(
        'postgres',
        "CREATE TABLE $table AS SELECT a FROM generate_series(1,10000) AS a;
@@ -37,9 +37,6 @@ sub check_relation_corruption
    my $relfilenode_corrupted = $node->safe_psql('postgres',
        "SELECT relfilenode FROM pg_class WHERE relname = '$table';");
 
-   # Set page header and block size
-   my $pageheader_size = 24;
-   my $block_size = $node->safe_psql('postgres', 'SHOW block_size;');
    $node->stop;
 
    # Checksums are correct for single relfilenode as the table is not
@@ -54,10 +51,7 @@ sub check_relation_corruption
    );
 
    # Time to create some corruption
-   open my $file, '+<', "$pgdata/$file_corrupted";
-   seek($file, $pageheader_size, SEEK_SET);
-   syswrite($file, "\0\0\0\0\0\0\0\0\0");
-   close $file;
+   $node->corrupt_page_checksum($file_corrupted, 0);
 
    # Checksum checks on single relfilenode fail
    $node->command_checks_all(
index 0295731bd0acfe058906f7862d229ec6248c3717..bee6aacf47cf39bea1d4b6d325481f1e98d46927 100644 (file)
@@ -2856,6 +2856,37 @@ sub pg_recvlogical_upto
 
 =pod
 
+=item $node->corrupt_page_checksum(self, file, page_offset)
+
+Intentionally corrupt the checksum field of one page in a file.
+The server must be stopped for this to work reliably.
+
+The file name should be specified relative to the cluster datadir.
+page_offset had better be a multiple of the cluster's block size.
+
+=cut
+
+sub corrupt_page_checksum
+{
+   my ($self, $file, $page_offset) = @_;
+   my $pgdata = $self->data_dir;
+   my $pageheader;
+
+   open my $fh, '+<', "$pgdata/$file" or die "open($file) failed: $!";
+   binmode $fh;
+   sysseek($fh, $page_offset, 0) or die "sysseek failed: $!";
+   sysread($fh, $pageheader, 24) or die "sysread failed: $!";
+   # This inverts the pd_checksum field (only); see struct PageHeaderData
+   $pageheader ^= "\0\0\0\0\0\0\0\0\xff\xff";
+   sysseek($fh, $page_offset, 0) or die "sysseek failed: $!";
+   syswrite($fh, $pageheader) or die "syswrite failed: $!";
+   close $fh;
+
+   return;
+}
+
+=pod
+
 =back
 
 =cut