Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit2041bc4

Browse files
committed
Add the system identifier to backup manifests.
Before this patch, if you took a full backup on server A and thentried to use the backup manifest to take an incremental backup onserver B, it wouldn't know that the manifest was from a differentserver and so the incremental backup operation could potentiallycomplete without error. When you later tried to run pg_combinebackup,you'd find out that your incremental backup was and always had beeninvalid. That's poor timing, because nobody likes finding out aboutbackup problems only at restore time.With this patch, you'll get an error when trying to take the (invalid)incremental backup, which seems a lot nicer.Amul Sul, revised by me. Review by Michael Paquier.Discussion:http://postgr.es/m/CA+TgmoYLZzbSAMM3cAjV4Y+iCRZn-bR9H2+Mdz7NdaJFU1Zb5w@mail.gmail.com
1 parent1ee910c commit2041bc4

File tree

16 files changed

+351
-23
lines changed

16 files changed

+351
-23
lines changed

‎doc/src/sgml/backup-manifest.sgml‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,22 @@
3737
<term><literal>PostgreSQL-Backup-Manifest-Version</literal></term>
3838
<listitem>
3939
<para>
40-
The associated value is always the integer 1.
40+
The associated value is an integer. Beginning in
41+
<productname>PostgreSQL</productname> <literal>17</literal>,
42+
it is <literal>2</literal>; in older versions, it is <literal>1</literal>.
43+
</para>
44+
</listitem>
45+
</varlistentry>
46+
47+
<varlistentry>
48+
<term><literal>System-Identifier</literal></term>
49+
<listitem>
50+
<para>
51+
The database system identifier of the
52+
<productname>PostgreSQL</productname> instance where the backup was
53+
taken. This field is present only when
54+
<literal>PostgreSQL-Backup-Manifest-Version</literal> is
55+
<literal>2</literal>.
4156
</para>
4257
</listitem>
4358
</varlistentry>

‎doc/src/sgml/ref/pg_verifybackup.sgml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,10 @@ PostgreSQL documentation
5353
Backup verification proceeds in four stages. First,
5454
<literal>pg_verifybackup</literal> reads the
5555
<literal>backup_manifest</literal> file. If that file
56-
does not exist, cannot be read, is malformed, or fails verification
57-
against its own internal checksum, <literal>pg_verifybackup</literal>
58-
will terminate with a fatal error.
56+
does not exist, cannot be read, is malformed, fails to match the system
57+
identifier with <filename>pg_control</filename> of the backup directory or
58+
fails verification against its own internal checksum,
59+
<literal>pg_verifybackup</literal> will terminate with a fatal error.
5960
</para>
6061

6162
<para>

‎src/backend/backup/backup_manifest.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include"postgres.h"
1414

1515
#include"access/timeline.h"
16+
#include"access/xlog.h"
1617
#include"backup/backup_manifest.h"
1718
#include"backup/basebackup_sink.h"
1819
#include"mb/pg_wchar.h"
@@ -77,8 +78,10 @@ InitializeBackupManifest(backup_manifest_info *manifest,
7778

7879
if (want_manifest!=MANIFEST_OPTION_NO)
7980
AppendToManifest(manifest,
80-
"{ \"PostgreSQL-Backup-Manifest-Version\": 1,\n"
81-
"\"Files\": [");
81+
"{ \"PostgreSQL-Backup-Manifest-Version\": 2,\n"
82+
"\"System-Identifier\": "UINT64_FORMAT",\n"
83+
"\"Files\": [",
84+
GetSystemIdentifier());
8285
}
8386

