Hi,
Am Freitag, den 23.03.2018, 17:43 +0100 schrieb Michael Banck:
> Am Freitag, den 23.03.2018, 10:54 -0400 schrieb David Steele:
> > In my experience actual block errors are relatively rare, so there
> > aren't likely to be more than a few in a file. More common are
> > overwritten or transposed files, rogue files, etc. These produce a lot
> > of output.
> >
> > Maybe stop after five?
The attached patch does that, and outputs the total number of
verification failures of that file after it got sent.
> I'm on board with this, but I have the feeling that this is not a very
> common pattern in Postgres, or might not be project style at all. I
> can't remember even seen an error message like that.
>
> Anybody know whether we're doing this in a similar fashion elsewhere?
I tried to have look around and couldn't find any examples, so I'm not
sure that patch should go in. On the other hand, we abort on checksum
failures usually (in pg_dump e.g.), so limiting the number of warnings
does makes sense.
I guess we need to see what others think.
Michael
--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax: +49 2166 9901-100
Email: michael.ba...@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
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 8ea6682a45..1cb17636e6 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -1265,6 +1265,7 @@ sendFile(const char *readfilename, const char *tarfilename, struct stat *statbuf
BlockNumber blkno = 0;
char buf[TAR_SEND_SIZE];
uint16 checksum;
+ int checksum_failures = 0;
size_t cnt;
char *filename;
int i;
@@ -1353,13 +1354,21 @@ sendFile(const char *readfilename, const char *tarfilename, struct stat *statbuf
phdr = (PageHeader) page;
if (phdr->pd_checksum != checksum)
{
- ereport(WARNING,
- (errmsg("checksum verification failed in file "
- "\"%s\", block %d: calculated %X but "
- "expected %X",
- readfilename, blkno, checksum,
- phdr->pd_checksum)));
+ if (checksum_failures <= 5)
+ ereport(WARNING,
+ (errmsg("checksum verification failed in "
+ "file \"%s\", block %d: calculated "
+ "%X but expected %X",
+ readfilename, blkno, checksum,
+ phdr->pd_checksum)));
+ if (checksum_failures == 5)
+ ereport(WARNING,
+ (errmsg("further checksum verification "
+ "failures in file \"%s\" will not "
+ "be reported", readfilename)));
+
checksum_failure = true;
+ checksum_failures++;
}
}
blkno++;
@@ -1383,6 +1392,7 @@ sendFile(const char *readfilename, const char *tarfilename, struct stat *statbuf
*/
break;
}
+
}
/* If the file was truncated while we were sending it, pad it with zeros */
@@ -1411,6 +1421,11 @@ sendFile(const char *readfilename, const char *tarfilename, struct stat *statbuf
FreeFile(fp);
+ if (checksum_failures > 5)
+ ereport(WARNING,
+ (errmsg("file \"%s\" has a total of %d checksum verification "
+ "failures", readfilename, checksum_failures)));
+
return true;
}
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index d4c47f1377..7955747ae0 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -4,7 +4,7 @@ use Cwd;
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 84;
+use Test::More tests => 87;
program_help_ok('pg_basebackup');
program_version_ok('pg_basebackup');
@@ -337,7 +337,22 @@ $node->command_checks_all([ 'pg_basebackup', '-D', "$tempdir/backup_corrupt"],
'pg_basebackup reports checksum mismatch'
);
+# induce further corruption
+open $file, '+<', "$pgdata/$pg_class";
+for (my $offset = 4000 + 8*1024; $offset < 50000; $offset += 8*1024) {
+ seek($file, $offset, 0);
+ syswrite($file, '\0\0\0\0\0\0\0\0\0');
+}
+close $file;
+
+$node->command_checks_all([ 'pg_basebackup', '-D', "$tempdir/backup_corrupt2"],
+ 1,
+ [qr{^$}],
+ [qr/^WARNING.*further.*failures.will.not.be.reported/s],
+ 'pg_basebackup does not report more than 5 checksum mismatches'
+);
+
# do not verify checksums, should return ok
$node->command_ok(
- [ 'pg_basebackup', '-D', "$tempdir/backup_corrupt2", '-k' ],
+ [ 'pg_basebackup', '-D', "$tempdir/backup_corrupt3", '-k' ],
'pg_basebackup with -k does not report checksum mismatch');
