Skip to content

Commit dc8f2d7

Browse files
robertmhaasrobertmhaas
committed
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
1 parent dd7ea37 commit dc8f2d7

File tree

1 file changed

+34
-41
lines changed

1 file changed

+34
-41
lines changed

‎src/bin/pg_verifybackup/pg_verifybackup.c

+34-41
Original file line numberDiff line numberDiff line change
@@ -94,31 +94,28 @@ typedef struct manifest_wal_range
9494
} manifest_wal_range;
9595

9696
/*
97-
* Details we need in callbacks that occur while parsing a backup manifest.
97+
* All the data parsed from a backup_manifest file.
9898
*/
99-
typedef struct parser_context
99+
typedef struct manifest_data
100100
{
101-
manifest_files_hash *ht;
101+
manifest_files_hash *files;
102102
manifest_wal_range *first_wal_range;
103103
manifest_wal_range *last_wal_range;
104-
} parser_context;
104+
} manifest_data;
105105

106106
/*
107107
* All of the context information we need while checking a backup manifest.
108108
*/
109109
typedef struct verifier_context
110110
{
111-
manifest_files_hash *ht;
111+
manifest_data *manifest;
112112
char *backup_directory;
113113
SimpleStringList ignore_list;
114114
bool exit_on_error;
115115
bool saw_any_error;
116116
} verifier_context;
117117

118-
static void parse_manifest_file(char *manifest_path,
119-
manifest_files_hash **ht_p,
120-
manifest_wal_range **first_wal_range_p);
121-
118+
static manifest_data *parse_manifest_file(char *manifest_path);
122119
static void verifybackup_per_file_cb(JsonManifestParseContext *context,
123120
char *pathname, size_t size,
124121
pg_checksum_type checksum_type,
@@ -142,8 +139,7 @@ static void verify_file_checksum(verifier_context *context,
142139
manifest_file *m, char *fullpath);
143140
static void parse_required_wal(verifier_context *context,
144141
char *pg_waldump_path,
145-
char *wal_directory,
146-
manifest_wal_range *first_wal_range);
142+
char *wal_directory);
147143

148144
static void report_backup_error(verifier_context *context,
149145
const char *pg_restrict fmt,...)
@@ -185,7 +181,6 @@ main(int argc, char **argv)
185181

186182
int c;
187183
verifier_context context;
188-
manifest_wal_range *first_wal_range;
189184
char *manifest_path = NULL;
190185
bool no_parse_wal = false;
191186
bool quiet = false;
@@ -338,7 +333,7 @@ main(int argc, char **argv)
338333
* the manifest as fatal; there doesn't seem to be much point in trying to
339334
* verify the backup directory against a corrupted manifest.
340335
*/
341-
parse_manifest_file(manifest_path, &context.ht, &first_wal_range);
336+
context.manifest = parse_manifest_file(manifest_path);
342337

343338
/*
344339
* Now scan the files in the backup directory. At this stage, we verify
@@ -367,8 +362,7 @@ main(int argc, char **argv)
367362
* not to do so.
368363
*/
369364
if (!no_parse_wal)
370-
parse_required_wal(&context, pg_waldump_path,
371-
wal_directory, first_wal_range);
365+
parse_required_wal(&context, pg_waldump_path, wal_directory);
372366

373367
/*
374368
* If everything looks OK, tell the user this, unless we were asked to
@@ -385,9 +379,8 @@ main(int argc, char **argv)
385379
* all the files it mentions, and a linked list of all the WAL ranges it
386380
* mentions.
387381
*/
388-
static void
389-
parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p,
390-
manifest_wal_range **first_wal_range_p)
382+
static manifest_data *
383+
parse_manifest_file(char *manifest_path)
391384
{
392385
int fd;
393386
struct stat statbuf;
@@ -396,8 +389,8 @@ parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p,
396389
manifest_files_hash *ht;
397390
char *buffer;
398391
int rc;
399-
parser_context private_context;
400392
JsonManifestParseContext context;
393+
manifest_data *result;
401394

402395
/* Open the manifest file. */
403396
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,
436429
close(fd);
437430

438431
/* Parse the manifest. */
439-
private_context.ht = ht;
440-
private_context.first_wal_range = NULL;
441-
private_context.last_wal_range = NULL;
442-
context.private_data = &private_context;
432+
result = pg_malloc0(sizeof(manifest_data));
433+
result->files = ht;
434+
context.private_data = result;
443435
context.per_file_cb = verifybackup_per_file_cb;
444436
context.per_wal_range_cb = verifybackup_per_wal_range_cb;
445437
context.error_cb = report_manifest_error;
@@ -448,9 +440,7 @@ parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p,
448440
/* Done with the buffer. */
449441
pfree(buffer);
450442

451-
/* Return the file hash table and WAL range list we constructed. */
452-
*ht_p = ht;
453-
*first_wal_range_p = private_context.first_wal_range;
443+
return result;
454444
}
455445

