From 46c7f921a9dff0faad8172061b553b868d59f5c2 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Wed, 6 May 2020 02:14:32 +0300 Subject: [PATCH 1/9] [Issue #201] WIP --- src/data.c | 28 ++++++++++++++++++------ src/pg_probackup.h | 9 +++++++- src/util.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/src/data.c b/src/data.c index 7628c37e..469a294b 100644 --- a/src/data.c +++ b/src/data.c @@ -860,7 +860,8 @@ restore_data_file(parray *parent_chain, pgFile *dest_file, FILE *out, const char size_t total_write_len = 0; char *in_buf = pgut_malloc(STDIO_BUFSIZE); - for (i = parray_num(parent_chain) - 1; i >= 0; i--) +// for (i = parray_num(parent_chain) - 1; i >= 0; i--) + for (i = 0; i < parray_num(parent_chain); i++) { char from_root[MAXPGPATH]; char from_fullpath[MAXPGPATH]; @@ -916,11 +917,14 @@ restore_data_file(parray *parent_chain, pgFile *dest_file, FILE *out, const char */ total_write_len += restore_data_file_internal(in, out, tmp_file, parse_program_version(backup->program_version), - from_fullpath, to_fullpath, dest_file->n_blocks); + from_fullpath, to_fullpath, dest_file->n_blocks, + &(dest_file)->pagemap); if (fclose(in) != 0) elog(ERROR, "Cannot close file \"%s\": %s", from_fullpath, strerror(errno)); + +// datapagemap_print_debug(&(dest_file)->pagemap); } pg_free(in_buf); @@ -929,7 +933,8 @@ restore_data_file(parray *parent_chain, pgFile *dest_file, FILE *out, const char size_t restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_version, - const char *from_fullpath, const char *to_fullpath, int nblocks) + const char *from_fullpath, const char *to_fullpath, int nblocks, + datapagemap_t *map) { BackupPageHeader header; BlockNumber blknum = 0; @@ -987,9 +992,9 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers } /* sanity? */ - if (header.block < blknum) - elog(ERROR, "Backup is broken at block %u of \"%s\"", - blknum, from_fullpath); +// if (header.block < blknum) +// elog(ERROR, "Backup is broken at block %u of \"%s\"", +// blknum, from_fullpath); blknum = header.block; @@ -1037,6 +1042,15 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers if (compressed_size > BLCKSZ) elog(ERROR, "Size of a blknum %i exceed BLCKSZ", blknum); + /* if this page was already restored, then skip it */ + if (datapagemap_is_set(map, blknum)) + { + elog(WARNING, "Skipping block %u because is was already restored", blknum); + /* TODO: check error */ + fseek(in, compressed_size, SEEK_CUR); + continue; + } + /* read a page from file */ read_len = fread(page.data, 1, MAXALIGN(compressed_size), in); @@ -1092,6 +1106,8 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers write_len += BLCKSZ; cur_pos = write_pos + BLCKSZ; /* update current write position */ + + datapagemap_add(map, blknum); } elog(VERBOSE, "Copied file \"%s\": %lu bytes", from_fullpath, write_len); diff --git a/src/pg_probackup.h b/src/pg_probackup.h index 6a4fc6da..a126bf4e 100644 --- a/src/pg_probackup.h +++ b/src/pg_probackup.h @@ -925,7 +925,8 @@ extern void backup_non_data_file_internal(const char *from_fullpath, extern size_t restore_data_file(parray *parent_chain, pgFile *dest_file, FILE *out, const char *to_fullpath); extern size_t restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_version, - const char *from_fullpath, const char *to_fullpath, int nblocks); + const char *from_fullpath, const char *to_fullpath, int nblocks, + datapagemap_t *map); extern size_t restore_non_data_file(parray *parent_chain, pgBackup *dest_backup, pgFile *dest_file, FILE *out, const char *to_fullpath); extern void restore_non_data_file_internal(FILE *in, FILE *out, pgFile *file, @@ -1037,4 +1038,10 @@ extern void get_header_errormsg(Page page, char **errormsg); extern void get_checksum_errormsg(Page page, char **errormsg, BlockNumber absolute_blkno); +extern bool +datapagemap_is_set(datapagemap_t *map, BlockNumber blkno); + +extern void +datapagemap_print_debug(datapagemap_t *map); + #endif /* PG_PROBACKUP_H */ diff --git a/src/util.c b/src/util.c index 776c2ec7..bf9c6e33 100644 --- a/src/util.c +++ b/src/util.c @@ -495,3 +495,57 @@ str2status(const char *status) return BACKUP_STATUS_INVALID; } + +bool +datapagemap_is_set(datapagemap_t *map, BlockNumber blkno) +{ + int offset; + int bitno; + + offset = blkno / 8; + bitno = blkno % 8; + + /* enlarge or create bitmap if needed */ + if (map->bitmapsize <= offset) + { + int oldsize = map->bitmapsize; + int newsize; + + /* + * The minimum to hold the new bit is offset + 1. But add some + * headroom, so that we don't need to repeatedly enlarge the bitmap in + * the common case that blocks are modified in order, from beginning + * of a relation to the end. + */ + newsize = offset + 1; + newsize += 10; + + map->bitmap = pg_realloc(map->bitmap, newsize); + + /* zero out the newly allocated region */ + memset(&map->bitmap[oldsize], 0, newsize - oldsize); + + map->bitmapsize = newsize; + } + + //datapagemap_print(map); + + /* check the bit */ + return map->bitmap[offset] & (1 << bitno); +} + +/* + * A debugging aid. Prints out the contents of the page map. + */ +void +datapagemap_print_debug(datapagemap_t *map) +{ + datapagemap_iterator_t *iter; + BlockNumber blocknum; + + iter = datapagemap_iterate(map); + while (datapagemap_next(iter, &blocknum)) + elog(INFO, " block %u", blocknum); + + pg_free(iter); +} From 26a834b44c7cde63378ea93b0ae87e14049cf035 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Wed, 6 May 2020 02:30:13 +0300 Subject: [PATCH 2/9] [Issue #201] fix alignment when skipping block during restore --- src/data.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/data.c b/src/data.c index 469a294b..45b159fc 100644 --- a/src/data.c +++ b/src/data.c @@ -1047,7 +1047,7 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers { elog(WARNING, "Skipping block %u because is was already restored", blknum); /* TODO: check error */ - fseek(in, compressed_size, SEEK_CUR); + fseek(in, MAXALIGN(compressed_size), SEEK_CUR); continue; } From b949231b9390edbcae79494338539f4802e8e1e1 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Thu, 7 May 2020 16:21:26 +0300 Subject: [PATCH 3/9] [Issue #201] check fseek return code --- src/data.c | 9 +++++---- src/restore.c | 3 +++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/data.c b/src/data.c index 45b159fc..733aec05 100644 --- a/src/data.c +++ b/src/data.c @@ -1042,12 +1042,13 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers if (compressed_size > BLCKSZ) elog(ERROR, "Size of a blknum %i exceed BLCKSZ", blknum); - /* if this page was already restored, then skip it */ + /* if this page is marked as already restored, then skip it */ if (datapagemap_is_set(map, blknum)) { - elog(WARNING, "Skipping block %u because is was already restored", blknum); - /* TODO: check error */ - fseek(in, MAXALIGN(compressed_size), SEEK_CUR); + /* skip to the next page */ + if (fseek(in, MAXALIGN(compressed_size), SEEK_CUR) != 0) + elog(ERROR, "Cannot seek block %u of '%s': %s", + blknum, from_fullpath, strerror(errno)); continue; } diff --git a/src/restore.c b/src/restore.c index 9e770281..ecd99022 100644 --- a/src/restore.c +++ b/src/restore.c @@ -876,6 +876,9 @@ restore_files(void *arg) arguments->dest_backup, dest_file, out, to_fullpath); } + /* free pagemap used for restore optimization */ + pg_free(dest_file->pagemap.bitmap); + done: /* close file */ if (fio_fclose(out) != 0) From ba29f9f6f70c1da1aa61519ee7d8b4e47aef3f4e Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Thu, 21 May 2020 21:28:38 +0300 Subject: [PATCH 4/9] fix after merge --- src/backup.c | 2 +- src/delete.c | 1 - src/dir.c | 16 ++++++++-------- src/pg_probackup.h | 10 +++++----- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/backup.c b/src/backup.c index 8d084fe4..72fd7c26 100644 --- a/src/backup.c +++ b/src/backup.c @@ -2223,7 +2223,7 @@ parse_filelist_filenames(parray *files, const char *root) if (S_ISREG(file->mode) && file->tblspcOid != 0 && file->name && file->name[0]) { - if (file->forkName == INIT) + if (file->forkName == init) { /* * Do not backup files of unlogged relations. diff --git a/src/delete.c b/src/delete.c index b5e6fb68..20185db5 100644 --- a/src/delete.c +++ b/src/delete.c @@ -966,7 +966,6 @@ do_delete_instance(void) { parray *backup_list; int i; - int rc; char instance_config_path[MAXPGPATH]; diff --git a/src/dir.c b/src/dir.c index 13c8d0cf..ea8b56b5 100644 --- a/src/dir.c +++ b/src/dir.c @@ -709,22 +709,22 @@ dir_check_file(pgFile *file) { /* Auxiliary fork of the relfile */ if (strcmp(fork_name, "vm") == 0) - file->forkName = VM; + file->forkName = vm; else if (strcmp(fork_name, "fsm") == 0) - file->forkName = FSM; + file->forkName = fsm; else if (strcmp(fork_name, "cfm") == 0) - file->forkName = CFM; - - else if (strcmp(fork_name, "init") == 0) - file->forkName = INIT; + file->forkName = cfm; else if (strcmp(fork_name, "ptrack") == 0) - file->forkName = PTRACK; + file->forkName = ptrack; + + else if (strcmp(fork_name, "init") == 0) + file->forkName = init; /* Do not backup ptrack files */ - if (file->forkName == PTRACK) + if (file->forkName == ptrack) return CHECK_FALSE; } else diff --git a/src/pg_probackup.h b/src/pg_probackup.h index 1d636493..5c8f4349 100644 --- a/src/pg_probackup.h +++ b/src/pg_probackup.h @@ -120,11 +120,11 @@ typedef enum CompressAlg typedef enum ForkName { - VM, - FSM, - CFM, - INIT, - PTRACK + vm, + fsm, + cfm, + init, + ptrack } ForkName; #define INIT_FILE_CRC32(use_crc32c, crc) \ From eea727db1755fef802f3f8675ccaabe170dc95bc Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Fri, 22 May 2020 23:22:54 +0300 Subject: [PATCH 5/9] tests: fix tests.backup.BackupTest.test_page_detect_corruption --- tests/backup.py | 75 ++++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/tests/backup.py b/tests/backup.py index d99cfe10..be51dfef 100644 --- a/tests/backup.py +++ b/tests/backup.py @@ -324,7 +324,6 @@ class BackupTest(ProbackupTest, unittest.TestCase): node = self.make_simple_node( base_dir=os.path.join(module_name, fname, 'node'), set_replication=True, - ptrack_enable=True, initdb_params=['--data-checksums']) backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup') @@ -356,26 +355,32 @@ class BackupTest(ProbackupTest, unittest.TestCase): "postgres", "select pg_relation_filepath('t_heap')").rstrip() - with open(os.path.join(node.data_dir, heap_path), "rb+", 0) as f: + path = os.path.join(node.data_dir, heap_path) + with open(path, "rb+", 0) as f: f.seek(9000) f.write(b"bla") f.flush() f.close - self.backup_node( - backup_dir, 'node', node, backup_type="full", - options=["-j", "4", "--stream", "--log-level-file=VERBOSE"]) + try: + self.backup_node( + backup_dir, 'node', node, backup_type="full", + options=["-j", "4", "--stream", "--log-level-file=VERBOSE"]) + self.assertEqual( + 1, 0, + "Expecting Error because data file is corrupted" + "\n Output: {0} \n CMD: {1}".format( + repr(self.output), self.cmd)) + except ProbackupException as e: + self.assertTrue( + 'ERROR: Corruption detected in file "{0}", ' + 'block 1: page verification failed, calculated checksum'.format(path), + e.message) - # open log file and check - with open(os.path.join(backup_dir, 'log', 'pg_probackup.log')) as f: - log_content = f.read() - self.assertIn('block 1, try to fetch via shared buffer', log_content) - self.assertIn('SELECT pg_catalog.pg_ptrack_get_block', log_content) - f.close - - self.assertTrue( - self.show_pb(backup_dir, 'node')[1]['status'] == 'OK', - "Backup Status should be OK") + self.assertEqual( + self.show_pb(backup_dir, 'node')[1]['status'], + 'ERROR', + "Backup Status should be ERROR") # Clean after yourself self.del_test_dir(module_name, fname) @@ -453,8 +458,8 @@ class BackupTest(ProbackupTest, unittest.TestCase): repr(self.output), self.cmd)) except ProbackupException as e: self.assertIn( - "ERROR: Corruption detected in file '{0}', block 1: " - "page verification failed, calculated checksum".format( + 'ERROR: Corruption detected in file "{0}", block 1: ' + 'page verification failed, calculated checksum'.format( heap_fullpath), e.message, '\n Unexpected Error Message: {0}\n CMD: {1}'.format( @@ -474,8 +479,8 @@ class BackupTest(ProbackupTest, unittest.TestCase): repr(self.output), self.cmd)) except ProbackupException as e: self.assertIn( - "ERROR: Corruption detected in file '{0}', block 1: " - "page verification failed, calculated checksum".format( + 'ERROR: Corruption detected in file "{0}", block 1: ' + 'page verification failed, calculated checksum'.format( heap_fullpath), e.message, '\n Unexpected Error Message: {0}\n CMD: {1}'.format( @@ -495,8 +500,8 @@ class BackupTest(ProbackupTest, unittest.TestCase): repr(self.output), self.cmd)) except ProbackupException as e: self.assertIn( - "ERROR: Corruption detected in file '{0}', block 1: " - "page verification failed, calculated checksum".format( + 'ERROR: Corruption detected in file "{0}", block 1: ' + 'page verification failed, calculated checksum'.format( heap_fullpath), e.message, '\n Unexpected Error Message: {0}\n CMD: {1}'.format( @@ -598,8 +603,8 @@ class BackupTest(ProbackupTest, unittest.TestCase): repr(self.output), self.cmd)) except ProbackupException as e: self.assertIn( - "ERROR: Corruption detected in file '{0}', block 1: " - "page header invalid, pd_lower".format(heap_fullpath), + 'ERROR: Corruption detected in file "{0}", block 1: ' + 'page header invalid, pd_lower'.format(heap_fullpath), e.message, '\n Unexpected Error Message: {0}\n CMD: {1}'.format( repr(e.message), self.cmd)) @@ -618,8 +623,8 @@ class BackupTest(ProbackupTest, unittest.TestCase): repr(self.output), self.cmd)) except ProbackupException as e: self.assertIn( - "ERROR: Corruption detected in file '{0}', block 1: " - "page header invalid, pd_lower".format(heap_fullpath), + 'ERROR: Corruption detected in file "{0}", block 1: ' + 'page header invalid, pd_lower'.format(heap_fullpath), e.message, '\n Unexpected Error Message: {0}\n CMD: {1}'.format( repr(e.message), self.cmd)) @@ -638,8 +643,8 @@ class BackupTest(ProbackupTest, unittest.TestCase): repr(self.output), self.cmd)) except ProbackupException as e: self.assertIn( - "ERROR: Corruption detected in file '{0}', block 1: " - "page header invalid, pd_lower".format(heap_fullpath), + 'ERROR: Corruption detected in file "{0}", block 1: ' + 'page header invalid, pd_lower'.format(heap_fullpath), e.message, '\n Unexpected Error Message: {0}\n CMD: {1}'.format( repr(e.message), self.cmd)) @@ -741,8 +746,8 @@ class BackupTest(ProbackupTest, unittest.TestCase): repr(self.output), self.cmd)) except ProbackupException as e: self.assertIn( - "ERROR: Corruption detected in file '{0}', block 1: " - "page header invalid, pd_lower".format(heap_fullpath), + 'ERROR: Corruption detected in file "{0}", block 1: ' + 'page header invalid, pd_lower'.format(heap_fullpath), e.message, '\n Unexpected Error Message: {0}\n CMD: {1}'.format( repr(e.message), self.cmd)) @@ -761,8 +766,8 @@ class BackupTest(ProbackupTest, unittest.TestCase): repr(self.output), self.cmd)) except ProbackupException as e: self.assertIn( - "ERROR: Corruption detected in file '{0}', block 1: " - "page header invalid, pd_lower".format(heap_fullpath), + 'ERROR: Corruption detected in file "{0}", block 1: ' + 'page header invalid, pd_lower'.format(heap_fullpath), e.message, '\n Unexpected Error Message: {0}\n CMD: {1}'.format( repr(e.message), self.cmd)) @@ -781,8 +786,8 @@ class BackupTest(ProbackupTest, unittest.TestCase): repr(self.output), self.cmd)) except ProbackupException as e: self.assertIn( - "ERROR: Corruption detected in file '{0}', block 1: " - "page header invalid, pd_lower".format(heap_fullpath), + 'ERROR: Corruption detected in file "{0}", block 1: ' + 'page header invalid, pd_lower'.format(heap_fullpath), e.message, '\n Unexpected Error Message: {0}\n CMD: {1}'.format( repr(e.message), self.cmd)) @@ -1385,8 +1390,8 @@ class BackupTest(ProbackupTest, unittest.TestCase): with open(os.path.join(backup_dir, 'log', 'pg_probackup.log')) as f: log_content = f.read() self.assertTrue( - "LOG: File '{0}' is not found".format(absolute_path) in log_content, - "File '{0}' should be deleted but it`s not".format(absolute_path)) + 'LOG: File "{0}" is not found'.format(absolute_path) in log_content, + 'File "{0}" should be deleted but it`s not'.format(absolute_path)) node.cleanup() self.restore_node(backup_dir, 'node', node, options=["-j", "4"]) From 8aa5231c9a50c681ee4bf79945b138eb4db200b8 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Sat, 23 May 2020 01:07:29 +0300 Subject: [PATCH 6/9] [Issue #201] fix restore of files from versions older than 2.3.0 --- src/data.c | 37 ++++++++++++++++++++++++++++++------- src/merge.c | 15 +++++++++++---- src/pg_probackup.h | 2 +- src/restore.c | 16 +++++++++++++++- 4 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/data.c b/src/data.c index 046ca191..571c40a0 100644 --- a/src/data.c +++ b/src/data.c @@ -861,14 +861,25 @@ backup_non_data_file(pgFile *file, pgFile *prev_file, * Apply changed blocks to destination file from every backup in parent chain. */ size_t -restore_data_file(parray *parent_chain, pgFile *dest_file, FILE *out, const char *to_fullpath) +restore_data_file(parray *parent_chain, pgFile *dest_file, FILE *out, + const char *to_fullpath, bool use_bitmap) { - int i; size_t total_write_len = 0; char *in_buf = pgut_malloc(STDIO_BUFSIZE); + int backup_seq = 0; + + // FULL -> INCR -> DEST + // 2 1 0 + if (use_bitmap) + /* start with dest backup */ + backup_seq = 0; + else + /* start with full backup */ + backup_seq = parray_num(parent_chain) - 1; // for (i = parray_num(parent_chain) - 1; i >= 0; i--) - for (i = 0; i < parray_num(parent_chain); i++) +// for (i = 0; i < parray_num(parent_chain); i++) + while (backup_seq >= 0 && backup_seq < parray_num(parent_chain)) { char from_root[MAXPGPATH]; char from_fullpath[MAXPGPATH]; @@ -877,7 +888,12 @@ restore_data_file(parray *parent_chain, pgFile *dest_file, FILE *out, const char pgFile **res_file = NULL; pgFile *tmp_file = NULL; - pgBackup *backup = (pgBackup *) parray_get(parent_chain, i); + pgBackup *backup = (pgBackup *) parray_get(parent_chain, backup_seq); + + if (use_bitmap) + backup_seq++; + else + backup_seq--; /* lookup file in intermediate backup */ res_file = parray_bsearch(backup->files, dest_file, pgFileCompareRelPathWithExternal); @@ -925,7 +941,7 @@ restore_data_file(parray *parent_chain, pgFile *dest_file, FILE *out, const char total_write_len += restore_data_file_internal(in, out, tmp_file, parse_program_version(backup->program_version), from_fullpath, to_fullpath, dest_file->n_blocks, - &(dest_file)->pagemap); + use_bitmap ? &(dest_file)->pagemap : NULL); if (fclose(in) != 0) elog(ERROR, "Cannot close file \"%s\": %s", from_fullpath, @@ -938,6 +954,11 @@ restore_data_file(parray *parent_chain, pgFile *dest_file, FILE *out, const char return total_write_len; } +/* Restore block from "in" file to "out" file. + * If "nblocks" is greater than zero, then skip restoring blocks, + * whose position if greater than "nblocks". + * If map is NULL, then page bitmap cannot be used for restore optimization + */ size_t restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_version, const char *from_fullpath, const char *to_fullpath, int nblocks, @@ -1050,7 +1071,7 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers elog(ERROR, "Size of a blknum %i exceed BLCKSZ", blknum); /* if this page is marked as already restored, then skip it */ - if (datapagemap_is_set(map, blknum)) + if (map && datapagemap_is_set(map, blknum)) { /* skip to the next page */ if (fseek(in, MAXALIGN(compressed_size), SEEK_CUR) != 0) @@ -1115,7 +1136,8 @@ restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_vers write_len += BLCKSZ; cur_pos = write_pos + BLCKSZ; /* update current write position */ - datapagemap_add(map, blknum); + if (map) + datapagemap_add(map, blknum); } elog(VERBOSE, "Copied file \"%s\": %lu bytes", from_fullpath, write_len); @@ -1191,6 +1213,7 @@ restore_non_data_file(parray *parent_chain, pgBackup *dest_backup, * full copy of destination file. * Full copy is latest possible destination file with size equal or * greater than zero. + * TODO: rewrite to use parent_link of dest backup. */ for (i = 1; i < parray_num(parent_chain); i++) { diff --git a/src/merge.c b/src/merge.c index dd521c31..bf7dc1fd 100644 --- a/src/merge.c +++ b/src/merge.c @@ -28,6 +28,7 @@ typedef struct // size_t in_place_merge_bytes; bool compression_match; bool program_version_match; + bool use_bitmap; /* * Return value from the thread. @@ -47,7 +48,7 @@ get_external_index(const char *key, const parray *list); static void merge_data_file(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup, pgFile *dest_file, - pgFile *tmp_file, const char *to_root); + pgFile *tmp_file, const char *to_root, bool use_bitmap); static void merge_non_data_file(parray *parent_chain, pgBackup *full_backup, @@ -439,6 +440,7 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup) *dest_externals = NULL; parray *result_filelist = NULL; + bool use_bitmap = true; // size_t total_in_place_merge_bytes = 0; pthread_t *threads = NULL; @@ -601,6 +603,9 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup) if (full_externals && dest_externals) reorder_external_dirs(full_backup, full_externals, dest_externals); + if (parse_program_version(dest_backup->program_version) < 20300) + use_bitmap = false; + /* Setup threads */ for (i = 0; i < parray_num(dest_backup->files); i++) { @@ -639,6 +644,7 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup) arg->compression_match = compression_match; arg->program_version_match = program_version_match; + arg->use_bitmap = use_bitmap; /* By default there are some error */ arg->ret = 1; @@ -1028,7 +1034,8 @@ merge_files(void *arg) arguments->full_backup, arguments->dest_backup, dest_file, tmp_file, - arguments->full_database_dir); + arguments->full_database_dir, + arguments->use_bitmap); else merge_non_data_file(arguments->parent_chain, arguments->full_backup, @@ -1128,7 +1135,7 @@ reorder_external_dirs(pgBackup *to_backup, parray *to_external, void merge_data_file(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup, pgFile *dest_file, pgFile *tmp_file, - const char *full_database_dir) + const char *full_database_dir, bool use_bitmap) { FILE *out = NULL; char *buffer = pgut_malloc(STDIO_BUFSIZE); @@ -1154,7 +1161,7 @@ merge_data_file(parray *parent_chain, pgBackup *full_backup, setvbuf(out, buffer, _IOFBF, STDIO_BUFSIZE); /* restore file into temp file */ - tmp_file->size = restore_data_file(parent_chain, dest_file, out, to_fullpath_tmp1); + tmp_file->size = restore_data_file(parent_chain, dest_file, out, to_fullpath_tmp1, use_bitmap); fclose(out); pg_free(buffer); diff --git a/src/pg_probackup.h b/src/pg_probackup.h index c27c4139..c9a2a5ab 100644 --- a/src/pg_probackup.h +++ b/src/pg_probackup.h @@ -935,7 +935,7 @@ extern void backup_non_data_file_internal(const char *from_fullpath, bool missing_ok); extern size_t restore_data_file(parray *parent_chain, pgFile *dest_file, - FILE *out, const char *to_fullpath); + FILE *out, const char *to_fullpath, bool use_bitmap); extern size_t restore_data_file_internal(FILE *in, FILE *out, pgFile *file, uint32 backup_version, const char *from_fullpath, const char *to_fullpath, int nblocks, datapagemap_t *map); diff --git a/src/restore.c b/src/restore.c index bd339fbd..10e52b1a 100644 --- a/src/restore.c +++ b/src/restore.c @@ -27,6 +27,7 @@ typedef struct bool skip_external_dirs; const char *to_root; size_t restored_bytes; + bool use_bitmap; /* * Return value from the thread. @@ -501,6 +502,7 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain, pthread_t *threads; restore_files_arg *threads_args; bool restore_isok = true; + bool use_bitmap = true; /* fancy reporting */ char pretty_dest_bytes[20]; @@ -560,6 +562,13 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain, parray_qsort(backup->files, pgFileCompareRelPathWithExternal); } + /* If dest backup version is older than 2.3.0, then bitmap optimization + * is impossible to use, because bitmap restore rely on pgFile.n_blocks, + * which is not always available in old backups. + */ + if (parse_program_version(dest_backup->program_version) < 20300) + use_bitmap = false; + /* * Restore dest_backup internal directories. */ @@ -645,6 +654,7 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain, arg->dbOid_exclude_list = dbOid_exclude_list; arg->skip_external_dirs = params->skip_external_dirs; arg->to_root = pgdata_path; + arg->use_bitmap = use_bitmap; threads_args[i].restored_bytes = 0; /* By default there are some error */ threads_args[i].ret = 1; @@ -859,7 +869,8 @@ restore_files(void *arg) setvbuf(out, out_buf, _IOFBF, STDIO_BUFSIZE); /* Destination file is data file */ arguments->restored_bytes += restore_data_file(arguments->parent_chain, - dest_file, out, to_fullpath); + dest_file, out, to_fullpath, + arguments->use_bitmap); } else { @@ -976,6 +987,9 @@ create_recovery_conf(time_t backup_id, elog(ERROR, "cannot open file \"%s\": %s", path, strerror(errno)); + if (fio_chmod(path, FILE_PERMISSION, FIO_DB_HOST) == -1) + elog(ERROR, "Cannot change mode of \"%s\": %s", path, strerror(errno)); + #if PG_VERSION_NUM >= 120000 fio_fprintf(fp, "# probackup_recovery.conf generated by pg_probackup %s\n", PROGRAM_VERSION); From d4a8384cfaa720f6e7dfd2d287a8acc5e3fc7b06 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Sat, 23 May 2020 03:45:24 +0300 Subject: [PATCH 7/9] [Issue #201] use parent link instead of backup list in restore_non_data_file() --- src/data.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/data.c b/src/data.c index 571c40a0..ced1c716 100644 --- a/src/data.c +++ b/src/data.c @@ -1191,7 +1191,7 @@ size_t restore_non_data_file(parray *parent_chain, pgBackup *dest_backup, pgFile *dest_file, FILE *out, const char *to_fullpath) { - int i; +// int i; char from_root[MAXPGPATH]; char from_fullpath[MAXPGPATH]; FILE *in = NULL; @@ -1213,14 +1213,12 @@ restore_non_data_file(parray *parent_chain, pgBackup *dest_backup, * full copy of destination file. * Full copy is latest possible destination file with size equal or * greater than zero. - * TODO: rewrite to use parent_link of dest backup. */ - for (i = 1; i < parray_num(parent_chain); i++) + tmp_backup = dest_backup->parent_backup_link; + while (tmp_backup) { pgFile **res_file = NULL; - tmp_backup = (pgBackup *) parray_get(parent_chain, i); - /* lookup file in intermediate backup */ res_file = parray_bsearch(tmp_backup->files, dest_file, pgFileCompareRelPathWithExternal); tmp_file = (res_file) ? *res_file : NULL; @@ -1243,6 +1241,8 @@ restore_non_data_file(parray *parent_chain, pgBackup *dest_backup, /* Full copy is found */ if (tmp_file->write_size > 0) break; + + tmp_backup = tmp_backup->parent_backup_link; } } @@ -1254,6 +1254,11 @@ restore_non_data_file(parray *parent_chain, pgBackup *dest_backup, if (!tmp_file) elog(ERROR, "Failed to locate a full copy of non-data file \"%s\"", to_fullpath); + if (tmp_file->write_size <= 0) + elog(ERROR, "Full copy of non-data file has invalid size. " + "Metadata corruption in backup %s in file: \"%s\"", + base36enc(tmp_backup->start_time), to_fullpath); + if (tmp_file->external_dir_num == 0) join_path_components(from_root, tmp_backup->root_dir, DATABASE_DIR); else From d67cb67243dae7a93c383a996fa56e90893d5df8 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Sat, 23 May 2020 04:25:19 +0300 Subject: [PATCH 8/9] [Issue #201] do not employ page bitmap when restoring a single full backup --- src/restore.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/restore.c b/src/restore.c index 10e52b1a..50440f56 100644 --- a/src/restore.c +++ b/src/restore.c @@ -569,6 +569,10 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain, if (parse_program_version(dest_backup->program_version) < 20300) use_bitmap = false; + /* There is no point in bitmap restore, when restoring a single FULL backup */ + if (parray_num(parent_chain) == 1) + use_bitmap = false; + /* * Restore dest_backup internal directories. */ From 721a15360b32b7cd862e65e85c4f16400a8c95b3 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Sat, 23 May 2020 13:48:16 +0300 Subject: [PATCH 9/9] tests: fix tests.retention.RetentionTest.test_window_merge_interleaved_incremental_chains_1 --- tests/retention.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/retention.py b/tests/retention.py index e797d3c6..7a2e80a5 100644 --- a/tests/retention.py +++ b/tests/retention.py @@ -629,15 +629,15 @@ class RetentionTest(ProbackupTest, unittest.TestCase): self.set_archiving(backup_dir, 'node', node) node.slow_start() - node.pgbench_init(scale=3) + node.pgbench_init(scale=5) # Take FULL BACKUPs backup_id_a = self.backup_node(backup_dir, 'node', node) - pgbench = node.pgbench(options=['-t', '10', '-c', '2']) + pgbench = node.pgbench(options=['-t', '20', '-c', '1']) pgbench.wait() backup_id_b = self.backup_node(backup_dir, 'node', node) - pgbench = node.pgbench(options=['-t', '10', '-c', '2']) + pgbench = node.pgbench(options=['-t', '20', '-c', '1']) pgbench.wait() # Change FULL B backup status to ERROR @@ -648,7 +648,7 @@ class RetentionTest(ProbackupTest, unittest.TestCase): pgdata_a1 = self.pgdata_content(node.data_dir) - pgbench = node.pgbench(options=['-t', '10', '-c', '2']) + pgbench = node.pgbench(options=['-t', '20', '-c', '1']) pgbench.wait() # PAGEa1 OK @@ -666,20 +666,20 @@ class RetentionTest(ProbackupTest, unittest.TestCase): page_id_b1 = self.backup_node( backup_dir, 'node', node, backup_type='page') - pgbench = node.pgbench(options=['-t', '10', '-c', '2']) + pgbench = node.pgbench(options=['-t', '20', '-c', '1']) pgbench.wait() page_id_b2 = self.backup_node( backup_dir, 'node', node, backup_type='page') - pgbench = node.pgbench(options=['-t', '10', '-c', '2']) + pgbench = node.pgbench(options=['-t', '20', '-c', '1']) pgbench.wait() page_id_b3 = self.backup_node( backup_dir, 'node', node, backup_type='page') pgdata_b3 = self.pgdata_content(node.data_dir) - pgbench = node.pgbench(options=['-t', '10', '-c', '2']) + pgbench = node.pgbench(options=['-t', '20', '-c', '1']) pgbench.wait() # PAGEb3 OK