rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

pg_verifybackup: Refactor parse_manifest_file. 

Return a pointer to the manifest_data instead of individual pointers
to relevant data stored within the manifest_data object. The previous
approach scales poorly if we add more things to the backup manifest,
as has been proposed.

Amul Sul, reviewed by Sravan Velagandula, Michael Paquier, and me.

Discussion: http://postgr.es/m/CAAJ_b95=1LONf99-M_ep588fL_WgLJfdnb7XG4GWE7JDD22E4w@mail.gmail.com
This commit is contained in:
Robert Haas 2024-03-04 14:42:17 -05:00
parent dd7ea37c43
commit dc8f2d7c06
1 changed files with 34 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
src/bin/pg_verifybackup/pg_verifybackup.c
View File

@ -94,31 +94,28 @@ typedef struct manifest_wal_range
} manifest_wal_range; } manifest_wal_range;
/* /*
* Details we need in callbacks that occur while parsing a backup manifest. * All the data parsed from a backup_manifest file.
*/ */
typedef struct parser_context typedef struct manifest_data
{ {
manifest_files_hash *ht; manifest_files_hash *files;
manifest_wal_range *first_wal_range; manifest_wal_range *first_wal_range;
manifest_wal_range *last_wal_range; manifest_wal_range *last_wal_range;
} parser_context; } manifest_data;
/* /*
* All of the context information we need while checking a backup manifest. * All of the context information we need while checking a backup manifest.
*/ */
typedef struct verifier_context typedef struct verifier_context
{ {
manifest_files_hash *ht; manifest_data *manifest;
char *backup_directory; char *backup_directory;
SimpleStringList ignore_list; SimpleStringList ignore_list;
bool exit_on_error; bool exit_on_error;
bool saw_any_error; bool saw_any_error;
} verifier_context; } verifier_context;
static void parse_manifest_file(char *manifest_path, static manifest_data *parse_manifest_file(char *manifest_path);
manifest_files_hash **ht_p,
manifest_wal_range **first_wal_range_p);
static void verifybackup_per_file_cb(JsonManifestParseContext *context, static void verifybackup_per_file_cb(JsonManifestParseContext *context,
char *pathname, size_t size, char *pathname, size_t size,
pg_checksum_type checksum_type, pg_checksum_type checksum_type,
@ -142,8 +139,7 @@ static void verify_file_checksum(verifier_context *context,
manifest_file *m, char *fullpath); manifest_file *m, char *fullpath);
static void parse_required_wal(verifier_context *context, static void parse_required_wal(verifier_context *context,
char *pg_waldump_path, char *pg_waldump_path,
char *wal_directory, char *wal_directory);
manifest_wal_range *first_wal_range);
static void report_backup_error(verifier_context *context, static void report_backup_error(verifier_context *context,
const char *pg_restrict fmt,...) const char *pg_restrict fmt,...)
@ -185,7 +181,6 @@ main(int argc, char **argv)
int c; int c;
verifier_context context; verifier_context context;
manifest_wal_range *first_wal_range;
char *manifest_path = NULL; char *manifest_path = NULL;
bool no_parse_wal = false; bool no_parse_wal = false;
bool quiet = false; bool quiet = false;
@ -338,7 +333,7 @@ main(int argc, char **argv)
* the manifest as fatal; there doesn't seem to be much point in trying to * the manifest as fatal; there doesn't seem to be much point in trying to
* verify the backup directory against a corrupted manifest. * verify the backup directory against a corrupted manifest.
*/ */
parse_manifest_file(manifest_path, &context.ht, &first_wal_range); context.manifest = parse_manifest_file(manifest_path);
/* /*
* Now scan the files in the backup directory. At this stage, we verify * Now scan the files in the backup directory. At this stage, we verify
@ -367,8 +362,7 @@ main(int argc, char **argv)
* not to do so. * not to do so.
*/ */
if (!no_parse_wal) if (!no_parse_wal)
parse_required_wal(&context, pg_waldump_path, parse_required_wal(&context, pg_waldump_path, wal_directory);
wal_directory, first_wal_range);
/* /*
* If everything looks OK, tell the user this, unless we were asked to * If everything looks OK, tell the user this, unless we were asked to
@ -385,9 +379,8 @@ main(int argc, char **argv)
* all the files it mentions, and a linked list of all the WAL ranges it * all the files it mentions, and a linked list of all the WAL ranges it
* mentions. * mentions.
*/ */
static void static manifest_data *
parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p, parse_manifest_file(char *manifest_path)
manifest_wal_range **first_wal_range_p)
{ {
int fd; int fd;
struct stat statbuf; struct stat statbuf;
@ -396,8 +389,8 @@ parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p,
manifest_files_hash *ht; manifest_files_hash *ht;
char *buffer; char *buffer;
int rc; int rc;
parser_context private_context;
JsonManifestParseContext context; JsonManifestParseContext context;
manifest_data *result;
/* Open the manifest file. */ /* Open the manifest file. */
if ((fd = open(manifest_path, O_RDONLY | PG_BINARY, 0)) < 0) if ((fd = open(manifest_path, O_RDONLY | PG_BINARY, 0)) < 0)
@ -436,10 +429,9 @@ parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p,
close(fd); close(fd);
/* Parse the manifest. */ /* Parse the manifest. */
private_context.ht = ht; result = pg_malloc0(sizeof(manifest_data));
private_context.first_wal_range = NULL; result->files = ht;
private_context.last_wal_range = NULL; context.private_data = result;
context.private_data = &private_context;
context.per_file_cb = verifybackup_per_file_cb; context.per_file_cb = verifybackup_per_file_cb;
context.per_wal_range_cb = verifybackup_per_wal_range_cb; context.per_wal_range_cb = verifybackup_per_wal_range_cb;
context.error_cb = report_manifest_error; context.error_cb = report_manifest_error;
@ -448,9 +440,7 @@ parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p,
/* Done with the buffer. */ /* Done with the buffer. */
pfree(buffer); pfree(buffer);
/* Return the file hash table and WAL range list we constructed. */ return result;
*ht_p = ht;
*first_wal_range_p = private_context.first_wal_range;
} }
/* /*
@ -480,8 +470,8 @@ verifybackup_per_file_cb(JsonManifestParseContext *context,
pg_checksum_type checksum_type, pg_checksum_type checksum_type,
int checksum_length, uint8 *checksum_payload) int checksum_length, uint8 *checksum_payload)
{ {
parser_context *pcxt = context->private_data; manifest_data *manifest = context->private_data;
manifest_files_hash *ht = pcxt->ht; manifest_files_hash *ht = manifest->files;
manifest_file *m; manifest_file *m;
bool found; bool found;
@ -508,7 +498,7 @@ verifybackup_per_wal_range_cb(JsonManifestParseContext *context,
TimeLineID tli, TimeLineID tli,
XLogRecPtr start_lsn, XLogRecPtr end_lsn) XLogRecPtr start_lsn, XLogRecPtr end_lsn)
{ {
parser_context *pcxt = context->private_data; manifest_data *manifest = context->private_data;
manifest_wal_range *range; manifest_wal_range *range;
/* Allocate and initialize a struct describing this WAL range. */ /* Allocate and initialize a struct describing this WAL range. */
@ -516,15 +506,15 @@ verifybackup_per_wal_range_cb(JsonManifestParseContext *context,
range->tli = tli; range->tli = tli;
range->start_lsn = start_lsn; range->start_lsn = start_lsn;
range->end_lsn = end_lsn; range->end_lsn = end_lsn;
range->prev = pcxt->last_wal_range; range->prev = manifest->last_wal_range;
range->next = NULL; range->next = NULL;
/* Add it to the end of the list. */ /* Add it to the end of the list. */
if (pcxt->first_wal_range == NULL) if (manifest->first_wal_range == NULL)
pcxt->first_wal_range = range; manifest->first_wal_range = range;
else else
pcxt->last_wal_range->next = range; manifest->last_wal_range->next = range;
pcxt->last_wal_range = range; manifest->last_wal_range = range;
} }
/* /*
@ -639,7 +629,7 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath)
} }
/* Check whether there's an entry in the manifest hash. */ /* Check whether there's an entry in the manifest hash. */
m = manifest_files_lookup(context->ht, relpath); m = manifest_files_lookup(context->manifest->files, relpath);
if (m == NULL) if (m == NULL)
{ {
report_backup_error(context, report_backup_error(context,
@ -679,11 +669,12 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath)
static void static void
report_extra_backup_files(verifier_context *context) report_extra_backup_files(verifier_context *context)
{ {
manifest_data *manifest = context->manifest;
manifest_files_iterator it; manifest_files_iterator it;
manifest_file *m; manifest_file *m;
manifest_files_start_iterate(context->ht, &it); manifest_files_start_iterate(manifest->files, &it);
while ((m = manifest_files_iterate(context->ht, &it)) != NULL) while ((m = manifest_files_iterate(manifest->files, &it)) != NULL)
if (!m->matched && !should_ignore_relpath(context, m->pathname)) if (!m->matched && !should_ignore_relpath(context, m->pathname))
report_backup_error(context, report_backup_error(context,
"\"%s\" is present in the manifest but not on disk", "\"%s\" is present in the manifest but not on disk",
@ -698,13 +689,14 @@ report_extra_backup_files(verifier_context *context)
static void static void
verify_backup_checksums(verifier_context *context) verify_backup_checksums(verifier_context *context)
{ {
manifest_data *manifest = context->manifest;
manifest_files_iterator it; manifest_files_iterator it;
manifest_file *m; manifest_file *m;
progress_report(false); progress_report(false);
manifest_files_start_iterate(context->ht, &it); manifest_files_start_iterate(manifest->files, &it);
while ((m = manifest_files_iterate(context->ht, &it)) != NULL) while ((m = manifest_files_iterate(manifest->files, &it)) != NULL)
{ {
if (should_verify_checksum(m) && if (should_verify_checksum(m) &&
!should_ignore_relpath(context, m->pathname)) !should_ignore_relpath(context, m->pathname))
@ -833,9 +825,10 @@ verify_file_checksum(verifier_context *context, manifest_file *m,
*/ */
static void static void
parse_required_wal(verifier_context *context, char *pg_waldump_path, parse_required_wal(verifier_context *context, char *pg_waldump_path,
char *wal_directory, manifest_wal_range *first_wal_range) char *wal_directory)
{ {
manifest_wal_range *this_wal_range = first_wal_range; manifest_data *manifest = context->manifest;
manifest_wal_range *this_wal_range = manifest->first_wal_range;
while (this_wal_range != NULL) while (this_wal_range != NULL)
{ {