8487
/*

‎src/backend/backup/basebackup_incremental.c‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include"postgres.h"
2121

2222
#include"access/timeline.h"
23+
#include"access/xlog.h"
2324
#include"backup/basebackup_incremental.h"
2425
#include"backup/walsummary.h"
2526
#include"common/blkreftable.h"
@@ -113,6 +114,10 @@ struct IncrementalBackupInfo
113114
BlockRefTable*brtab;
114115
};
115116

117+
staticvoidmanifest_process_version(JsonManifestParseContext*context,
118+
intmanifest_version);
119+
staticvoidmanifest_process_system_identifier(JsonManifestParseContext*context,
120+
uint64manifest_system_identifier);
116121
staticvoidmanifest_process_file(JsonManifestParseContext*context,
117122
char*pathname,
118123
size_tsize,
@@ -199,6 +204,8 @@ FinalizeIncrementalManifest(IncrementalBackupInfo *ib)
199204

200205
/* Parse the manifest. */
201206
context.private_data=ib;
207+
context.version_cb=manifest_process_version;
208+
context.system_identifier_cb=manifest_process_system_identifier;
202209
context.per_file_cb=manifest_process_file;
203210
context.per_wal_range_cb=manifest_process_wal_range;
204211
context.error_cb=manifest_report_error;
@@ -927,6 +934,39 @@ hash_string_pointer(const char *s)
927934
returnhash_bytes(ss,strlen(s));
928935
}
929936

937+
/*
938+
* This callback to validate the manifest version for incremental backup.
939+
*/
940+
staticvoid
941+
manifest_process_version(JsonManifestParseContext*context,
942+
intmanifest_version)
943+
{
944+
/* Incremental backups don't work with manifest version 1 */
945+
if (manifest_version==1)
946+
context->error_cb(context,
947+
"backup manifest version 1 does not support incremental backup");
948+
}
949+
950+
/*
951+
* This callback to validate the manifest system identifier against the current
952+
* database server.
953+
*/
954+
staticvoid
955+
manifest_process_system_identifier(JsonManifestParseContext*context,
956+
uint64manifest_system_identifier)
957+
{
958+
uint64system_identifier;
959+
960+
/* Get system identifier of current system */
961+
system_identifier=GetSystemIdentifier();
962+
963+
if (manifest_system_identifier!=system_identifier)
964+
context->error_cb(context,
965+
"manifest system identifier is %llu, but database system identifier is %llu",
966+
(unsigned long long)manifest_system_identifier,
967+
(unsigned long long)system_identifier);
968+
}
969+
930970
/*
931971
* This callback is invoked for each file mentioned in the backup manifest.
932972
*

‎src/bin/pg_basebackup/t/010_pg_basebackup.pl‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -965,4 +965,20 @@
965965
my@dst_tblspc =glob"$backupdir/pg_tblspc/$tblspc_oid/PG_*";
966966
is(@dst_tblspc, 1,'tblspc directory copied');
967967

968+
# Can't take backup with referring manifest of different cluster
969+
#
970+
# Set up another new database instance with force initdb option. We don't want
971+
# to initializing database system by copying initdb template for this, because
972+
# we want it to be a separate cluster with a different system ID.
973+
my$node2 = PostgreSQL::Test::Cluster->new('node2');
974+
$node2->init(force_initdb=> 1,has_archiving=> 1,allows_streaming=> 1);
975+
$node2->append_conf('postgresql.conf','summarize_wal = on');
976+
$node2->start;
977+
978+
$node2->command_fails_like(
979+
[@pg_basebackup_defs,'-D',"$tempdir" .'/diff_sysid',
980+
'--incremental',"$backupdir" .'/backup_manifest' ],
981+
qr/manifest system identifier is .*, but database system identifier is/,
982+
"pg_basebackup fails with different database system manifest");
983+
968984
done_testing();

‎src/bin/pg_combinebackup/load_manifest.c‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ static uint32 hash_string_pointer(char *s);
5050
#defineSH_DEFINE
5151
#include"lib/simplehash.h"
5252

53+
staticvoidcombinebackup_version_cb(JsonManifestParseContext*context,
54+
intmanifest_version);
55+
staticvoidcombinebackup_system_identifier_cb(JsonManifestParseContext*context,
56+
uint64manifest_system_identifier);
5357
staticvoidcombinebackup_per_file_cb(JsonManifestParseContext*context,
5458
char*pathname,size_tsize,
5559
pg_checksum_typechecksum_type,
@@ -153,6 +157,8 @@ load_backup_manifest(char *backup_directory)
153157
result=pg_malloc0(sizeof(manifest_data));
154158
result->files=ht;
155159
context.private_data=result;
160+
context.version_cb=combinebackup_version_cb;
161+
context.system_identifier_cb=combinebackup_system_identifier_cb;
156162
context.per_file_cb=combinebackup_per_file_cb;
157163
context.per_wal_range_cb=combinebackup_per_wal_range_cb;
158164
context.error_cb=report_manifest_error;
@@ -181,6 +187,31 @@ report_manifest_error(JsonManifestParseContext *context, const char *fmt,...)
181187
exit(1);
182188
}
183189

190+
/*
191+
* This callback to validate the manifest version number for incremental backup.
192+
*/
193+
staticvoid
194+
combinebackup_version_cb(JsonManifestParseContext*context,
195+
intmanifest_version)
196+
{
197+
/* Incremental backups supported on manifest version 2 or later */
198+
if (manifest_version==1)
199+
pg_fatal("backup manifest version 1 does not support incremental backup");
200+
}
201+
202+
/*
203+
* Record system identifier extracted from the backup manifest.
204+
*/
205+
staticvoid
206+
combinebackup_system_identifier_cb(JsonManifestParseContext*context,
207+
uint64manifest_system_identifier)
208+
{
209+
manifest_data*manifest=context->private_data;
210+
211+
/* Validation will be at the later stage */
212+
manifest->system_identifier=manifest_system_identifier;
213+
}
214+
184215
/*
185216
* Record details extracted from the backup manifest for one file.
186217
*/

