From 2041bc4276c95ac014510032e622a4baf70b29f1 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 13 Mar 2024 15:04:22 -0400 Subject: [PATCH] Add the system identifier to backup manifests. Before this patch, if you took a full backup on server A and then tried to use the backup manifest to take an incremental backup on server B, it wouldn't know that the manifest was from a different server and so the incremental backup operation could potentially complete without error. When you later tried to run pg_combinebackup, you'd find out that your incremental backup was and always had been invalid. That's poor timing, because nobody likes finding out about backup 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 --- doc/src/sgml/backup-manifest.sgml | 17 +++- doc/src/sgml/ref/pg_verifybackup.sgml | 7 +- src/backend/backup/backup_manifest.c | 7 +- src/backend/backup/basebackup_incremental.c | 40 +++++++++ src/bin/pg_basebackup/t/010_pg_basebackup.pl | 16 ++++ src/bin/pg_combinebackup/load_manifest.c | 31 +++++++ src/bin/pg_combinebackup/load_manifest.h | 1 + src/bin/pg_combinebackup/pg_combinebackup.c | 34 ++++++-- src/bin/pg_combinebackup/t/005_integrity.pl | 14 ++++ src/bin/pg_combinebackup/write_manifest.c | 8 +- src/bin/pg_combinebackup/write_manifest.h | 3 +- src/bin/pg_verifybackup/pg_verifybackup.c | 82 ++++++++++++++++++- src/bin/pg_verifybackup/t/003_corruption.pl | 26 +++++- src/bin/pg_verifybackup/t/005_bad_manifest.pl | 6 +- src/common/parse_manifest.c | 76 ++++++++++++++++- src/include/common/parse_manifest.h | 6 ++ 16 files changed, 351 insertions(+), 23 deletions(-) diff --git a/doc/src/sgml/backup-manifest.sgml b/doc/src/sgml/backup-manifest.sgml index 771be1310a..d5ec244834 100644 --- a/doc/src/sgml/backup-manifest.sgml +++ b/doc/src/sgml/backup-manifest.sgml @@ -37,7 +37,22 @@ PostgreSQL-Backup-Manifest-Version - The associated value is always the integer 1. + The associated value is an integer. Beginning in + PostgreSQL 17, + it is 2; in older versions, it is 1. + + + + + + System-Identifier + + + The database system identifier of the + PostgreSQL instance where the backup was + taken. This field is present only when + PostgreSQL-Backup-Manifest-Version is + 2. diff --git a/doc/src/sgml/ref/pg_verifybackup.sgml b/doc/src/sgml/ref/pg_verifybackup.sgml index 36335e0a18..a3f167f9f6 100644 --- a/doc/src/sgml/ref/pg_verifybackup.sgml +++ b/doc/src/sgml/ref/pg_verifybackup.sgml @@ -53,9 +53,10 @@ PostgreSQL documentation Backup verification proceeds in four stages. First, pg_verifybackup reads the backup_manifest file. If that file - does not exist, cannot be read, is malformed, or fails verification - against its own internal checksum, pg_verifybackup - will terminate with a fatal error. + does not exist, cannot be read, is malformed, fails to match the system + identifier with pg_control of the backup directory or + fails verification against its own internal checksum, + pg_verifybackup will terminate with a fatal error. diff --git a/src/backend/backup/backup_manifest.c b/src/backend/backup/backup_manifest.c index 9c14f18401..b360a13547 100644 --- a/src/backend/backup/backup_manifest.c +++ b/src/backend/backup/backup_manifest.c @@ -13,6 +13,7 @@ #include "postgres.h" #include "access/timeline.h" +#include "access/xlog.h" #include "backup/backup_manifest.h" #include "backup/basebackup_sink.h" #include "mb/pg_wchar.h" @@ -77,8 +78,10 @@ InitializeBackupManifest(backup_manifest_info *manifest, if (want_manifest != MANIFEST_OPTION_NO) AppendToManifest(manifest, - "{ \"PostgreSQL-Backup-Manifest-Version\": 1,\n" - "\"Files\": ["); + "{ \"PostgreSQL-Backup-Manifest-Version\": 2,\n" + "\"System-Identifier\": " UINT64_FORMAT ",\n" + "\"Files\": [", + GetSystemIdentifier()); } /* diff --git a/src/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c index 2a522e4aea..990b2872ea 100644 --- a/src/backend/backup/basebackup_incremental.c +++ b/src/backend/backup/basebackup_incremental.c @@ -20,6 +20,7 @@ #include "postgres.h" #include "access/timeline.h" +#include "access/xlog.h" #include "backup/basebackup_incremental.h" #include "backup/walsummary.h" #include "common/blkreftable.h" @@ -113,6 +114,10 @@ struct IncrementalBackupInfo BlockRefTable *brtab; }; +static void manifest_process_version(JsonManifestParseContext *context, + int manifest_version); +static void manifest_process_system_identifier(JsonManifestParseContext *context, + uint64 manifest_system_identifier); static void manifest_process_file(JsonManifestParseContext *context, char *pathname, size_t size, @@ -199,6 +204,8 @@ FinalizeIncrementalManifest(IncrementalBackupInfo *ib) /* Parse the manifest. */ context.private_data = ib; + context.version_cb = manifest_process_version; + context.system_identifier_cb = manifest_process_system_identifier; context.per_file_cb = manifest_process_file; context.per_wal_range_cb = manifest_process_wal_range; context.error_cb = manifest_report_error; @@ -927,6 +934,39 @@ hash_string_pointer(const char *s) return hash_bytes(ss, strlen(s)); } +/* + * This callback to validate the manifest version for incremental backup. + */ +static void +manifest_process_version(JsonManifestParseContext *context, + int manifest_version) +{ + /* Incremental backups don't work with manifest version 1 */ + if (manifest_version == 1) + context->error_cb(context, + "backup manifest version 1 does not support incremental backup"); +} + +/* + * This callback to validate the manifest system identifier against the current + * database server. + */ +static void +manifest_process_system_identifier(JsonManifestParseContext *context, + uint64 manifest_system_identifier) +{ + uint64 system_identifier; + + /* Get system identifier of current system */ + system_identifier = GetSystemIdentifier(); + + if (manifest_system_identifier != system_identifier) + context->error_cb(context, + "manifest system identifier is %llu, but database system identifier is %llu", + (unsigned long long) manifest_system_identifier, + (unsigned long long) system_identifier); +} + /* * This callback is invoked for each file mentioned in the backup manifest. * diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index 977ced71f8..d3c83f26e4 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -965,4 +965,20 @@ $backupdir = $node->backup_dir . '/backup3'; my @dst_tblspc = glob "$backupdir/pg_tblspc/$tblspc_oid/PG_*"; is(@dst_tblspc, 1, 'tblspc directory copied'); +# Can't take backup with referring manifest of different cluster +# +# Set up another new database instance with force initdb option. We don't want +# to initializing database system by copying initdb template for this, because +# we want it to be a separate cluster with a different system ID. +my $node2 = PostgreSQL::Test::Cluster->new('node2'); +$node2->init(force_initdb => 1, has_archiving => 1, allows_streaming => 1); +$node2->append_conf('postgresql.conf', 'summarize_wal = on'); +$node2->start; + +$node2->command_fails_like( + [ @pg_basebackup_defs, '-D', "$tempdir" . '/diff_sysid', + '--incremental', "$backupdir" . '/backup_manifest' ], + qr/manifest system identifier is .*, but database system identifier is/, + "pg_basebackup fails with different database system manifest"); + done_testing(); diff --git a/src/bin/pg_combinebackup/load_manifest.c b/src/bin/pg_combinebackup/load_manifest.c index 2b8e74fcf3..7bc10fbe10 100644 --- a/src/bin/pg_combinebackup/load_manifest.c +++ b/src/bin/pg_combinebackup/load_manifest.c @@ -50,6 +50,10 @@ static uint32 hash_string_pointer(char *s); #define SH_DEFINE #include "lib/simplehash.h" +static void combinebackup_version_cb(JsonManifestParseContext *context, + int manifest_version); +static void combinebackup_system_identifier_cb(JsonManifestParseContext *context, + uint64 manifest_system_identifier); static void combinebackup_per_file_cb(JsonManifestParseContext *context, char *pathname, size_t size, pg_checksum_type checksum_type, @@ -153,6 +157,8 @@ load_backup_manifest(char *backup_directory) result = pg_malloc0(sizeof(manifest_data)); result->files = ht; context.private_data = result; + context.version_cb = combinebackup_version_cb; + context.system_identifier_cb = combinebackup_system_identifier_cb; context.per_file_cb = combinebackup_per_file_cb; context.per_wal_range_cb = combinebackup_per_wal_range_cb; context.error_cb = report_manifest_error; @@ -181,6 +187,31 @@ report_manifest_error(JsonManifestParseContext *context, const char *fmt,...) exit(1); } +/* + * This callback to validate the manifest version number for incremental backup. + */ +static void +combinebackup_version_cb(JsonManifestParseContext *context, + int manifest_version) +{ + /* Incremental backups supported on manifest version 2 or later */ + if (manifest_version == 1) + pg_fatal("backup manifest version 1 does not support incremental backup"); +} + +/* + * Record system identifier extracted from the backup manifest. + */ +static void +combinebackup_system_identifier_cb(JsonManifestParseContext *context, + uint64 manifest_system_identifier) +{ + manifest_data *manifest = context->private_data; + + /* Validation will be at the later stage */ + manifest->system_identifier = manifest_system_identifier; +} + /* * Record details extracted from the backup manifest for one file. */ diff --git a/src/bin/pg_combinebackup/load_manifest.h b/src/bin/pg_combinebackup/load_manifest.h index 9163e071af..8a5a70e447 100644 --- a/src/bin/pg_combinebackup/load_manifest.h +++ b/src/bin/pg_combinebackup/load_manifest.h @@ -55,6 +55,7 @@ typedef struct manifest_wal_range */ typedef struct manifest_data { + uint64 system_identifier; manifest_files_hash *files; manifest_wal_range *first_wal_range; manifest_wal_range *last_wal_range; diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c index 4197cfeade..6f0814d9ac 100644 --- a/src/bin/pg_combinebackup/pg_combinebackup.c +++ b/src/bin/pg_combinebackup/pg_combinebackup.c @@ -92,7 +92,7 @@ cb_cleanup_dir *cleanup_dir_list = NULL; static void add_tablespace_mapping(cb_options *opt, char *arg); static StringInfo check_backup_label_files(int n_backups, char **backup_dirs); -static void check_control_files(int n_backups, char **backup_dirs); +static uint64 check_control_files(int n_backups, char **backup_dirs); static void check_input_dir_permissions(char *dir); static void cleanup_directories_atexit(void); static void create_output_directory(char *dirname, cb_options *opt); @@ -134,11 +134,13 @@ main(int argc, char *argv[]) const char *progname; char *last_input_dir; + int i; int optindex; int c; int n_backups; int n_prior_backups; int version; + uint64 system_identifier; char **prior_backup_dirs; cb_options opt; cb_tablespace *tablespaces; @@ -216,7 +218,7 @@ main(int argc, char *argv[]) /* Sanity-check control files. */ n_backups = argc - optind; - check_control_files(n_backups, argv + optind); + system_identifier = check_control_files(n_backups, argv + optind); /* Sanity-check backup_label files, and get the contents of the last one. */ last_backup_label = check_backup_label_files(n_backups, argv + optind); @@ -231,6 +233,26 @@ main(int argc, char *argv[]) /* Load backup manifests. */ manifests = load_backup_manifests(n_backups, prior_backup_dirs); + /* + * Validate the manifest system identifier against the backup system + * identifier. + */ + for (i = 0; i < n_backups; i++) + { + if (manifests[i] && + manifests[i]->system_identifier != system_identifier) + { + char *controlpath; + + controlpath = psprintf("%s/%s", prior_backup_dirs[i], "global/pg_control"); + + pg_fatal("%s: manifest system identifier is %llu, but control file has %llu", + controlpath, + (unsigned long long) manifests[i]->system_identifier, + (unsigned long long) system_identifier); + } + } + /* Figure out which tablespaces are going to be included in the output. */ last_input_dir = argv[argc - 1]; check_input_dir_permissions(last_input_dir); @@ -256,7 +278,7 @@ main(int argc, char *argv[]) /* If we need to write a backup_manifest, prepare to do so. */ if (!opt.dry_run && !opt.no_manifest) { - mwriter = create_manifest_writer(opt.output); + mwriter = create_manifest_writer(opt.output, system_identifier); /* * 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) } /* - * Sanity check control files. + * Sanity check control files and return system_identifier. */ -static void +static uint64 check_control_files(int n_backups, char **backup_dirs) { int i; @@ -564,6 +586,8 @@ check_control_files(int n_backups, char **backup_dirs) */ pg_log_debug("system identifier is %llu", (unsigned long long) system_identifier); + + return system_identifier; } /* diff --git a/src/bin/pg_combinebackup/t/005_integrity.pl b/src/bin/pg_combinebackup/t/005_integrity.pl index 5dc71ddcf8..c3f1a2a6f4 100644 --- a/src/bin/pg_combinebackup/t/005_integrity.pl +++ b/src/bin/pg_combinebackup/t/005_integrity.pl @@ -8,6 +8,7 @@ use strict; use warnings FATAL => 'all'; use File::Compare; use File::Path qw(rmtree); +use File::Copy; use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; @@ -79,6 +80,19 @@ $node1->command_fails_like( qr/expected system identifier.*but found/, "can't combine backups from different nodes"); +# Can't combine when different manifest system identifier +rename("$backup2path/backup_manifest", "$backup2path/backup_manifest.orig") + or die "could not move $backup2path/backup_manifest"; +copy("$backupother2path/backup_manifest", "$backup2path/backup_manifest") + or die "could not copy $backupother2path/backup_manifest"; +$node1->command_fails_like( + [ 'pg_combinebackup', $backup1path, $backup2path, $backup3path, '-o', $resultpath ], + qr/ manifest system identifier is .*, but control file has /, + "can't combine backups with different manifest system identifier "); +# Restore the backup state +move("$backup2path/backup_manifest.orig", "$backup2path/backup_manifest") + or die "could not move $backup2path/backup_manifest"; + # Can't omit a required backup. $node1->command_fails_like( [ 'pg_combinebackup', $backup1path, $backup3path, '-o', $resultpath ], diff --git a/src/bin/pg_combinebackup/write_manifest.c b/src/bin/pg_combinebackup/write_manifest.c index 01deb82cc9..7a2065e1db 100644 --- a/src/bin/pg_combinebackup/write_manifest.c +++ b/src/bin/pg_combinebackup/write_manifest.c @@ -45,7 +45,7 @@ static size_t hex_encode(const uint8 *src, size_t len, char *dst); * in the specified directory. */ manifest_writer * -create_manifest_writer(char *directory) +create_manifest_writer(char *directory, uint64 system_identifier) { manifest_writer *mwriter = pg_malloc(sizeof(manifest_writer)); @@ -57,8 +57,10 @@ create_manifest_writer(char *directory) pg_checksum_init(&mwriter->manifest_ctx, CHECKSUM_TYPE_SHA256); appendStringInfo(&mwriter->buf, - "{ \"PostgreSQL-Backup-Manifest-Version\": 1,\n" - "\"Files\": ["); + "{ \"PostgreSQL-Backup-Manifest-Version\": 2,\n" + "\"System-Identifier\": " UINT64_FORMAT ",\n" + "\"Files\": [", + system_identifier); return mwriter; } diff --git a/src/bin/pg_combinebackup/write_manifest.h b/src/bin/pg_combinebackup/write_manifest.h index de0f742779..ebc4f9441a 100644 --- a/src/bin/pg_combinebackup/write_manifest.h +++ b/src/bin/pg_combinebackup/write_manifest.h @@ -19,7 +19,8 @@ struct manifest_wal_range; struct manifest_writer; typedef struct manifest_writer manifest_writer; -extern manifest_writer *create_manifest_writer(char *directory); +extern manifest_writer *create_manifest_writer(char *directory, + uint64 system_identifier); extern void add_file_to_manifest(manifest_writer *mwriter, const char *manifest_path, size_t size, time_t mtime, diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c index 8561678a7d..0e9b59f2a8 100644 --- a/src/bin/pg_verifybackup/pg_verifybackup.c +++ b/src/bin/pg_verifybackup/pg_verifybackup.c @@ -18,6 +18,7 @@ #include #include +#include "common/controldata_utils.h" #include "common/hashfn.h" #include "common/logging.h" #include "common/parse_manifest.h" @@ -98,6 +99,8 @@ typedef struct manifest_wal_range */ typedef struct manifest_data { + int version; + uint64 system_identifier; manifest_files_hash *files; manifest_wal_range *first_wal_range; manifest_wal_range *last_wal_range; @@ -116,6 +119,10 @@ typedef struct verifier_context } verifier_context; static manifest_data *parse_manifest_file(char *manifest_path); +static void verifybackup_version_cb(JsonManifestParseContext *context, + int manifest_version); +static void verifybackup_system_identifier(JsonManifestParseContext *context, + uint64 manifest_system_identifier); static void verifybackup_per_file_cb(JsonManifestParseContext *context, char *pathname, size_t size, pg_checksum_type checksum_type, @@ -133,6 +140,8 @@ static void verify_backup_directory(verifier_context *context, char *relpath, char *fullpath); static void verify_backup_file(verifier_context *context, char *relpath, char *fullpath); +static void verify_control_file(const char *controlpath, + uint64 manifest_system_identifier); static void report_extra_backup_files(verifier_context *context); static void verify_backup_checksums(verifier_context *context); static void verify_file_checksum(verifier_context *context, @@ -375,9 +384,7 @@ main(int argc, char **argv) } /* - * Parse a manifest file. Construct a hash table with information about - * all the files it mentions, and a linked list of all the WAL ranges it - * mentions. + * Parse a manifest file and return a data structure describing the contents. */ static manifest_data * parse_manifest_file(char *manifest_path) @@ -432,6 +439,8 @@ parse_manifest_file(char *manifest_path) result = pg_malloc0(sizeof(manifest_data)); result->files = ht; context.private_data = result; + context.version_cb = verifybackup_version_cb; + context.system_identifier_cb = verifybackup_system_identifier; context.per_file_cb = verifybackup_per_file_cb; context.per_wal_range_cb = verifybackup_per_wal_range_cb; context.error_cb = report_manifest_error; @@ -461,6 +470,32 @@ report_manifest_error(JsonManifestParseContext *context, const char *fmt,...) exit(1); } +/* + * Record details extracted from the backup manifest. + */ +static void +verifybackup_version_cb(JsonManifestParseContext *context, + int manifest_version) +{ + manifest_data *manifest = context->private_data; + + /* Validation will be at the later stage */ + manifest->version = manifest_version; +} + +/* + * Record details extracted from the backup manifest. + */ +static void +verifybackup_system_identifier(JsonManifestParseContext *context, + uint64 manifest_system_identifier) +{ + manifest_data *manifest = context->private_data; + + /* Validation will be at the later stage */ + manifest->system_identifier = manifest_system_identifier; +} + /* * Record details extracted from the backup manifest for one file. */ @@ -650,6 +685,14 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath) m->bad = true; } + /* + * Validate the manifest system identifier, not available in manifest + * version 1. + */ + if (context->manifest->version != 1 && + strcmp(relpath, "global/pg_control") == 0) + verify_control_file(fullpath, context->manifest->system_identifier); + /* Update statistics for progress report, if necessary */ if (show_progress && !skip_checksums && should_verify_checksum(m)) total_size += m->size; @@ -662,6 +705,39 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath) */ } +/* + * Sanity check control file and validate system identifier against manifest + * system identifier. + */ +static void +verify_control_file(const char *controlpath, uint64 manifest_system_identifier) +{ + ControlFileData *control_file; + bool crc_ok; + + pg_log_debug("reading \"%s\"", controlpath); + control_file = get_controlfile_by_exact_path(controlpath, &crc_ok); + + /* Control file contents not meaningful if CRC is bad. */ + if (!crc_ok) + report_fatal_error("%s: CRC is incorrect", controlpath); + + /* Can't interpret control file if not current version. */ + if (control_file->pg_control_version != PG_CONTROL_VERSION) + report_fatal_error("%s: unexpected control file version", + controlpath); + + /* System identifiers should match. */ + if (manifest_system_identifier != control_file->system_identifier) + report_fatal_error("%s: manifest system identifier is %llu, but control file has %llu", + controlpath, + (unsigned long long) manifest_system_identifier, + (unsigned long long) control_file->system_identifier); + + /* Release memory. */ + pfree(control_file); +} + /* * Scan the hash table for entries where the 'matched' flag is not set; report * that such files are present in the manifest but not on disk. diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl index 11bd577081..36d032288f 100644 --- a/src/bin/pg_verifybackup/t/003_corruption.pl +++ b/src/bin/pg_verifybackup/t/003_corruption.pl @@ -6,6 +6,7 @@ use strict; use warnings FATAL => 'all'; use File::Path qw(rmtree); +use File::Copy; use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; @@ -68,6 +69,11 @@ my @scenario = ( 'mutilate' => \&mutilate_replace_file, 'fails_like' => qr/checksum mismatch for file/ }, + { + 'name' => 'system_identifier', + 'mutilate' => \&mutilate_system_identifier, + 'fails_like' => qr/manifest system identifier is .*, but control file has/ + }, { 'name' => 'bad_manifest', 'mutilate' => \&mutilate_bad_manifest, @@ -216,7 +222,7 @@ sub mutilate_append_to_file sub mutilate_truncate_file { my ($backup_path) = @_; - my $pathname = "$backup_path/global/pg_control"; + my $pathname = "$backup_path/pg_hba.conf"; open(my $fh, '>', $pathname) || die "open $pathname: $!"; close($fh); return; @@ -236,6 +242,24 @@ sub mutilate_replace_file return; } +# Copy manifest of other backups to demonstrate the case where the wrong +# manifest is referred +sub mutilate_system_identifier +{ + my ($backup_path) = @_; + + # Set up another new database instance with different system identifier and + # make backup + my $node = PostgreSQL::Test::Cluster->new('node'); + $node->init(force_initdb => 1, allows_streaming => 1); + $node->start; + $node->backup('backup2'); + move($node->backup_dir.'/backup2/backup_manifest', $backup_path.'/backup_manifest') + or BAIL_OUT "could not copy manifest to $backup_path"; + $node->teardown_node(fail_ok => 1); + return; +} + # Corrupt the backup manifest. sub mutilate_bad_manifest { diff --git a/src/bin/pg_verifybackup/t/005_bad_manifest.pl b/src/bin/pg_verifybackup/t/005_bad_manifest.pl index e278ccea5a..77fdfbb9d0 100644 --- a/src/bin/pg_verifybackup/t/005_bad_manifest.pl +++ b/src/bin/pg_verifybackup/t/005_bad_manifest.pl @@ -29,10 +29,14 @@ test_parse_error('expected version indicator', <state = JM_EXPECT_SYSTEM_IDENTIFIER_VALUE; + break; + } + /* Is this the list of files? */ if (strcmp(fname, "Files") == 0) { @@ -404,9 +416,14 @@ json_manifest_scalar(void *state, char *token, JsonTokenType tokentype) switch (parse->state) { case JM_EXPECT_VERSION_VALUE: - if (strcmp(token, "1") != 0) - json_manifest_parse_failure(parse->context, - "unexpected manifest version"); + parse->manifest_version = token; + json_manifest_finalize_version(parse); + parse->state = JM_EXPECT_TOPLEVEL_FIELD; + break; + + case JM_EXPECT_SYSTEM_IDENTIFIER_VALUE: + parse->manifest_system_identifier = token; + json_manifest_finalize_system_identifier(parse); parse->state = JM_EXPECT_TOPLEVEL_FIELD; break; @@ -464,6 +481,59 @@ json_manifest_scalar(void *state, char *token, JsonTokenType tokentype) return JSON_SUCCESS; } +/* + * Do additional parsing and sanity-checking of the manifest version, and invoke + * the callback so that the caller can gets that detail and take actions + * accordingly. This happens for each manifest when the corresponding JSON + * object is completely parsed. + */ +static void +json_manifest_finalize_version(JsonManifestParseState *parse) +{ + JsonManifestParseContext *context = parse->context; + int version; + char *ep; + + Assert(parse->saw_version_field); + + /* Parse version. */ + version = strtoi64(parse->manifest_version, &ep, 10); + if (*ep) + json_manifest_parse_failure(parse->context, + "manifest version not an integer"); + + if (version != 1 && version != 2) + json_manifest_parse_failure(parse->context, + "unexpected manifest version"); + + /* Invoke the callback for version */ + context->version_cb(context, version); +} + +/* + * Do additional parsing and sanity-checking of the system identifier, and + * invoke the callback so that the caller can gets that detail and take actions + * accordingly. + */ +static void +json_manifest_finalize_system_identifier(JsonManifestParseState *parse) +{ + JsonManifestParseContext *context = parse->context; + uint64 system_identifier; + char *ep; + + Assert(parse->manifest_system_identifier != NULL); + + /* Parse system identifier. */ + system_identifier = strtou64(parse->manifest_system_identifier, &ep, 10); + if (*ep) + json_manifest_parse_failure(parse->context, + "manifest system identifier not an integer"); + + /* Invoke the callback for system identifier */ + context->system_identifier_cb(context, system_identifier); +} + /* * Do additional parsing and sanity-checking of the details gathered for one * file, and invoke the per-file callback so that the caller gets those diff --git a/src/include/common/parse_manifest.h b/src/include/common/parse_manifest.h index f74be0db35..78b052c045 100644 --- a/src/include/common/parse_manifest.h +++ b/src/include/common/parse_manifest.h @@ -21,6 +21,10 @@ struct JsonManifestParseContext; typedef struct JsonManifestParseContext JsonManifestParseContext; +typedef void (*json_manifest_version_callback) (JsonManifestParseContext *, + int manifest_version); +typedef void (*json_manifest_system_identifier_callback) (JsonManifestParseContext *, + uint64 manifest_system_identifier); typedef void (*json_manifest_per_file_callback) (JsonManifestParseContext *, char *pathname, size_t size, pg_checksum_type checksum_type, @@ -35,6 +39,8 @@ typedef void (*json_manifest_error_callback) (JsonManifestParseContext *, struct JsonManifestParseContext { void *private_data; + json_manifest_version_callback version_cb; + json_manifest_system_identifier_callback system_identifier_cb; json_manifest_per_file_callback per_file_cb; json_manifest_per_wal_range_callback per_wal_range_cb; json_manifest_error_callback error_cb;