456446
/*
@@ -480,8 +470,8 @@ verifybackup_per_file_cb(JsonManifestParseContext *context,
480470
pg_checksum_type checksum_type,
481471
int checksum_length, uint8 *checksum_payload)
482472
{
483-
parser_context *pcxt = context->private_data;
484-
manifest_files_hash *ht = pcxt->ht;
473+
manifest_data *manifest = context->private_data;
474+
manifest_files_hash *ht = manifest->files;
485475
manifest_file *m;
486476
bool found;
487477

@@ -508,23 +498,23 @@ verifybackup_per_wal_range_cb(JsonManifestParseContext *context,
508498
TimeLineID tli,
509499
XLogRecPtr start_lsn, XLogRecPtr end_lsn)
510500
{
511-
parser_context *pcxt = context->private_data;
501+
manifest_data *manifest = context->private_data;
512502
manifest_wal_range *range;
513503

514504
/* Allocate and initialize a struct describing this WAL range. */
515505
range = palloc(sizeof(manifest_wal_range));
516506
range->tli = tli;
517507
range->start_lsn = start_lsn;
518508
range->end_lsn = end_lsn;
519-
range->prev = pcxt->last_wal_range;
509+
range->prev = manifest->last_wal_range;
520510
range->next = NULL;
521511

522512
/* Add it to the end of the list. */
523-
if (pcxt->first_wal_range == NULL)
524-
pcxt->first_wal_range = range;
513+
if (manifest->first_wal_range == NULL)
514+
manifest->first_wal_range = range;
525515
else
526-
pcxt->last_wal_range->next = range;
527-
pcxt->last_wal_range = range;
516+
manifest->last_wal_range->next = range;
517+
manifest->last_wal_range = range;
528518
}
529519

530520
/*
@@ -639,7 +629,7 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath)
639629
}
640630

641631
/* Check whether there's an entry in the manifest hash. */
642-
m = manifest_files_lookup(context->ht, relpath);
632+
m = manifest_files_lookup(context->manifest->files, relpath);
643633
if (m == NULL)
644634
{
645635
report_backup_error(context,
@@ -679,11 +669,12 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath)
679669
static void
680670
report_extra_backup_files(verifier_context *context)
681671
{
672+
manifest_data *manifest = context->manifest;
682673
manifest_files_iterator it;
683674
manifest_file *m;
684675

685-
manifest_files_start_iterate(context->ht, &it);
686-
while ((m = manifest_files_iterate(context->ht, &it)) != NULL)
676+
manifest_files_start_iterate(manifest->files, &it);
677+
while ((m = manifest_files_iterate(manifest->files, &it)) != NULL)
687678
if (!m->matched && !should_ignore_relpath(context, m->pathname))
688679
report_backup_error(context,
689680
"\"%s\" is present in the manifest but not on disk",
@@ -698,13 +689,14 @@ report_extra_backup_files(verifier_context *context)
698689
static void
699690
verify_backup_checksums(verifier_context *context)
700691
{
692+
manifest_data *manifest = context->manifest;
701693
manifest_files_iterator it;
702694
manifest_file *m;
703695

704696
progress_report(false);
705697

706-
manifest_files_start_iterate(context->ht, &it);
707-
while ((m = manifest_files_iterate(context->ht, &it)) != NULL)
698+
manifest_files_start_iterate(manifest->files, &it);
699+
while ((m = manifest_files_iterate(manifest->files, &it)) != NULL)
708700
{
709701
if (should_verify_checksum(m) &&
710702
!should_ignore_relpath(context, m->pathname))
@@ -833,9 +825,10 @@ verify_file_checksum(verifier_context *context, manifest_file *m,
833825
*/
834826
static void
835827
parse_required_wal(verifier_context *context, char *pg_waldump_path,
836-
char *wal_directory, manifest_wal_range *first_wal_range)
828+
char *wal_directory)
837829
{
838-
manifest_wal_range *this_wal_range = first_wal_range;
830+
manifest_data *manifest = context->manifest;
831+
manifest_wal_range *this_wal_range = manifest->first_wal_range;
839832

840833
while (this_wal_range != NULL)
841834
{

0 commit comments

Comments
 (0)