From 1fd1c8d35c8a02ef6f3d443080cb46f0bbf640fb Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Sun, 28 Feb 2021 23:20:03 +0300 Subject: [PATCH 1/8] [Issue #327] add test coverage --- tests/archive.py | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/tests/archive.py b/tests/archive.py index a7089395..ad2320a3 100644 --- a/tests/archive.py +++ b/tests/archive.py @@ -983,6 +983,83 @@ class ArchiveTest(ProbackupTest, unittest.TestCase): # Clean after yourself self.del_test_dir(module_name, fname) + + # @unittest.expectedFailure + # @unittest.skip("skip") + def test_concurrent_archiving(self): + """ + Concurrent archiving from master, replica and cascade replica + https://github.com/postgrespro/pg_probackup/issues/327 + + For PG >= 11 it is expected to pass this test + """ + + if self.pg_config_version < self.version_to_num('11.0'): + return unittest.skip('You need PostgreSQL >= 11 for this test') + + fname = self.id().split('.')[3] + backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup') + master = self.make_simple_node( + base_dir=os.path.join(module_name, fname, 'master'), + set_replication=True, + initdb_params=['--data-checksums'], + pg_options={'autovacuum': 'off'}) + + self.init_pb(backup_dir) + self.add_instance(backup_dir, 'node', master) + self.set_archiving(backup_dir, 'node', master, replica=True) + master.slow_start() + + master.pgbench_init(scale=10) + + # TAKE FULL ARCHIVE BACKUP FROM MASTER + self.backup_node(backup_dir, 'node', master) + + # Settings for Replica + replica = self.make_simple_node( + base_dir=os.path.join(module_name, fname, 'replica')) + replica.cleanup() + self.restore_node(backup_dir, 'node', replica) + + self.set_replica(master, replica, synchronous=True) + self.set_archiving(backup_dir, 'node', replica, replica=True) + self.set_auto_conf(replica, {'port': replica.port}) + replica.slow_start(replica=True) + + # create cascade replicas + replica1 = self.make_simple_node( + base_dir=os.path.join(module_name, fname, 'replica1')) + replica1.cleanup() + + # Settings for casaced replica + self.restore_node(backup_dir, 'node', replica1) + self.set_replica(replica, replica1, synchronous=False) + self.set_auto_conf(replica1, {'port': replica1.port}) + replica1.slow_start(replica=True) + + # Take full backup from master + self.backup_node(backup_dir, 'node', master) + + pgbench = master.pgbench( + stdout=subprocess.PIPE, stderr=subprocess.STDOUT, + options=['-T', '30', '-c', '1']) + + # Take several incremental backups from master + self.backup_node(backup_dir, 'node', master, backup_type='page', options=['--no-validate']) + + self.backup_node(backup_dir, 'node', master, backup_type='page', options=['--no-validate']) + + pgbench.wait() + pgbench.stdout.close() + + with open(os.path.join(replica1.logs_dir, 'postgresql.log'), 'r') as f: + log_content = f.read() + + self.assertNotIn('different checksum', log_content) + + # Clean after yourself + self.del_test_dir(module_name, fname) + # @unittest.expectedFailure # @unittest.skip("skip") def test_archive_pg_receivexlog(self): From 2974c03e7df0b5279d94eb24903a2e1b2679211e Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Sun, 28 Feb 2021 23:22:35 +0300 Subject: [PATCH 2/8] tests: minor fix --- tests/archive.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/archive.py b/tests/archive.py index ad2320a3..ac1b2a0d 100644 --- a/tests/archive.py +++ b/tests/archive.py @@ -1052,9 +1052,16 @@ class ArchiveTest(ProbackupTest, unittest.TestCase): pgbench.wait() pgbench.stdout.close() + with open(os.path.join(master.logs_dir, 'postgresql.log'), 'r') as f: + log_content = f.read() + self.assertNotIn('different checksum', log_content) + + with open(os.path.join(replica.logs_dir, 'postgresql.log'), 'r') as f: + log_content = f.read() + self.assertNotIn('different checksum', log_content) + with open(os.path.join(replica1.logs_dir, 'postgresql.log'), 'r') as f: log_content = f.read() - self.assertNotIn('different checksum', log_content) # Clean after yourself From e0dfc9c89e26e9b9e18826dbc0ac36cac7d33359 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Fri, 5 Mar 2021 17:01:15 +0300 Subject: [PATCH 3/8] [Issue #342] check error when writing to pg_probackup.conf and sync it to disk before rename --- src/configure.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/configure.c b/src/configure.c index 1aae3df1..cf172242 100644 --- a/src/configure.c +++ b/src/configure.c @@ -299,6 +299,7 @@ do_set_config(bool missing_ok) for (i = 0; instance_options[i].type; i++) { + int rc = 0; ConfigOption *opt = &instance_options[i]; char *value; @@ -319,13 +320,25 @@ do_set_config(bool missing_ok) } if (strchr(value, ' ')) - fprintf(fp, "%s = '%s'\n", opt->lname, value); + rc = fprintf(fp, "%s = '%s'\n", opt->lname, value); else - fprintf(fp, "%s = %s\n", opt->lname, value); + rc = fprintf(fp, "%s = %s\n", opt->lname, value); + + if (rc < 0) + elog(ERROR, "Cannot write to configuration file: \"%s\"", path_temp); + pfree(value); } - fclose(fp); + if (ferror(fp) || fflush(fp)) + elog(ERROR, "Cannot write to configuration file: \"%s\"", path_temp); + + if (fclose(fp)) + elog(ERROR, "Cannot close configuration file: \"%s\"", path_temp); + + if (fio_sync(path_temp, FIO_LOCAL_HOST) != 0) + elog(ERROR, "Failed to sync temp configuration file \"%s\": %s", + path_temp, strerror(errno)); if (rename(path_temp, path) < 0) { From 916ffb6d91b162b018cf29ce755fa5235241e8aa Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Sun, 7 Mar 2021 18:34:45 +0300 Subject: [PATCH 4/8] [Issue #343] redundant errno check when reading "/backup_dir/backups/instance_name" directory --- src/catalog.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/catalog.c b/src/catalog.c index 15386d42..995dd8fa 100644 --- a/src/catalog.c +++ b/src/catalog.c @@ -972,17 +972,11 @@ catalog_get_backup_list(const char *instance_name, time_t requested_backup_id) continue; } parray_append(backups, backup); - - if (errno && errno != ENOENT) - { - elog(WARNING, "cannot read data directory \"%s\": %s", - data_ent->d_name, strerror(errno)); - goto err_proc; - } } + if (errno) { - elog(WARNING, "cannot read backup root directory \"%s\": %s", + elog(WARNING, "Cannot read backup root directory \"%s\": %s", backup_instance_path, strerror(errno)); goto err_proc; } From 79a98911a40ea7bfcade730d152e1903e53659c3 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Sat, 20 Mar 2021 22:23:28 +0300 Subject: [PATCH 5/8] fix achive_timeout=0 due to old bug from 2.0.x version --- src/backup.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/backup.c b/src/backup.c index ca8baa77..be0147e9 100644 --- a/src/backup.c +++ b/src/backup.c @@ -1572,8 +1572,13 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn, */ if (pg_stop_backup_is_sent && !in_cleanup) { + int timeout = ARCHIVE_TIMEOUT_DEFAULT; res = NULL; + /* kludge against some old bug in archive_timeout. TODO: remove in 3.0.0 */ + if (instance_config.archive_timeout > 0) + timeout = instance_config.archive_timeout; + while (1) { if (!PQconsumeInput(conn)) @@ -1598,11 +1603,10 @@ pg_stop_backup(pgBackup *backup, PGconn *pg_startbackup_conn, * If postgres haven't answered in archive_timeout seconds, * send an interrupt. */ - if (pg_stop_backup_timeout > instance_config.archive_timeout) + if (pg_stop_backup_timeout > timeout) { pgut_cancel(conn); - elog(ERROR, "pg_stop_backup doesn't answer in %d seconds, cancel it", - instance_config.archive_timeout); + elog(ERROR, "pg_stop_backup doesn't answer in %d seconds, cancel it", timeout); } } else From 69754925b2a952c65f743eaa6fbd41ff69beca11 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Sat, 20 Mar 2021 23:26:19 +0300 Subject: [PATCH 6/8] [Issue #348] added tests.compatibility.CompatibilityTest.test_compatibility_tablespace --- tests/compatibility.py | 82 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/compatibility.py b/tests/compatibility.py index 18f60150..da9d72f8 100644 --- a/tests/compatibility.py +++ b/tests/compatibility.py @@ -1419,3 +1419,85 @@ class CompatibilityTest(ProbackupTest, unittest.TestCase): # Clean after yourself self.del_test_dir(module_name, fname) + + # @unittest.skip("skip") + def test_compatibility_tablespace(self): + """ + https://github.com/postgrespro/pg_probackup/issues/348 + """ + fname = self.id().split('.')[3] + node = self.make_simple_node( + base_dir=os.path.join(module_name, fname, 'node'), + set_replication=True, + 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, old_binary=True) + node.slow_start() + + backup_id = self.backup_node( + backup_dir, 'node', node, backup_type="full", + options=["-j", "4", "--stream"], old_binary=True) + + tblspace_old_path = self.get_tblspace_path(node, 'tblspace_old') + + self.create_tblspace_in_node( + node, 'tblspace', + tblspc_path=tblspace_old_path) + + node.safe_psql( + "postgres", + "create table t_heap_lame tablespace tblspace " + "as select 1 as id, md5(i::text) as text, " + "md5(repeat(i::text,10))::tsvector as tsvector " + "from generate_series(0,1000) i") + + tblspace_new_path = self.get_tblspace_path(node, 'tblspace_new') + + node_restored = self.make_simple_node( + base_dir=os.path.join(module_name, fname, 'node_restored')) + node_restored.cleanup() + + try: + self.restore_node( + backup_dir, 'node', node_restored, + options=[ + "-j", "4", + "-T", "{0}={1}".format( + tblspace_old_path, tblspace_new_path)]) + # we should die here because exception is what we expect to happen + self.assertEqual( + 1, 0, + "Expecting Error because tablespace mapping is incorrect" + "\n Output: {0} \n CMD: {1}".format( + repr(self.output), self.cmd)) + except ProbackupException as e: + self.assertIn( + 'ERROR: Backup {0} has no tablespaceses, ' + 'nothing to remap'.format(backup_id), + e.message, + '\n Unexpected Error Message: {0}\n CMD: {1}'.format( + repr(e.message), self.cmd)) + + self.backup_node( + backup_dir, 'node', node, backup_type="delta", + options=["-j", "4", "--stream"], old_binary=True) + + self.restore_node( + backup_dir, 'node', node_restored, + options=[ + "-j", "4", + "-T", "{0}={1}".format( + tblspace_old_path, tblspace_new_path)]) + + if self.paranoia: + pgdata = self.pgdata_content(node.data_dir) + + if self.paranoia: + pgdata_restored = self.pgdata_content(node_restored.data_dir) + self.compare_pgdata(pgdata, pgdata_restored) + + # Clean after yourself + self.del_test_dir(module_name, fname) From 58d02b27071980023bf7f2157b577b31e62d96c0 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Sat, 20 Mar 2021 23:27:34 +0300 Subject: [PATCH 7/8] [Issue #248] Use crc32 when validating backup of version "2.0.21 < version <= 2.0.25" --- src/validate.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/validate.c b/src/validate.c index 21900c8e..a91fe817 100644 --- a/src/validate.c +++ b/src/validate.c @@ -716,6 +716,8 @@ validate_tablespace_map(pgBackup *backup) pgFile **tablespace_map = NULL; pg_crc32 crc; parray *files = get_backup_filelist(backup, true); + bool use_crc32c = parse_program_version(backup->program_version) <= 20021 || + parse_program_version(backup->program_version) >= 20025; parray_qsort(files, pgFileCompareRelPathWithExternal); join_path_components(map_path, backup->database_dir, PG_TABLESPACE_MAP_FILE); @@ -738,7 +740,7 @@ validate_tablespace_map(pgBackup *backup) map_path, base36enc(backup->backup_id)); /* check tablespace map checksumms */ - crc = pgFileGetCRC(map_path, true, false); + crc = pgFileGetCRC(map_path, use_crc32c, false); if ((*tablespace_map)->crc != crc) elog(ERROR, "Invalid CRC of tablespace map file \"%s\" : %X. Expected %X, " From f4ab4b9d2b46f337b601af9b6ee640d5f03abf47 Mon Sep 17 00:00:00 2001 From: Grigory Smolkin Date: Sun, 21 Mar 2021 01:33:36 +0300 Subject: [PATCH 8/8] Version 2.4.11 --- src/pg_probackup.h | 2 +- tests/expected/option_version.out | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pg_probackup.h b/src/pg_probackup.h index 1e001bd0..f28f96e2 100644 --- a/src/pg_probackup.h +++ b/src/pg_probackup.h @@ -306,7 +306,7 @@ typedef enum ShowFormat #define BYTES_INVALID (-1) /* file didn`t changed since previous backup, DELTA backup do not rely on it */ #define FILE_NOT_FOUND (-2) /* file disappeared during backup */ #define BLOCKNUM_INVALID (-1) -#define PROGRAM_VERSION "2.4.10" +#define PROGRAM_VERSION "2.4.11" /* update when remote agent API or behaviour changes */ #define AGENT_PROTOCOL_VERSION 20409 diff --git a/tests/expected/option_version.out b/tests/expected/option_version.out index b5204e46..35f065a1 100644 --- a/tests/expected/option_version.out +++ b/tests/expected/option_version.out @@ -1 +1 @@ -pg_probackup 2.4.10 \ No newline at end of file +pg_probackup 2.4.11 \ No newline at end of file