Skip to content

Commit e367114

Browse files
robertmhaasrobertmhaas
committed
pg_combinebackup: Error if incremental file exists in full backup.
Suppose that you run a command like "pg_combinebackup b1 b2 -o output", but both b1 and b2 contain an INCREMENTAL.$something file in a directory that is expected to contain relation files. This is an error, but the previous code would not detect the problem and instead write a garbage full file named $something to the output directory. This commit adds code to detect the error and a test case to verify the behavior. It's difficult to imagine that this will ever happen unless someone is intentionally trying to break incremental backup, but per discussion, let's consider that the lack of adequate sanity checking in this area is a bug and back-patch to v17, where incremental backup was introduced. Patch by me, reviewed by Bertrand Drouvot and Amul Sul. Discussion: http://postgr.es/m/CA+TgmoaD7dBYPqe7kMtO0dyto7rd0rUh7joh=JPUSaFszKY6Pg@mail.gmail.com
1 parent 0d635b6 commit e367114

File tree

3 files changed

+75
-0
lines changed

3 files changed

+75
-0
lines changed

‎src/bin/pg_combinebackup/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ tests += {
3636
't/006_db_file_copy.pl',
3737
't/007_wal_level_minimal.pl',
3838
't/008_promote.pl',
39+
't/009_no_full_file.pl',
3940
],
4041
}
4142
}

‎src/bin/pg_combinebackup/reconstruct.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,19 @@ reconstruct_from_incremental_file(char *input_filename,
326326
* result, then forget about performing reconstruction and just copy that
327327
* file in its entirety.
328328
*
329+
* If we have only incremental files, and there's no full file at any
330+
* point in the backup chain, something has gone wrong. Emit an error.
331+
*
329332
* Otherwise, reconstruct.
330333
*/
331334
if (copy_source != NULL)
332335
copy_file(copy_source->filename, output_filename,
333336
&checksum_ctx, copy_method, dry_run);
337+
else if (sidx == 0 && source[0]->header_length != 0)
338+
{
339+
pg_fatal("full backup contains unexpected incremental file \"%s\"",
340+
source[0]->filename);
341+
}
334342
else
335343
{
336344
write_reconstructed_file(input_filename, output_filename,

‎src/bin/pg_combinebackup/t/009_no_full_file.pl

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
# Copyright (c) 2021-2024, PostgreSQL Global Development Group
2+
3+
use strict;
4+
use warnings FATAL => 'all';
5+
use File::Copy;
6+
use PostgreSQL::Test::Cluster;
7+
use PostgreSQL::Test::Utils;
8+
use Test::More;
9+
10+
# Can be changed to test the other modes.
11+
my $mode = $ENV{PG_TEST_PG_COMBINEBACKUP_MODE} || '--copy';
12+
13+
note "testing using mode $mode";
14+
15+
# Set up a new database instance.
16+
my $primary = PostgreSQL::Test::Cluster->new('primary');
17+
$primary->init(has_archiving => 1, allows_streaming => 1);
18+
$primary->append_conf('postgresql.conf', 'summarize_wal = on');
19+
$primary->start;
20+
21+
# Take a full backup.
22+
my $backup1path = $primary->backup_dir . '/backup1';
23+
$primary->command_ok(
24+
[ 'pg_basebackup', '-D', $backup1path, '--no-sync', '-cfast' ],
25+
"full backup");
26+
27+
# Take an incremental backup.
28+
my $backup2path = $primary->backup_dir . '/backup2';
29+
$primary->command_ok(
30+
[
31+
'pg_basebackup', '-D', $backup2path, '--no-sync', '-cfast',
32+
'--incremental', $backup1path . '/backup_manifest'
33+
],
34+
"incremental backup");
35+
36+
# Find an incremental file in the incremental backup for which there is a full
37+
# file in the full backup. When we find one, replace the full file with an
38+
# incremental file.
39+
my @filelist = grep { /^INCREMENTAL\./ } slurp_dir("$backup2path/base/1");
40+
my $success = 0;
41+
for my $iname (@filelist)
42+
{
43+
my $name = $iname;
44+
$name =~ s/^INCREMENTAL.//;
45+
46+
if (-f "$backup1path/base/1/$name")
47+
{
48+
copy("$backup2path/base/1/$iname", "$backup1path/base/1/$iname")
49+
|| die "copy $backup2path/base/1/$iname: $!";
50+
unlink("$backup1path/base/1/$name")
51+
|| die "unlink $backup1path/base/1/$name: $!";
52+
$success = 1;
53+
last;
54+
}
55+
}
56+
57+
# pg_combinebackup should fail.
58+
my $outpath = $primary->backup_dir . '/out';
59+
$primary->command_fails_like(
60+
[
61+
'pg_combinebackup', $backup1path, $backup2path, '-o', $outpath,
62+
],
63+
qr/full backup contains unexpected incremental file/,
64+
"pg_combinebackup fails");
65+
66+
done_testing();

0 commit comments

Comments
 (0)