‎src/bin/pg_combinebackup/load_manifest.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ typedef struct manifest_wal_range
5555
*/
5656
typedefstructmanifest_data
5757
{
58+
uint64system_identifier;
5859
manifest_files_hash*files;
5960
manifest_wal_range*first_wal_range;
6061
manifest_wal_range*last_wal_range;

‎src/bin/pg_combinebackup/pg_combinebackup.c‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ cb_cleanup_dir *cleanup_dir_list = NULL;
9292

9393
staticvoidadd_tablespace_mapping(cb_options*opt,char*arg);
9494
staticStringInfocheck_backup_label_files(intn_backups,char**backup_dirs);
95-
staticvoidcheck_control_files(intn_backups,char**backup_dirs);
95+
staticuint64check_control_files(intn_backups,char**backup_dirs);
9696
staticvoidcheck_input_dir_permissions(char*dir);
9797
staticvoidcleanup_directories_atexit(void);
9898
staticvoidcreate_output_directory(char*dirname,cb_options*opt);
@@ -134,11 +134,13 @@ main(int argc, char *argv[])
134134

135135
constchar*progname;
136136
char*last_input_dir;
137+
inti;
137138
intoptindex;
138139
intc;
139140
intn_backups;
140141
intn_prior_backups;
141142
intversion;
143+
uint64system_identifier;
142144
char**prior_backup_dirs;
143145
cb_optionsopt;
144146
cb_tablespace*tablespaces;
@@ -216,7 +218,7 @@ main(int argc, char *argv[])
216218

217219
/* Sanity-check control files. */
218220
n_backups=argc-optind;
219-
check_control_files(n_backups,argv+optind);
221+
system_identifier=check_control_files(n_backups,argv+optind);
220222

221223
/* Sanity-check backup_label files, and get the contents of the last one. */
222224
last_backup_label=check_backup_label_files(n_backups,argv+optind);
@@ -231,6 +233,26 @@ main(int argc, char *argv[])
231233
/* Load backup manifests. */
232234
manifests=load_backup_manifests(n_backups,prior_backup_dirs);
233235

236+
/*
237+
* Validate the manifest system identifier against the backup system
238+
* identifier.
239+
*/
240+
for (i=0;i<n_backups;i++)
241+
{
242+
if (manifests[i]&&
243+
manifests[i]->system_identifier!=system_identifier)
244+
{
245+
char*controlpath;
246+
247+
controlpath=psprintf("%s/%s",prior_backup_dirs[i],"global/pg_control");
248+
249+
pg_fatal("%s: manifest system identifier is %llu, but control file has %llu",
250+
controlpath,
251+
(unsigned long long)manifests[i]->system_identifier,
252+
(unsigned long long)system_identifier);
253+
}
254+
}
255+
234256
/* Figure out which tablespaces are going to be included in the output. */
235257
last_input_dir=argv[argc-1];
236258
check_input_dir_permissions(last_input_dir);
@@ -256,7 +278,7 @@ main(int argc, char *argv[])
256278
/* If we need to write a backup_manifest, prepare to do so. */
257279
if (!opt.dry_run&& !opt.no_manifest)
258280
{
259-
mwriter=create_manifest_writer(opt.output);
281+
mwriter=create_manifest_writer(opt.output,system_identifier);
260282

261283
/*
262284
* Verify that we have a backup manifest for the final backup; else we
@@ -517,9 +539,9 @@ check_backup_label_files(int n_backups, char **backup_dirs)
517539
}
518540

519541
/*
520-
* Sanity check control files.
542+
* Sanity check control files and return system_identifier.
521543
*/
522-
staticvoid
544+
staticuint64
523545
check_control_files(intn_backups,char**backup_dirs)
524546
{
525547
inti;
@@ -564,6 +586,8 @@ check_control_files(int n_backups, char **backup_dirs)
564586
*/
565587
pg_log_debug("system identifier is %llu",
566588
(unsigned long long)system_identifier);
589+
590+
returnsystem_identifier;
567591
}
568592

569593
/*

‎src/bin/pg_combinebackup/t/005_integrity.pl‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use warningsFATAL=>'all';
99
use File::Compare;
1010
use File::Pathqw(rmtree);
11+
use File::Copy;
1112
use PostgreSQL::Test::Cluster;
1213
use PostgreSQL::Test::Utils;
1314
use Test::More;
@@ -79,6 +80,19 @@
7980
qr/expected system identifier.*but found/,
8081
"can't combine backups from different nodes");
8182

83+
# Can't combine when different manifest system identifier
84+
rename("$backup2path/backup_manifest","$backup2path/backup_manifest.orig")
85+
ordie"could not move$backup2path/backup_manifest";
86+
copy("$backupother2path/backup_manifest","$backup2path/backup_manifest")
87+
ordie"could not copy$backupother2path/backup_manifest";
88+
$node1->command_fails_like(
89+
['pg_combinebackup',$backup1path,$backup2path,$backup3path,'-o',$resultpath ],
90+
qr/ manifest system identifier is .*, but control file has/,
91+
"can't combine backups with different manifest system identifier");
92+
# Restore the backup state
93+
move("$backup2path/backup_manifest.orig","$backup2path/backup_manifest")
94+
ordie"could not move$backup2path/backup_manifest";
95+
8296
# Can't omit a required backup.
8397
$node1->command_fails_like(
8498
['pg_combinebackup',$backup1path,$backup3path,'-o',$resultpath ],

‎src/bin/pg_combinebackup/write_manifest.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ static size_t hex_encode(const uint8 *src, size_t len, char *dst);
4545
* in the specified directory.
4646
*/
4747
manifest_writer*
48-
create_manifest_writer(char*directory)
48+
create_manifest_writer(char*directory,uint64system_identifier)
4949
{
5050
manifest_writer*mwriter=pg_malloc(sizeof(manifest_writer));
5151

@@ -57,8 +57,10 @@ create_manifest_writer(char *directory)
5757
pg_checksum_init(&mwriter->manifest_ctx,CHECKSUM_TYPE_SHA256);
5858

5959
appendStringInfo(&mwriter->buf,
60-
"{ \"PostgreSQL-Backup-Manifest-Version\": 1,\n"
61-
"\"Files\": [");
60+
"{ \"PostgreSQL-Backup-Manifest-Version\": 2,\n"
61+
"\"System-Identifier\": "UINT64_FORMAT",\n"
62+
"\"Files\": [",
63+
system_identifier);
6264

6365
returnmwriter;
6466
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp