pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[Patch] checksumming-related buglets in pg_verify_checksums/pg_basebackup TAP tests
2+ messages / 2 participants
[nested] [flat]
* [Patch] checksumming-related buglets in pg_verify_checksums/pg_basebackup TAP tests
@ 2019-02-14 14:07 Michael Banck <michael.banck@credativ.de>
2019-02-18 05:25 ` Re: [Patch] checksumming-related buglets in pg_verify_checksums/pg_basebackup TAP tests Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 2+ messages in thread
From: Michael Banck @ 2019-02-14 14:07 UTC (permalink / raw)
To: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>; +Cc: Michael Paquier <michael@paquier.xyz>; Magnus Hagander <magnus@hagander.net>
Hi,
while hacking on pg_verify_checksums and looking at hexdumps, I noticed
that the TAP tests of pg_verify_checksums (and pg_basebackup from which
it was copy-pasted) actually write "305c305c[...]" (i.e. literal
backslashes and number 0s) instead of "000[...]" into the to-be-
corrupted relfilenodes due to wrong quoting.
This also revealed a second bug in the pg_basebackup test suite where
the offset for the corruption in the second file was wrong, so it
actually never got corrupted, and the tests only passed due to the above
twice than expected number of written bytes. The write() probably
overflowed into an adjacent block so that the total number of corrupted
blocks was as expected by addident. Oops, my bad, patch attached.
Michael
--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax: +49 2166 9901-100
Email: michael.banck@credativ.de
credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz
Attachments:
[text/x-patch] taptest_checksum_fixes.patch (1.8K, ../../1550153276.796.35.camel@credativ.de/2-taptest_checksum_fixes.patch)
download | inline diff:
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 3e1c3863c4..33869fecc9 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -499,7 +499,7 @@ my $block_size = $node->safe_psql('postgres', 'SHOW block_size;');
system_or_bail 'pg_ctl', '-D', $pgdata, 'stop';
open $file, '+<', "$pgdata/$file_corrupt1";
seek($file, $pageheader_size, 0);
-syswrite($file, '\0\0\0\0\0\0\0\0\0');
+syswrite($file, "\0\0\0\0\0\0\0\0\0");
close $file;
system_or_bail 'pg_ctl', '-D', $pgdata, 'start';
@@ -518,7 +518,7 @@ for my $i (1 .. 5)
{
my $offset = $pageheader_size + $i * $block_size;
seek($file, $offset, 0);
- syswrite($file, '\0\0\0\0\0\0\0\0\0');
+ syswrite($file, "\0\0\0\0\0\0\0\0\0");
}
close $file;
system_or_bail 'pg_ctl', '-D', $pgdata, 'start';
@@ -534,8 +534,8 @@ 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, 4000, 0);
-syswrite($file, '\0\0\0\0\0\0\0\0\0');
+seek($file, $pageheader_size, 0);
+syswrite($file, "\0\0\0\0\0\0\0\0\0");
close $file;
system_or_bail 'pg_ctl', '-D', $pgdata, 'start';
diff --git a/src/bin/pg_verify_checksums/t/002_actions.pl b/src/bin/pg_verify_checksums/t/002_actions.pl
index 5250b5a728..74ad5ad723 100644
--- a/src/bin/pg_verify_checksums/t/002_actions.pl
+++ b/src/bin/pg_verify_checksums/t/002_actions.pl
@@ -45,7 +45,7 @@ sub check_relation_corruption
# Time to create some corruption
open my $file, '+<', "$pgdata/$file_corrupted";
seek($file, $pageheader_size, 0);
- syswrite($file, '\0\0\0\0\0\0\0\0\0');
+ syswrite($file, "\0\0\0\0\0\0\0\0\0");
close $file;
# Checksum checks on single relfilenode fail
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: [Patch] checksumming-related buglets in pg_verify_checksums/pg_basebackup TAP tests
2019-02-14 14:07 [Patch] checksumming-related buglets in pg_verify_checksums/pg_basebackup TAP tests Michael Banck <michael.banck@credativ.de>
@ 2019-02-18 05:25 ` Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 2+ messages in thread
From: Michael Paquier @ 2019-02-18 05:25 UTC (permalink / raw)
To: Michael Banck <michael.banck@credativ.de>; +Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>; Magnus Hagander <magnus@hagander.net>
On Thu, Feb 14, 2019 at 03:07:56PM +0100, Michael Banck wrote:
> This also revealed a second bug in the pg_basebackup test suite where
> the offset for the corruption in the second file was wrong, so it
> actually never got corrupted, and the tests only passed due to the above
> twice than expected number of written bytes. The write() probably
> overflowed into an adjacent block so that the total number of corrupted
> blocks was as expected by accident. Oops, my bad, patch attached.
Fixed and back-patched where adapted, thanks! ee9e145 was a first
shot for a fix in pg_basebackup tests, but it has missed one seek()
call.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../20190218052503.GL15532@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2019-02-18 05:25 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-02-14 14:07 [Patch] checksumming-related buglets in pg_verify_checksums/pg_basebackup TAP tests Michael Banck <michael.banck@credativ.de>
2019-02-18 05:25 ` Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox