From 01fedaf04cc79247b1803d0ba80067d70d50c59f Mon Sep 17 00:00:00 2001 From: Anastasia Date: Wed, 3 Apr 2019 16:35:57 +0300 Subject: [PATCH 01/20] pgpro-2605. Use absolute path of pg_probackup binary in recovery.conf generated by pg_probackup --- src/pg_probackup.c | 9 +++++++++ src/restore.c | 2 +- src/utils/pgut.h | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/pg_probackup.c b/src/pg_probackup.c index 67db3bb1..24ba2759 100644 --- a/src/pg_probackup.c +++ b/src/pg_probackup.c @@ -22,6 +22,7 @@ const char *PROGRAM_VERSION = "2.0.27"; const char *PROGRAM_URL = "https://github.com/postgrespro/pg_probackup"; const char *PROGRAM_EMAIL = "https://github.com/postgrespro/pg_probackup/issues"; +const char *PROGRAM_FULL_PATH = NULL; typedef enum ProbackupSubcmd { @@ -201,6 +202,14 @@ main(int argc, char *argv[]) init_config(&instance_config); PROGRAM_NAME = get_progname(argv[0]); + PROGRAM_FULL_PATH = palloc0(MAXPGPATH); + + if (find_my_exec(argv[0],(char *) PROGRAM_FULL_PATH) < 0) + { + fprintf(stderr, _("%s: could not find own program executable\n"), PROGRAM_NAME); + exit(1); + } + set_pglocale_pgservice(argv[0], "pgscripts"); #if PG_VERSION_NUM >= 110000 diff --git a/src/restore.c b/src/restore.c index 7efd433a..bd8f5b72 100644 --- a/src/restore.c +++ b/src/restore.c @@ -796,7 +796,7 @@ create_recovery_conf(time_t backup_id, fprintf(fp, "restore_command = '%s archive-get -B %s --instance %s " "--wal-file-path %%p --wal-file-name %%f'\n", - PROGRAM_NAME, backup_path, instance_name); + PROGRAM_FULL_PATH, backup_path, instance_name); /* * We've already checked that only one of the four following mutually diff --git a/src/utils/pgut.h b/src/utils/pgut.h index bde5dee1..41a261d2 100644 --- a/src/utils/pgut.h +++ b/src/utils/pgut.h @@ -20,6 +20,7 @@ typedef void (*pgut_atexit_callback)(bool fatal, void *userdata); * pgut client variables and functions */ extern const char *PROGRAM_NAME; +extern const char *PROGRAM_FULL_PATH; extern const char *PROGRAM_VERSION; extern const char *PROGRAM_URL; extern const char *PROGRAM_EMAIL; From 9091bb540504f36aab39aa57103a7e0816c92c7b Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Wed, 3 Apr 2019 18:34:18 +0300 Subject: [PATCH 02/20] tests: additional tests for external directories --- tests/external.py | 173 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 173 insertions(+) diff --git a/tests/external.py b/tests/external.py index 1327402c..e085b153 100644 --- a/tests/external.py +++ b/tests/external.py @@ -1172,6 +1172,179 @@ class ExternalTest(ProbackupTest, unittest.TestCase): # Clean after yourself self.del_test_dir(module_name, fname) + # @unittest.expectedFailure + # @unittest.skip("skip") + def test_external_dir_contain_symlink_on_dir(self): + """ + Check that backup works correctly if external dir is symlink, + symlink pointing to external dir should be followed, + but restored as directory + """ + fname = self.id().split('.')[3] + backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup') + core_dir = os.path.join(self.tmp_path, module_name, fname) + shutil.rmtree(core_dir, ignore_errors=True) + node = self.make_simple_node( + base_dir=os.path.join(module_name, fname, 'node'), + set_replication=True, + initdb_params=['--data-checksums']) + + self.init_pb(backup_dir) + self.add_instance(backup_dir, 'node', node) + node.slow_start() + + external_dir = self.get_tblspace_path(node, 'external_dir') + dir_in_external_dir = os.path.join(external_dir, 'dir') + + node.pgbench_init(scale=3) + + # temp FULL backup + backup_id = self.backup_node( + backup_dir, 'node', node, options=["-j", "4", "--stream"]) + + # fill some directory with data + core_dir = os.path.join(self.tmp_path, module_name, fname) + symlinked_dir = os.path.join(core_dir, 'symlinked') + + self.restore_node( + backup_dir, 'node', node, + data_dir=symlinked_dir, options=["-j", "4"]) + + # drop temp FULL backup + self.delete_pb(backup_dir, 'node', backup_id=backup_id) + + # create symlink to directory in external directory + print(symlinked_dir) + print(dir_in_external_dir) + os.mkdir(external_dir) + os.symlink(symlinked_dir, dir_in_external_dir) + + # FULL backup with external directories + backup_id = self.backup_node( + backup_dir, 'node', node, + options=[ + "-j", "4", "--stream", + "-E", "{0}".format( + external_dir)]) + + pgdata = self.pgdata_content( + node.base_dir, exclude_dirs=['logs']) + + node_restored = self.make_simple_node( + base_dir=os.path.join(module_name, fname, 'node_restored')) + + # RESTORE + node_restored.cleanup() + + external_dir_new = self.get_tblspace_path( + node_restored, 'external_dir') + + self.restore_node( + backup_dir, 'node', node_restored, + options=[ + "-j", "4", "--external-mapping={0}={1}".format( + external_dir, external_dir_new)]) + + pgdata_restored = self.pgdata_content( + node_restored.base_dir, exclude_dirs=['logs']) + + self.compare_pgdata(pgdata, pgdata_restored) + + self.assertEqual( + external_dir, + self.show_pb( + backup_dir, 'node', + backup_id=backup_id)['external-dirs']) + + # Clean after yourself + self.del_test_dir(module_name, fname) + + # @unittest.expectedFailure + # @unittest.skip("skip") + def test_external_dir_contain_symlink_on_file(self): + """ + Check that backup works correctly if external dir is symlink, + symlink pointing to external dir should be followed, + but restored as directory + """ + fname = self.id().split('.')[3] + backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup') + core_dir = os.path.join(self.tmp_path, module_name, fname) + shutil.rmtree(core_dir, ignore_errors=True) + node = self.make_simple_node( + base_dir=os.path.join(module_name, fname, 'node'), + set_replication=True, + initdb_params=['--data-checksums']) + + self.init_pb(backup_dir) + self.add_instance(backup_dir, 'node', node) + node.slow_start() + + external_dir = self.get_tblspace_path(node, 'external_dir') + file_in_external_dir = os.path.join(external_dir, 'file') + + node.pgbench_init(scale=3) + + # temp FULL backup + backup_id = self.backup_node( + backup_dir, 'node', node, options=["-j", "4", "--stream"]) + + # fill some directory with data + core_dir = os.path.join(self.tmp_path, module_name, fname) + symlinked_dir = os.path.join(core_dir, 'symlinked') + + self.restore_node( + backup_dir, 'node', node, + data_dir=symlinked_dir, options=["-j", "4"]) + + # drop temp FULL backup + self.delete_pb(backup_dir, 'node', backup_id=backup_id) + + # create symlink to directory in external directory + src_file = os.path.join(symlinked_dir, 'postgresql.conf') + os.mkdir(external_dir) + os.symlink(src_file, file_in_external_dir) + + # FULL backup with external directories + backup_id = self.backup_node( + backup_dir, 'node', node, + options=[ + "-j", "4", "--stream", + "-E", "{0}".format( + external_dir)]) + + pgdata = self.pgdata_content( + node.base_dir, exclude_dirs=['logs']) + + node_restored = self.make_simple_node( + base_dir=os.path.join(module_name, fname, 'node_restored')) + + # RESTORE + node_restored.cleanup() + + external_dir_new = self.get_tblspace_path( + node_restored, 'external_dir') + + self.restore_node( + backup_dir, 'node', node_restored, + options=[ + "-j", "4", "--external-mapping={0}={1}".format( + external_dir, external_dir_new)]) + + pgdata_restored = self.pgdata_content( + node_restored.base_dir, exclude_dirs=['logs']) + + self.compare_pgdata(pgdata, pgdata_restored) + + self.assertEqual( + external_dir, + self.show_pb( + backup_dir, 'node', + backup_id=backup_id)['external-dirs']) + + # Clean after yourself + self.del_test_dir(module_name, fname) + # @unittest.expectedFailure # @unittest.skip("skip") def test_external_dir_is_tablespace(self): From 6b6491e3b788241cecf5320a7cc565673db17eab Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Wed, 3 Apr 2019 22:25:58 +0300 Subject: [PATCH 03/20] fix symlink handling for external directories --- src/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dir.c b/src/dir.c index dcf1476d..ad118b12 100644 --- a/src/dir.c +++ b/src/dir.c @@ -472,7 +472,7 @@ dir_list_file(parray *files, const char *root, bool exclude, bool omit_symlink, parray_qsort(black_list, BlackListCompare); } - file = pgFileNew(root, external_dir_num ? omit_symlink : false, omit_symlink); + file = pgFileNew(root, external_dir_num ? true : false, external_dir_num); if (file == NULL) return; From 0fbbc22c6af35427b657e4bf1caac8df2b1d33a3 Mon Sep 17 00:00:00 2001 From: Arthur Zakirov Date: Thu, 4 Apr 2019 13:30:30 +0300 Subject: [PATCH 04/20] Little optimization, remove unnecessary find_direct_child() --- src/catalog.c | 40 ++++++---------------------------------- src/delete.c | 5 ++++- src/pg_probackup.h | 1 - 3 files changed, 10 insertions(+), 36 deletions(-) diff --git a/src/catalog.c b/src/catalog.c index 11ce830e..becdbf3b 100644 --- a/src/catalog.c +++ b/src/catalog.c @@ -1026,13 +1026,14 @@ is_prolific(parray *backup_list, pgBackup *target_backup) if (tmp_backup->parent_backup == target_backup->start_time && (tmp_backup->status == BACKUP_STATUS_OK || tmp_backup->status == BACKUP_STATUS_DONE)) + { child_counter++; + if (child_counter > 1) + return true; + } } - if (child_counter > 1) - return true; - else - return false; + return false; } /* @@ -1067,35 +1068,6 @@ find_parent_full_backup(pgBackup *current_backup) return base_full_backup; } -/* - * Find closest child of target_backup. If there are several direct - * offsprings in backup_list, then first win. - */ -pgBackup* -find_direct_child(parray *backup_list, pgBackup *target_backup) -{ - int i; - - for (i = 0; i < parray_num(backup_list); i++) - { - pgBackup *tmp_backup = (pgBackup *) parray_get(backup_list, i); - - if (tmp_backup->backup_mode == BACKUP_MODE_FULL) - continue; - - /* Consider only OK and DONE children */ - if (tmp_backup->parent_backup == target_backup->start_time && - (tmp_backup->status == BACKUP_STATUS_OK || - tmp_backup->status == BACKUP_STATUS_DONE)) - { - return tmp_backup; - } - } - elog(WARNING, "Failed to find a direct child for backup %s", - base36enc(target_backup->start_time)); - return NULL; -} - /* * Interate over parent chain and look for any problems. * Return 0 if chain is broken. @@ -1204,4 +1176,4 @@ get_backup_index_number(parray *backup_list, pgBackup *backup) } elog(WARNING, "Failed to find backup %s", base36enc(backup->start_time)); return -1; -} \ No newline at end of file +} diff --git a/src/delete.c b/src/delete.c index 535c7a5f..14c0f1bc 100644 --- a/src/delete.c +++ b/src/delete.c @@ -129,6 +129,9 @@ int do_retention(void) bool retention_is_set = false; /* At least one retention policy is set */ bool backup_list_is_empty = false; + backup_deleted = false; + backup_merged = false; + /* Get a complete list of backups. */ backup_list = catalog_get_backup_list(INVALID_BACKUP_ID); @@ -411,7 +414,7 @@ do_retention_merge(parray *backup_list, parray *to_keep_list, parray *to_purge_l continue; } - /* FULL backup in purge list, thanks to sparsing of keep_list current backup is + /* FULL backup in purge list, thanks to sparsing of keep_list current backup is * final target for merge, but there could be intermediate incremental * backups from purge_list. */ diff --git a/src/pg_probackup.h b/src/pg_probackup.h index 85c1fd53..ff4ae866 100644 --- a/src/pg_probackup.h +++ b/src/pg_probackup.h @@ -501,7 +501,6 @@ extern int pgBackupCompareId(const void *f1, const void *f2); extern int pgBackupCompareIdDesc(const void *f1, const void *f2); extern int pgBackupCompareIdEqual(const void *l, const void *r); -extern pgBackup* find_direct_child(parray *backup_list, pgBackup *target_backup); extern pgBackup* find_parent_full_backup(pgBackup *current_backup); extern int scan_parent_chain(pgBackup *current_backup, pgBackup **result_backup); extern bool is_parent(time_t parent_backup_time, pgBackup *child_backup, bool inclusive); From 9a5c8074dbabf5afc0eb2396be65636e8f1d5e44 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Thu, 4 Apr 2019 14:39:59 +0300 Subject: [PATCH 05/20] tests: fix test_delta_archive --- tests/delta.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tests/delta.py b/tests/delta.py index d5c3a03f..e4c8305d 100644 --- a/tests/delta.py +++ b/tests/delta.py @@ -352,17 +352,11 @@ class DeltaTest(ProbackupTest, unittest.TestCase): node = self.make_simple_node( base_dir=os.path.join(module_name, fname, 'node'), set_replication=True, - initdb_params=['--data-checksums'], - pg_options={ - 'wal_level': 'replica', - 'max_wal_senders': '2', - 'checkpoint_timeout': '30s' - } - ) + initdb_params=['--data-checksums']) self.init_pb(backup_dir) self.add_instance(backup_dir, 'node', node) - # self.set_archiving(backup_dir, 'node', node) + self.set_archiving(backup_dir, 'node', node) node.slow_start() # FULL BACKUP @@ -372,8 +366,7 @@ class DeltaTest(ProbackupTest, unittest.TestCase): "md5(i::text)::tsvector as tsvector from generate_series(0,1) i") full_result = node.execute("postgres", "SELECT * FROM t_heap") full_backup_id = self.backup_node( - backup_dir, 'node', node, - backup_type='full', options=['--stream']) + backup_dir, 'node', node, backup_type='full') # delta BACKUP node.safe_psql( @@ -382,8 +375,7 @@ class DeltaTest(ProbackupTest, unittest.TestCase): "md5(i::text)::tsvector as tsvector from generate_series(0,2) i") delta_result = node.execute("postgres", "SELECT * FROM t_heap") delta_backup_id = self.backup_node( - backup_dir, 'node', node, - backup_type='delta', options=['--stream']) + backup_dir, 'node', node, backup_type='delta') # Drop Node node.cleanup() From 514a01dc15e252f1dcf2cf3c0cc2134b65e78bcf Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Thu, 4 Apr 2019 18:36:35 +0300 Subject: [PATCH 06/20] another symlink fix --- src/backup.c | 4 ++-- src/dir.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/backup.c b/src/backup.c index 7b99e1af..e7c20e2d 100644 --- a/src/backup.c +++ b/src/backup.c @@ -659,8 +659,8 @@ do_backup_instance(void) * https://github.com/postgrespro/pg_probackup/issues/48 */ - if (parray_num(backup_files_list) == 0) - elog(ERROR, "PGDATA is empty. Either it was concurrently deleted or " + if (parray_num(backup_files_list) < 100) + elog(ERROR, "PGDATA is almost empty. Either it was concurrently deleted or " "pg_probackup do not possess sufficient permissions to list PGDATA content"); /* diff --git a/src/dir.c b/src/dir.c index ad118b12..ec32d877 100644 --- a/src/dir.c +++ b/src/dir.c @@ -472,7 +472,7 @@ dir_list_file(parray *files, const char *root, bool exclude, bool omit_symlink, parray_qsort(black_list, BlackListCompare); } - file = pgFileNew(root, external_dir_num ? true : false, external_dir_num); + file = pgFileNew(root, omit_symlink, external_dir_num); if (file == NULL) return; From 1746b549780b0fde570b935d54dedd3be96c640c Mon Sep 17 00:00:00 2001 From: Arthur Zakirov Date: Thu, 4 Apr 2019 20:26:36 +0300 Subject: [PATCH 07/20] Make changes for C89 --- src/catalog.c | 2 +- src/merge.c | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/catalog.c b/src/catalog.c index becdbf3b..b0e6a5e7 100644 --- a/src/catalog.c +++ b/src/catalog.c @@ -476,7 +476,7 @@ pgBackupCreateDir(pgBackup *backup) parray *external_list; external_list = make_external_directory_list(backup->external_dir_str); - for (int i = 0; i < parray_num(external_list); i++) + for (i = 0; i < parray_num(external_list); i++) { char temp[MAXPGPATH]; /* Numeration of externaldirs starts with 1 */ diff --git a/src/merge.c b/src/merge.c index 3ac95ea2..bd365408 100644 --- a/src/merge.c +++ b/src/merge.c @@ -673,10 +673,12 @@ merge_files(void *arg) static void remove_dir_with_files(const char *path) { - parray *files = parray_new(); + parray *files = parray_new(); + int i; + dir_list_file(files, path, true, true, true, 0); parray_qsort(files, pgFileComparePathDesc); - for (int i = 0; i < parray_num(files); i++) + for (i = 0; i < parray_num(files); i++) { pgFile *file = (pgFile *) parray_get(files, i); @@ -689,9 +691,11 @@ remove_dir_with_files(const char *path) static int get_external_index(const char *key, const parray *list) { + int i; + if (!list) /* Nowhere to search */ return -1; - for (int i = 0; i < parray_num(list); i++) + for (i = 0; i < parray_num(list); i++) { if (strcmp(key, parray_get(list, i)) == 0) return i + 1; @@ -704,11 +708,12 @@ static void reorder_external_dirs(pgBackup *to_backup, parray *to_external, parray *from_external) { - char externaldir_template[MAXPGPATH]; + char externaldir_template[MAXPGPATH]; + int i; pgBackupGetPath(to_backup, externaldir_template, lengthof(externaldir_template), EXTERNAL_DIR); - for (int i = 0; i < parray_num(to_external); i++) + for (i = 0; i < parray_num(to_external); i++) { int from_num = get_external_index(parray_get(to_external, i), from_external); From 049f81830e27e2b63b010abaf87b463b5853d535 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Fri, 5 Apr 2019 00:57:06 +0300 Subject: [PATCH 08/20] minor fix for del-instance --- src/delete.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/delete.c b/src/delete.c index 14c0f1bc..63c7aac3 100644 --- a/src/delete.c +++ b/src/delete.c @@ -835,8 +835,9 @@ do_delete_instance(void) if (rmdir(backup_instance_path) != 0) elog(ERROR, "can't remove \"%s\": %s", backup_instance_path, strerror(errno)); + if (rmdir(arclog_path) != 0) - elog(ERROR, "can't remove \"%s\": %s", backup_instance_path, + elog(ERROR, "can't remove \"%s\": %s", arclog_path, strerror(errno)); elog(INFO, "Instance '%s' successfully deleted", instance_name); From 6b49402e9fe98ad88b62c13646972db907fee293 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Mon, 8 Apr 2019 12:03:05 +0300 Subject: [PATCH 09/20] tests: added delete.DeleteTest.test_del_instance_archive --- tests/delete.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/delete.py b/tests/delete.py index 71919c86..8209e379 100644 --- a/tests/delete.py +++ b/tests/delete.py @@ -55,6 +55,36 @@ class DeleteTest(ProbackupTest, unittest.TestCase): # Clean after yourself self.del_test_dir(module_name, fname) + # @unittest.skip("skip") + # @unittest.expectedFailure + def test_del_instance_archive(self): + """delete full backups""" + fname = self.id().split('.')[3] + node = self.make_simple_node( + base_dir=os.path.join(module_name, fname, 'node'), + initdb_params=['--data-checksums']) + + backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup') + self.init_pb(backup_dir) + self.add_instance(backup_dir, 'node', node) + self.set_archiving(backup_dir, 'node', node) + node.slow_start() + + # full backup + self.backup_node(backup_dir, 'node', node) + + # full backup + self.backup_node(backup_dir, 'node', node) + + # restore + self.restore_node( + backup_dir, 'node', node) + + self.del_instance(backup_dir, 'node') + + # Clean after yourself + #self.del_test_dir(module_name, fname) + # @unittest.skip("skip") # @unittest.expectedFailure def test_delete_archive_mix_compress_and_non_compressed_segments(self): From 8df3a8cf4be5ece19da66d55ff740d621af818b2 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Mon, 8 Apr 2019 12:09:09 +0300 Subject: [PATCH 10/20] tests: minor fixes --- tests/delete.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/delete.py b/tests/delete.py index 8209e379..1359aaf0 100644 --- a/tests/delete.py +++ b/tests/delete.py @@ -76,10 +76,15 @@ class DeleteTest(ProbackupTest, unittest.TestCase): # full backup self.backup_node(backup_dir, 'node', node) + node.cleanup() + + # restore self.restore_node( backup_dir, 'node', node) + node.slow_start() + self.del_instance(backup_dir, 'node') # Clean after yourself From 64a7fac7a2d4fed3225c3574a9656d63e963394a Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Mon, 8 Apr 2019 13:54:29 +0300 Subject: [PATCH 11/20] tests: minor fixes for del_instance --- tests/delete.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/delete.py b/tests/delete.py index 1359aaf0..ac5a4284 100644 --- a/tests/delete.py +++ b/tests/delete.py @@ -76,19 +76,16 @@ class DeleteTest(ProbackupTest, unittest.TestCase): # full backup self.backup_node(backup_dir, 'node', node) - node.cleanup() - - # restore - self.restore_node( - backup_dir, 'node', node) - + node.cleanup() + self.restore_node(backup_dir, 'node', node) node.slow_start() + # Delete instance self.del_instance(backup_dir, 'node') # Clean after yourself - #self.del_test_dir(module_name, fname) + self.del_test_dir(module_name, fname) # @unittest.skip("skip") # @unittest.expectedFailure From df0cd6507b37c6f048ab1ac086a87f85b3db3c23 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Mon, 8 Apr 2019 17:39:48 +0300 Subject: [PATCH 12/20] bugfix: it is not a fatal error to backup a file, whose size is not multiple of BLCKSZ --- src/data.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/data.c b/src/data.c index 9cad2eab..32057e79 100644 --- a/src/data.c +++ b/src/data.c @@ -577,7 +577,7 @@ backup_data_file(backup_files_arg* arguments, if (file->size % BLCKSZ != 0) { fclose(in); - elog(ERROR, "File: %s, invalid file size %zu", file->path, file->size); + elog(WARNING, "File: %s, invalid file size %zu", file->path, file->size); } /* From 1ff905989efe5c2d13c2ec6a749e272fe1877130 Mon Sep 17 00:00:00 2001 From: Victor Spirin Date: Tue, 9 Apr 2019 18:05:36 +0300 Subject: [PATCH 13/20] Replaced the function strftime with pg_strftime for logs in Windows --- gen_probackup_project.pl | 1 + src/utils/logger.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/gen_probackup_project.pl b/gen_probackup_project.pl index 999a0d55..88a1a948 100644 --- a/gen_probackup_project.pl +++ b/gen_probackup_project.pl @@ -182,6 +182,7 @@ sub build_pgprobackup $probackup->AddFile("$pgsrc/src/bin/pg_rewind/datapagemap.c"); $probackup->AddFile("$pgsrc/src/interfaces/libpq/pthread-win32.c"); + $probackup->AddFile("$pgsrc/src/timezone/strftime.c"); $probackup->AddIncludeDir("$pgsrc/src/bin/pg_basebackup"); $probackup->AddIncludeDir("$pgsrc/src/bin/pg_rewind"); diff --git a/src/utils/logger.c b/src/utils/logger.c index fb100526..10f23105 100644 --- a/src/utils/logger.c +++ b/src/utils/logger.c @@ -470,7 +470,11 @@ logfile_getname(const char *format, time_t timestamp) len = strlen(filename); /* Treat log_filename as a strftime pattern */ +#ifdef WIN32 + if (pg_strftime(filename + len, MAXPGPATH - len, format, tm) <= 0) +#else if (strftime(filename + len, MAXPGPATH - len, format, tm) <= 0) +#endif elog_stderr(ERROR, "strftime(%s) failed: %s", format, strerror(errno)); return filename; From bc52046ac65ab47d1c0fd045920e8bf2cb52177b Mon Sep 17 00:00:00 2001 From: Arthur Zakirov Date: Wed, 10 Apr 2019 17:02:00 +0300 Subject: [PATCH 14/20] Do not wait for the next segment --- src/parsexlog.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parsexlog.c b/src/parsexlog.c index d9d169d4..87323431 100644 --- a/src/parsexlog.c +++ b/src/parsexlog.c @@ -1191,8 +1191,8 @@ XLogThreadWorker(void *arg) * Consider thread_arg->endSegNo and thread_arg->endpoint only if * they are valid. */ - xlogreader->ReadRecPtr == thread_arg->endpoint && - nextSegNo > thread_arg->endSegNo) + xlogreader->ReadRecPtr >= thread_arg->endpoint && + nextSegNo >= thread_arg->endSegNo) break; } From 8240bc2dc4c23be4969d3c7e87adc670deb2faae Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Wed, 10 Apr 2019 22:27:07 +0300 Subject: [PATCH 15/20] tests: fix archive.ArchiveTest.test_replica_archive --- tests/archive.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/archive.py b/tests/archive.py index 51e46ea0..685e6986 100644 --- a/tests/archive.py +++ b/tests/archive.py @@ -422,10 +422,10 @@ class ArchiveTest(ProbackupTest, unittest.TestCase): set_replication=True, initdb_params=['--data-checksums'], pg_options={ - 'max_wal_senders': '2', 'archive_timeout': '10s', - 'max_wal_size': '1GB'} - ) + 'checkpoint_timeout': '30s', + 'max_wal_size': '16MB'}) + self.init_pb(backup_dir) # ADD INSTANCE 'MASTER' self.add_instance(backup_dir, 'master', master) From af340b982c14ed7a0fd1b7928d69469f8a28a7e9 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Wed, 10 Apr 2019 23:43:30 +0300 Subject: [PATCH 16/20] tests: added delta.DeltaTest.test_delta_backup_from_past --- tests/delta.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/tests/delta.py b/tests/delta.py index e4c8305d..262c7fdc 100644 --- a/tests/delta.py +++ b/tests/delta.py @@ -711,14 +711,12 @@ class DeltaTest(ProbackupTest, unittest.TestCase): 1, 0, "Expecting Error because we are connecting to deleted database" "\n Output: {0} \n CMD: {1}".format( - repr(self.output), self.cmd) - ) + repr(self.output), self.cmd)) except QueryException as e: self.assertTrue( 'FATAL: database "db1" does not exist' in e.message, '\n Unexpected Error Message: {0}\n CMD: {1}'.format( - repr(e.message), self.cmd) - ) + repr(e.message), self.cmd)) # Clean after yourself self.del_test_dir(module_name, fname) @@ -1307,3 +1305,53 @@ class DeltaTest(ProbackupTest, unittest.TestCase): # Clean after yourself self.del_test_dir(module_name, fname) + + def test_delta_backup_from_past(self): + """ + make node, take FULL stream backup, take DELTA stream backup, + restore FULL backup, try to take second DELTA stream backup + """ + fname = self.id().split('.')[3] + node = self.make_simple_node( + base_dir=os.path.join(module_name, fname, 'node'), + initdb_params=['--data-checksums']) + + backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup') + self.init_pb(backup_dir) + self.add_instance(backup_dir, 'node', node) + node.slow_start() + + backup_id = self.backup_node( + backup_dir, 'node', node, options=['--stream']) + + node.pgbench_init(scale=3) + + # First DELTA + self.backup_node( + backup_dir, 'node', node, + backup_type='delta', options=['--stream']) + + # Restore FULL backup + node.cleanup() + self.restore_node(backup_dir, 'node', node, backup_id=backup_id) + node.slow_start() + + # Second DELTA backup + try: + self.backup_node( + backup_dir, 'node', node, + backup_type='delta', options=['--stream']) + # we should die here because exception is what we expect to happen + self.assertEqual( + 1, 0, + "Expecting Error because we are backing up an instance from the past" + "\n Output: {0} \n CMD: {1}".format( + repr(self.output), self.cmd)) + except QueryException as e: + self.assertTrue( + 'Insert error message' in e.message, + '\n Unexpected Error Message: {0}\n CMD: {1}'.format( + repr(e.message), self.cmd)) + + # Clean after yourself + self.del_test_dir(module_name, fname) From 9c83463f914646d7c1323431058efb3092235706 Mon Sep 17 00:00:00 2001 From: Arthur Zakirov Date: Thu, 11 Apr 2019 12:35:55 +0300 Subject: [PATCH 17/20] Consider DELETING status within merge_backups() --- src/merge.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/merge.c b/src/merge.c index bd365408..8e91babb 100644 --- a/src/merge.c +++ b/src/merge.c @@ -81,8 +81,8 @@ do_merge(time_t backup_id) /* It is possible that previous merging was interrupted */ backup->status != BACKUP_STATUS_MERGING && backup->status != BACKUP_STATUS_DELETING) - elog(ERROR, "Backup %s has status: %s", - base36enc(backup->start_time), status2str(backup->status)); + elog(ERROR, "Backup %s has status: %s", + base36enc(backup->start_time), status2str(backup->status)); if (backup->backup_mode == BACKUP_MODE_FULL) elog(ERROR, "Backup %s is full backup", @@ -109,10 +109,8 @@ do_merge(time_t backup_id) if (full_backup->status != BACKUP_STATUS_OK && /* It is possible that previous merging was interrupted */ full_backup->status != BACKUP_STATUS_MERGING) - elog(ERROR, "Backup %s has status: %s", - base36enc(full_backup->start_time), status2str(full_backup->status)); - - //Assert(full_backup_idx != dest_backup_idx); + elog(ERROR, "Backup %s has status: %s", + base36enc(full_backup->start_time), status2str(full_backup->status)); /* form merge list */ while(dest_backup->parent_backup_link) @@ -122,8 +120,8 @@ do_merge(time_t backup_id) /* It is possible that previous merging was interrupted */ dest_backup->status != BACKUP_STATUS_MERGING && dest_backup->status != BACKUP_STATUS_DELETING) - elog(ERROR, "Backup %s has status: %s", - base36enc(dest_backup->start_time), status2str(dest_backup->status)); + elog(ERROR, "Backup %s has status: %s", + base36enc(dest_backup->start_time), status2str(dest_backup->status)); parray_append(merge_list, dest_backup); dest_backup = dest_backup->parent_backup_link; @@ -205,7 +203,8 @@ merge_backups(pgBackup *to_backup, pgBackup *from_backup) * BACKUP_STATUS_MERGING status. */ Assert(from_backup->status == BACKUP_STATUS_OK || - from_backup->status == BACKUP_STATUS_MERGING); + from_backup->status == BACKUP_STATUS_MERGING || + from_backup->status == BACKUP_STATUS_DELETING); pgBackupValidate(from_backup); if (from_backup->status == BACKUP_STATUS_CORRUPT) elog(ERROR, "Interrupt merging"); From 8aeddc98924cec57b1d4ad658a9c6181f9c4ee93 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Thu, 11 Apr 2019 16:33:16 +0300 Subject: [PATCH 18/20] tests: minor fixes --- tests/__init__.py | 2 +- tests/expected/option_help.out | 14 +++++++++----- tests/merge.py | 4 +++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 033ce535..bc787ca4 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -18,7 +18,7 @@ def load_tests(loader, tests, pattern): # suite.addTests(loader.loadTestsFromModule(cfs_backup)) # suite.addTests(loader.loadTestsFromModule(cfs_restore)) # suite.addTests(loader.loadTestsFromModule(cfs_validate_backup)) -# suite.addTests(loader.loadTestsFromModule(logging)) + suite.addTests(loader.loadTestsFromModule(logging)) suite.addTests(loader.loadTestsFromModule(compression)) suite.addTests(loader.loadTestsFromModule(delete)) suite.addTests(loader.loadTestsFromModule(delta)) diff --git a/tests/expected/option_help.out b/tests/expected/option_help.out index a83c3905..cac631c1 100644 --- a/tests/expected/option_help.out +++ b/tests/expected/option_help.out @@ -39,7 +39,7 @@ pg_probackup - utility to manage backup/recovery of PostgreSQL database. [--log-directory=log-directory] [--log-rotation-size=log-rotation-size] [--log-rotation-age=log-rotation-age] - [--delete-expired] [--delete-wal] + [--delete-expired] [--delete-wal] [--merge-expired] [--retention-redundancy=retention-redundancy] [--retention-window=retention-window] [--compress] @@ -51,16 +51,19 @@ pg_probackup - utility to manage backup/recovery of PostgreSQL database. [--master-port=port] [--master-user=user_name] [--replica-timeout=timeout] [--skip-block-validation] + [--external-dirs=external-directory-path] pg_probackup restore -B backup-path --instance=instance_name - [-D pgdata-path] [-i backup-id] [--progress] + [-D pgdata-path] [-i backup-id] [-j num-threads] [--time=time|--xid=xid|--lsn=lsn [--inclusive=boolean]] - [--timeline=timeline] [-T OLDDIR=NEWDIR] + [--timeline=timeline] [-T OLDDIR=NEWDIR] [--progress] + [--external-mapping=OLDDIR=NEWDIR] [--immediate] [--recovery-target-name=target-name] [--recovery-target-action=pause|promote|shutdown] [--restore-as-replica] [--no-validate] [--skip-block-validation] + [--skip-external-dirs] pg_probackup validate -B backup-path [--instance=instance_name] [-i backup-id] [--progress] [-j num-threads] @@ -74,10 +77,11 @@ pg_probackup - utility to manage backup/recovery of PostgreSQL database. [--format=format] pg_probackup delete -B backup-path --instance=instance_name - [--wal] [-i backup-id | --expired] + [--wal] [-i backup-id | --expired | --merge-expired] + [--dry-run] pg_probackup merge -B backup-path --instance=instance_name - -i backup-id + -i backup-id [--progress] [-j num-threads] pg_probackup add-instance -B backup-path -D pgdata-path --instance=instance_name diff --git a/tests/merge.py b/tests/merge.py index 4608a34c..ee8cf2fb 100644 --- a/tests/merge.py +++ b/tests/merge.py @@ -1197,7 +1197,7 @@ class MergeTest(ProbackupTest, unittest.TestCase): def test_continue_failed_merge_2(self): """ - Check that failed MERGE on delete can`t be continued + Check that failed MERGE on delete can be continued """ fname = self.id().split('.')[3] backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup') @@ -1253,6 +1253,8 @@ class MergeTest(ProbackupTest, unittest.TestCase): backup_id_deleted = self.show_pb(backup_dir, "node")[1]["id"] + # TODO check that full backup has meta info is equal to DELETTING + # Try to continue failed MERGE self.merge_backup(backup_dir, "node", backup_id) # Clean after yourself From 06add7d59f6379d320b2e8a06a1a182eed88cb7b Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Thu, 11 Apr 2019 17:00:44 +0300 Subject: [PATCH 19/20] tests: minor fix for replica.ReplicaTest.test_replica_archive_page_backup --- tests/replica.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/replica.py b/tests/replica.py index 9ab49a6e..358d97d7 100644 --- a/tests/replica.py +++ b/tests/replica.py @@ -141,10 +141,10 @@ class ReplicaTest(ProbackupTest, unittest.TestCase): set_replication=True, initdb_params=['--data-checksums'], pg_options={ - 'wal_level': 'replica', - 'max_wal_senders': '2', - 'archive_timeout': '10s'} - ) + 'archive_timeout': '10s', + 'checkpoint_timeout': '30s', + 'max_wal_size': '16MB'}) + self.init_pb(backup_dir) self.add_instance(backup_dir, 'master', master) self.set_archiving(backup_dir, 'master', master) From 6d8bad7293ebcf5c5232da888d54bfb0b15c0213 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Thu, 11 Apr 2019 19:14:33 +0300 Subject: [PATCH 20/20] bugfix: explicitly forbid incremental backing up instance from the past --- src/backup.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/backup.c b/src/backup.c index e7c20e2d..f4e93042 100644 --- a/src/backup.c +++ b/src/backup.c @@ -582,6 +582,15 @@ do_backup_instance(void) strlen(" with pg_probackup")); pg_start_backup(label, smooth_checkpoint, ¤t); + /* For incremental backup check that start_lsn is not from the past */ + if (current.backup_mode != BACKUP_MODE_FULL && + prev_backup->start_lsn > current.start_lsn) + elog(ERROR, "Current START LSN %X/%X is lower than START LSN %X/%X of previous backup %s. " + "It may indicate that we are trying to backup PostgreSQL instance from the past.", + (uint32) (current.start_lsn >> 32), (uint32) (current.start_lsn), + (uint32) (prev_backup->start_lsn >> 32), (uint32) (prev_backup->start_lsn), + base36enc(prev_backup->start_time)); + /* Update running backup meta with START LSN */ write_backup(¤t);