From 25fc034509d22af508559b38fbab847ccf8db577 Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Mon, 5 Sep 2022 19:38:14 +0300 Subject: [PATCH 01/20] [PBCKP-236] stable test failure, dirty version --- tests/compatibility.py | 72 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/tests/compatibility.py b/tests/compatibility.py index e274c22be..262b940ef 100644 --- a/tests/compatibility.py +++ b/tests/compatibility.py @@ -10,6 +10,78 @@ class CompatibilityTest(ProbackupTest, unittest.TestCase): + def setUp(self): + self.fname = self.id().split('.')[3] + + # @unittest.expectedFailure + # @unittest.skip("skip") + def test_catchup_with_different_remote_major_pg(self): + "Decription in jira issue PBCKP-236" #TODO REVIEW XXX explain the test + self.verbose = True + self.remote = True + pg_config = os.environ['PG_CONFIG'] + pg_path_ee_9_6 = '/home/avaness/postgres/postgres.build.9.6/bin/' + pg_config_ee_9_6 = pg_path_ee_9_6 + 'pg_config' + probackup_path_ee_9_6 = pg_path_ee_9_6 + 'pg_probackup' + pg_path_ee_11 = '/home/avaness/postgres/postgres.build.11/bin/' + pg_config_ee_11 = pg_path_ee_11 + 'pg_config' + probackup_path_ee_11 = pg_path_ee_11 + 'pg_probackup' + + os.environ['PG_CONFIG'] = pg_config_ee_11 + self.probackup_path = probackup_path_ee_11 + # os.environ['PG_CONFIG'] = pg_config_ee_9_6 + # self.probackup_path = probackup_path_ee_9_6 + + # backup_dir = os.path.join(self.tmp_path, module_name, self.fname, 'backup') + src_pg = self.make_simple_node( + base_dir=os.path.join(module_name, self.fname, 'src'), + set_replication=True, + # initdb_params=['--data-checksums'] + ) + src_pg.slow_start() + src_pg.safe_psql( + "postgres", + "CREATE TABLE ultimate_question AS SELECT 42 AS answer") + + # do full catchup + os.environ['PG_CONFIG'] = pg_config_ee_11 + self.probackup_path = probackup_path_ee_11 + + dst_pg = self.make_empty_node(os.path.join(module_name, self.fname, 'dst')) + # dst_pg = self.make_simple_node( + # base_dir=os.path.join(module_name, self.fname, 'dst'), + # set_replication=True, + # # initdb_params=['--data-checksums'] + # ) + self.catchup_node( + backup_mode = 'FULL', + source_pgdata = src_pg.data_dir, + destination_node = dst_pg, + options=['-d', 'postgres', '-p', str(src_pg.port), '--stream']#, '--remote-path=' + pg_path_ee_9_6] + ) + + dst_options = {} + dst_options['port'] = str(dst_pg.port) + self.set_auto_conf(dst_pg, dst_options) + dst_pg.slow_start() + dst_pg.stop() + + src_pg.safe_psql( + "postgres", + "CREATE TABLE ultimate_question2 AS SELECT 42 AS answer") + + # do delta catchup + #TODO REVIEW XXX try to apply only one catchup (FULL) for test failure + self.catchup_node( + backup_mode = 'DELTA', + source_pgdata = src_pg.data_dir, + destination_node = dst_pg, + options=['-d', 'postgres', '-p', str(src_pg.port), '--stream', '--remote-path=' + pg_path_ee_9_6] + ) + + # Clean after yourself + self.del_test_dir(module_name, self.fname) + # @unittest.expectedFailure # @unittest.skip("skip") def test_backward_compatibility_page(self): From f78c63c8f56b8a7064f8799460156001dd1c6a76 Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Sun, 11 Sep 2022 04:14:18 +0300 Subject: [PATCH 02/20] [PBCKP-236] first-stage compatibility protocol impl with stubs --- src/pg_probackup.h | 8 ++++++-- src/utils/file.c | 20 +++++++++++++++++--- src/utils/file.h | 2 +- src/utils/remote.c | 34 +++++++++++++++++++++++++++++----- 4 files changed, 53 insertions(+), 11 deletions(-) diff --git a/src/pg_probackup.h b/src/pg_probackup.h index 1885a191e..e68afc571 100644 --- a/src/pg_probackup.h +++ b/src/pg_probackup.h @@ -341,8 +341,8 @@ typedef enum ShowFormat #define PROGRAM_VERSION "2.5.8" /* update when remote agent API or behaviour changes */ -#define AGENT_PROTOCOL_VERSION 20501 -#define AGENT_PROTOCOL_VERSION_STR "2.5.1" +#define AGENT_PROTOCOL_VERSION 20509 +#define AGENT_PROTOCOL_VERSION_STR "2.5.9" /* update only when changing storage format */ #define STORAGE_FORMAT_VERSION "2.4.4" @@ -881,6 +881,10 @@ extern bool tliIsPartOfHistory(const parray *timelines, TimeLineID tli); extern DestDirIncrCompatibility check_incremental_compatibility(const char *pgdata, uint64 system_identifier, IncrRestoreMode incremental_mode); +/* in remote.c */ +extern void check_remote_agent_compatibility(int agent_version, char *compatibility_str); +extern size_t prepare_remote_agent_compatibility_str(char* compatibility_buf, size_t buf_size); + /* in merge.c */ extern void do_merge(InstanceState *instanceState, time_t backup_id, bool no_validate, bool no_sync); extern void merge_backups(pgBackup *backup, pgBackup *next_backup); diff --git a/src/utils/file.c b/src/utils/file.c index 7103c8f1d..e3d1e5801 100644 --- a/src/utils/file.c +++ b/src/utils/file.c @@ -268,9 +268,10 @@ fio_write_all(int fd, void const* buf, size_t size) return offs; } +//TODO REVIEW XXX move to remote.c???? /* Get version of remote agent */ -int -fio_get_agent_version(void) +void +fio_get_agent_version(int* protocol, char* payload_buf, size_t payload_buf_size) { fio_header hdr; hdr.cop = FIO_AGENT_VERSION; @@ -278,8 +279,13 @@ fio_get_agent_version(void) IO_CHECK(fio_write_all(fio_stdout, &hdr, sizeof(hdr)), sizeof(hdr)); IO_CHECK(fio_read_all(fio_stdin, &hdr, sizeof(hdr)), sizeof(hdr)); + if (hdr.size > payload_buf_size) + { + elog(ERROR, "Bad protocol, insufficient payload_buf_size=%u", payload_buf_size); + } - return hdr.arg; + *protocol = hdr.arg; + IO_CHECK(fio_read_all(fio_stdin, payload_buf, hdr.size), hdr.size); } /* Open input stream. Remote file is fetched to the in-memory buffer and then accessed through Linux fmemopen */ @@ -3210,6 +3216,7 @@ fio_delete_impl(mode_t mode, char *buf) } /* Execute commands at remote host */ +//TODO REVIEW XXX move to remote.c? void fio_communicate(int in, int out) { @@ -3316,6 +3323,13 @@ fio_communicate(int in, int out) case FIO_AGENT_VERSION: hdr.arg = AGENT_PROTOCOL_VERSION; IO_CHECK(fio_write_all(out, &hdr, sizeof(hdr)), sizeof(hdr)); + //TODO REVIEW XXX is it allowed by ANSI C to declare new scope inside??? + { + size_t payload_size = prepare_remote_agent_compatibility_str(buf, buf_size); + IO_CHECK(fio_write_all(out, buf, payload_size), payload_size); + //TODO REVIEW XXX make INFO to LOG or VERBOSE + elog(INFO, "TODO REVIEW XXX sent agent compatibility\n %s", buf); + } break; case FIO_STAT: /* Get information about file with specified path */ hdr.size = sizeof(st); diff --git a/src/utils/file.h b/src/utils/file.h index a554b4ab0..92c5f2eaa 100644 --- a/src/utils/file.h +++ b/src/utils/file.h @@ -91,7 +91,7 @@ extern fio_location MyLocation; extern void fio_redirect(int in, int out, int err); extern void fio_communicate(int in, int out); -extern int fio_get_agent_version(void); +extern void fio_get_agent_version(int* protocol, char* payload_buf, size_t payload_buf_size); extern FILE* fio_fopen(char const* name, char const* mode, fio_location location); extern size_t fio_fwrite(FILE* f, void const* buf, size_t size); extern ssize_t fio_fwrite_async_compressed(FILE* f, void const* buf, size_t size, int compress_alg); diff --git a/src/utils/remote.c b/src/utils/remote.c index 046ebd818..c7a1f9330 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -117,6 +117,9 @@ bool launch_agent(void) int infd[2]; int errfd[2]; int agent_version; + //TODO REVIEW XXX review buf_size + size_t payload_buf_size = 1024 * 8; + char payload_buf[payload_buf_size]; ssh_argc = 0; #ifdef WIN32 @@ -238,10 +241,31 @@ bool launch_agent(void) fio_redirect(infd[0], outfd[1], errfd[0]); /* write to stdout */ } - /* Make sure that remote agent has the same version - * TODO: we must also check PG version and fork edition + /* Make sure that remote agent has the same version, fork and other features to be binary compatible */ - agent_version = fio_get_agent_version(); + fio_get_agent_version(&agent_version, payload_buf, payload_buf_size); + check_remote_agent_compatibility(0, payload_buf); + + return true; +} + +//TODO REVIEW XXX review macro +#define STR(macro) #macro +size_t prepare_remote_agent_compatibility_str(char* compatibility_buf, size_t buf_size) +{ + size_t payload_size = snprintf(compatibility_buf, buf_size, + "%s\n%s\n%s\n%s\n", + STR(PG_MAJORVERSION), PG_MAJORVERSION, + STR(PGPRO_EDN), PGPRO_EDN); + if (payload_size >= buf_size) + { + elog(ERROR, "TODO REVIEW XXX too bad message buffer exhaust"); + } + return payload_size + 1; +} + +void check_remote_agent_compatibility(int agent_version, char *compatibility_str) +{ if (agent_version != AGENT_PROTOCOL_VERSION) { char agent_version_str[1024]; @@ -254,6 +278,6 @@ bool launch_agent(void) "consider to upgrade pg_probackup binary", agent_version_str, AGENT_PROTOCOL_VERSION_STR); } - - return true; + assert(false); + elog(ERROR, " check_remote_agent_compatibility() not implemented"); } From 1dfa5b99c2c20a1a97d01b7f141d450a938a9a4f Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Sun, 11 Sep 2022 04:37:46 +0300 Subject: [PATCH 03/20] [PBCKP-236] draft, first-stage compatibility protocol impl with stubs --- src/utils/file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utils/file.c b/src/utils/file.c index e3d1e5801..6c7bdbbff 100644 --- a/src/utils/file.c +++ b/src/utils/file.c @@ -3323,6 +3323,7 @@ fio_communicate(int in, int out) case FIO_AGENT_VERSION: hdr.arg = AGENT_PROTOCOL_VERSION; IO_CHECK(fio_write_all(out, &hdr, sizeof(hdr)), sizeof(hdr)); + assert(false); //TODO REVIEW XXX is it allowed by ANSI C to declare new scope inside??? { size_t payload_size = prepare_remote_agent_compatibility_str(buf, buf_size); From c3d3c026c2f8446b93e5e73150a3c55a153f4317 Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Sun, 11 Sep 2022 04:44:18 +0300 Subject: [PATCH 04/20] [PBCKP-236] draft, first-stage compatibility protocol impl with stubs --- src/utils/remote.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/remote.c b/src/utils/remote.c index c7a1f9330..6a6c3c12b 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -255,8 +255,8 @@ size_t prepare_remote_agent_compatibility_str(char* compatibility_buf, size_t bu { size_t payload_size = snprintf(compatibility_buf, buf_size, "%s\n%s\n%s\n%s\n", - STR(PG_MAJORVERSION), PG_MAJORVERSION, - STR(PGPRO_EDN), PGPRO_EDN); + STR(PG_MAJORVERSION), PG_MAJORVERSION); +// STR(PGPRO_EDN), PGPRO_EDN); if (payload_size >= buf_size) { elog(ERROR, "TODO REVIEW XXX too bad message buffer exhaust"); From 46b7079edd63b43c41c104830d7feef9d4536476 Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Sun, 11 Sep 2022 04:46:15 +0300 Subject: [PATCH 05/20] [PBCKP-236] draft, first-stage compatibility protocol impl with stubs --- src/utils/remote.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/utils/remote.c b/src/utils/remote.c index 6a6c3c12b..e4963b62a 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -254,7 +254,8 @@ bool launch_agent(void) size_t prepare_remote_agent_compatibility_str(char* compatibility_buf, size_t buf_size) { size_t payload_size = snprintf(compatibility_buf, buf_size, - "%s\n%s\n%s\n%s\n", +// "%s\n%s\n%s\n%s\n", + "%s\n%s\n", STR(PG_MAJORVERSION), PG_MAJORVERSION); // STR(PGPRO_EDN), PGPRO_EDN); if (payload_size >= buf_size) From f5fde7ef8e1ee479932df78086f7bed817c53902 Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Mon, 12 Sep 2022 02:46:29 +0300 Subject: [PATCH 06/20] [PBCKP-236] draft, first-stage compatibility protocol impl with stubs --- src/utils/file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/file.c b/src/utils/file.c index 6c7bdbbff..65d0699c7 100644 --- a/src/utils/file.c +++ b/src/utils/file.c @@ -281,7 +281,7 @@ fio_get_agent_version(int* protocol, char* payload_buf, size_t payload_buf_size) IO_CHECK(fio_read_all(fio_stdin, &hdr, sizeof(hdr)), sizeof(hdr)); if (hdr.size > payload_buf_size) { - elog(ERROR, "Bad protocol, insufficient payload_buf_size=%u", payload_buf_size); + elog(ERROR, "Bad protocol, insufficient payload_buf_size=%zu", payload_buf_size); } *protocol = hdr.arg; @@ -3323,7 +3323,6 @@ fio_communicate(int in, int out) case FIO_AGENT_VERSION: hdr.arg = AGENT_PROTOCOL_VERSION; IO_CHECK(fio_write_all(out, &hdr, sizeof(hdr)), sizeof(hdr)); - assert(false); //TODO REVIEW XXX is it allowed by ANSI C to declare new scope inside??? { size_t payload_size = prepare_remote_agent_compatibility_str(buf, buf_size); @@ -3331,6 +3330,7 @@ fio_communicate(int in, int out) //TODO REVIEW XXX make INFO to LOG or VERBOSE elog(INFO, "TODO REVIEW XXX sent agent compatibility\n %s", buf); } + assert(false); break; case FIO_STAT: /* Get information about file with specified path */ hdr.size = sizeof(st); From 497751c0b63b8cfdf825beeec26f1d66a902be6e Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Thu, 15 Sep 2022 05:47:36 +0300 Subject: [PATCH 07/20] [PBCKP-236] draft, tests.CompatibilityTest.test_catchup_with_different_remote_major_pg fixes --- tests/compatibility.py | 50 +++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/tests/compatibility.py b/tests/compatibility.py index 262b940ef..0562b441c 100644 --- a/tests/compatibility.py +++ b/tests/compatibility.py @@ -14,29 +14,30 @@ def setUp(self): self.fname = self.id().split('.')[3] # @unittest.expectedFailure - # @unittest.skip("skip") + @unittest.skip("skip") def test_catchup_with_different_remote_major_pg(self): - "Decription in jira issue PBCKP-236" #TODO REVIEW XXX explain the test + """ + Decription in jira issue PBCKP-236 + This test requires builds both PGPROEE11 and PGPROEE9_6 + + prerequisites: + - git tag for PBCKP 2.5.1 + - master probackup build should be inside PGPROEE11 + - agent probackup build is inside PGPROEE9_6 + + calling probackup PGPROEE9_6 agent from PGPROEE11 probackup master for DELTA backup causes the PBCKP-236 problem + + please correct path for agent's pg_path_ee_9_6 = '/home/avaness/postgres/postgres.build.ee.9.6/bin/' + """ + self.verbose = True self.remote = True - pg_config = os.environ['PG_CONFIG'] - pg_path_ee_9_6 = '/home/avaness/postgres/postgres.build.9.6/bin/' - pg_config_ee_9_6 = pg_path_ee_9_6 + 'pg_config' - probackup_path_ee_9_6 = pg_path_ee_9_6 + 'pg_probackup' - pg_path_ee_11 = '/home/avaness/postgres/postgres.build.11/bin/' - pg_config_ee_11 = pg_path_ee_11 + 'pg_config' - probackup_path_ee_11 = pg_path_ee_11 + 'pg_probackup' - - os.environ['PG_CONFIG'] = pg_config_ee_11 - self.probackup_path = probackup_path_ee_11 - # os.environ['PG_CONFIG'] = pg_config_ee_9_6 - # self.probackup_path = probackup_path_ee_9_6 - - # backup_dir = os.path.join(self.tmp_path, module_name, self.fname, 'backup') + # please use your own local path + pg_path_ee_9_6 = '/home/avaness/postgres/postgres.build.ee.9.6/bin/' + src_pg = self.make_simple_node( base_dir=os.path.join(module_name, self.fname, 'src'), set_replication=True, - # initdb_params=['--data-checksums'] ) src_pg.slow_start() src_pg.safe_psql( @@ -44,20 +45,12 @@ def test_catchup_with_different_remote_major_pg(self): "CREATE TABLE ultimate_question AS SELECT 42 AS answer") # do full catchup - os.environ['PG_CONFIG'] = pg_config_ee_11 - self.probackup_path = probackup_path_ee_11 - dst_pg = self.make_empty_node(os.path.join(module_name, self.fname, 'dst')) - # dst_pg = self.make_simple_node( - # base_dir=os.path.join(module_name, self.fname, 'dst'), - # set_replication=True, - # # initdb_params=['--data-checksums'] - # ) self.catchup_node( backup_mode = 'FULL', source_pgdata = src_pg.data_dir, destination_node = dst_pg, - options=['-d', 'postgres', '-p', str(src_pg.port), '--stream']#, '--remote-path=' + pg_path_ee_9_6] + options=['-d', 'postgres', '-p', str(src_pg.port), '--stream'] ) dst_options = {} @@ -70,12 +63,13 @@ def test_catchup_with_different_remote_major_pg(self): "postgres", "CREATE TABLE ultimate_question2 AS SELECT 42 AS answer") - # do delta catchup - #TODO REVIEW XXX try to apply only one catchup (FULL) for test failure + # do delta catchup with remote pg_probackup agent with another postgres major version + # this DELTA backup should fail without PBCKP-236 patch. self.catchup_node( backup_mode = 'DELTA', source_pgdata = src_pg.data_dir, destination_node = dst_pg, + # here's substitution of --remoge-path pg_probackup agent compiled with another postgres version options=['-d', 'postgres', '-p', str(src_pg.port), '--stream', '--remote-path=' + pg_path_ee_9_6] ) From eefd88768a8cb2b6350d69ca47cc28a5e8beb160 Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Fri, 16 Sep 2022 01:06:52 +0300 Subject: [PATCH 08/20] [PBCKP-236] draft, solution without couple of unapplied shortenings --- src/pg_probackup.h | 5 ++- src/utils/file.c | 16 ++++--- src/utils/remote.c | 103 ++++++++++++++++++++++++++++++++++++++------- 3 files changed, 99 insertions(+), 25 deletions(-) diff --git a/src/pg_probackup.h b/src/pg_probackup.h index e68afc571..13dfe1989 100644 --- a/src/pg_probackup.h +++ b/src/pg_probackup.h @@ -882,8 +882,9 @@ extern DestDirIncrCompatibility check_incremental_compatibility(const char *pgda IncrRestoreMode incremental_mode); /* in remote.c */ -extern void check_remote_agent_compatibility(int agent_version, char *compatibility_str); -extern size_t prepare_remote_agent_compatibility_str(char* compatibility_buf, size_t buf_size); +extern void check_remote_agent_compatibility(int agent_version, + char *compatibility_str, size_t compatibility_str_max_size); +extern size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_buf_size); /* in merge.c */ extern void do_merge(InstanceState *instanceState, time_t backup_id, bool no_validate, bool no_sync); diff --git a/src/utils/file.c b/src/utils/file.c index 65d0699c7..b0dc39ae9 100644 --- a/src/utils/file.c +++ b/src/utils/file.c @@ -281,7 +281,8 @@ fio_get_agent_version(int* protocol, char* payload_buf, size_t payload_buf_size) IO_CHECK(fio_read_all(fio_stdin, &hdr, sizeof(hdr)), sizeof(hdr)); if (hdr.size > payload_buf_size) { - elog(ERROR, "Bad protocol, insufficient payload_buf_size=%zu", payload_buf_size); + //TODO REVIEW XXX %zu is C99 but not ANSI S standard, should we cast to unsigned long? + elog(ERROR, "Corrupted remote compatibility protocol: insufficient payload_buf_size=%zu", payload_buf_size); } *protocol = hdr.arg; @@ -3321,17 +3322,18 @@ fio_communicate(int in, int out) IO_CHECK(fio_write_all(out, buf, hdr.size), hdr.size); break; case FIO_AGENT_VERSION: - hdr.arg = AGENT_PROTOCOL_VERSION; - IO_CHECK(fio_write_all(out, &hdr, sizeof(hdr)), sizeof(hdr)); - //TODO REVIEW XXX is it allowed by ANSI C to declare new scope inside??? { - size_t payload_size = prepare_remote_agent_compatibility_str(buf, buf_size); + size_t payload_size = prepare_compatibility_str(buf, buf_size); + + hdr.arg = AGENT_PROTOCOL_VERSION; + hdr.size = payload_size; + + IO_CHECK(fio_write_all(out, &hdr, sizeof(hdr)), sizeof(hdr)); IO_CHECK(fio_write_all(out, buf, payload_size), payload_size); //TODO REVIEW XXX make INFO to LOG or VERBOSE elog(INFO, "TODO REVIEW XXX sent agent compatibility\n %s", buf); + break; } - assert(false); - break; case FIO_STAT: /* Get information about file with specified path */ hdr.size = sizeof(st); rc = hdr.arg ? stat(buf, &st) : lstat(buf, &st); diff --git a/src/utils/remote.c b/src/utils/remote.c index e4963b62a..af3e460c0 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -118,7 +118,7 @@ bool launch_agent(void) int errfd[2]; int agent_version; //TODO REVIEW XXX review buf_size - size_t payload_buf_size = 1024 * 8; + int payload_buf_size = 1024 * 8; char payload_buf[payload_buf_size]; ssh_argc = 0; @@ -244,29 +244,83 @@ bool launch_agent(void) /* Make sure that remote agent has the same version, fork and other features to be binary compatible */ fio_get_agent_version(&agent_version, payload_buf, payload_buf_size); - check_remote_agent_compatibility(0, payload_buf); + check_remote_agent_compatibility(agent_version, payload_buf, payload_buf_size); return true; } -//TODO REVIEW XXX review macro -#define STR(macro) #macro -size_t prepare_remote_agent_compatibility_str(char* compatibility_buf, size_t buf_size) +#define COMPATIBILITY_VAL(macro) #macro, macro +#define COMPATIBILITY_STR(macro) #macro +#define COMPATIBILITY_VAL_STR(macro) #macro, COMPATIBILITY_STR(macro) + +#define COMPATIBILITY_VAL_SEPARATOR "=" +#define COMPATIBILITY_LINE_SEPARATOR "\n" + +static char* compatibility_params[] = { + COMPATIBILITY_VAL(PG_MAJORVERSION), + //TODO remove? + //TODO doesn't work macro name check for ints!!!! + COMPATIBILITY_VAL_STR(SIZEOF_VOID_P), + //TODO REVIEW XXX can use edition.h/extract_pgpro_edition() +#ifdef PGPRO_EDN + //TODO add vanilla + //TODO make "1c" -> "vanilla" + COMPATIBILITY_VAL(PGPRO_EDN), +#endif +}; + +/* + * Compose compatibility string to be sent by pg_probackup agent + * through ssh and to be verified by pg_probackup peer. + * Compatibility string contains postgres essential vars as strings + * in format "var_name" + COMPATIBILITY_VAL_SEPARATOR + "var_value" + COMPATIBILITY_LINE_SEPARATOR + */ +size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_buf_size) { - size_t payload_size = snprintf(compatibility_buf, buf_size, -// "%s\n%s\n%s\n%s\n", - "%s\n%s\n", - STR(PG_MAJORVERSION), PG_MAJORVERSION); -// STR(PGPRO_EDN), PGPRO_EDN); - if (payload_size >= buf_size) + char tmp_buf[1024]; + int size_inc = 0; + size_t result_size = 1; + size_t compatibility_params_array_size = sizeof compatibility_params / sizeof compatibility_params[0];; + + *compatibility_buf = '\0'; + Assert(compatibility_params_array_size % 2 == 0); + + //TODO !!!! + for (int i = 0; i < compatibility_params_array_size; i+=2) { - elog(ERROR, "TODO REVIEW XXX too bad message buffer exhaust"); + size_inc = snprintf(compatibility_buf + size_inc, compatibility_buf_size, + "%s" COMPATIBILITY_VAL_SEPARATOR "%s" COMPATIBILITY_LINE_SEPARATOR, + compatibility_params[i], compatibility_params[i+1]); + +// size_inc = snprintf(tmp_buf, sizeof tmp_buf, +// "%s" COMPATIBILITY_VAL_SEPARATOR "%s" COMPATIBILITY_LINE_SEPARATOR, +// compatibility_params[i], compatibility_params[i+1]); + if (size_inc >= sizeof tmp_buf) + { + //TODO make Assert + elog(ERROR, "Compatibility params from agent doesn't fit to %zu chars, %s=%s", + sizeof tmp_buf - 1, compatibility_params[i], compatibility_params[i+1] ); + } + + result_size += size_inc; + if (result_size > compatibility_buf_size) + { + //TODO make Assert + elog(ERROR, "Can't fit compatibility string size %zu to buffer size %zu:\n%s\n%s", + result_size, compatibility_buf_size, compatibility_buf, tmp_buf); + } + strcat(compatibility_buf, tmp_buf); } - return payload_size + 1; + return result_size; } -void check_remote_agent_compatibility(int agent_version, char *compatibility_str) +/* + * Check incoming remote agent's compatibility params for equality to local ones. + */ +void check_remote_agent_compatibility(int agent_version, char *compatibility_str, size_t compatibility_str_max_size) { + elog(LOG, "Agent version=%d", agent_version); + if (agent_version != AGENT_PROTOCOL_VERSION) { char agent_version_str[1024]; @@ -279,6 +333,23 @@ void check_remote_agent_compatibility(int agent_version, char *compatibility_str "consider to upgrade pg_probackup binary", agent_version_str, AGENT_PROTOCOL_VERSION_STR); } - assert(false); - elog(ERROR, " check_remote_agent_compatibility() not implemented"); + + if (strnlen(compatibility_str, compatibility_str_max_size) == compatibility_str_max_size) + { + elog(ERROR, "Corrupted remote compatibility protocol: compatibility string has no terminating \\0"); + } + + elog(LOG, "Agent compatibility params: '%s'", compatibility_str); + + /* checking compatibility params */ + { + char *buf[compatibility_str_max_size]; + + prepare_compatibility_str(buf, sizeof buf); + if(!strcmp(compatibility_str, buf)) + { + elog(ERROR, "Incompatible agent params, expected %s", buf); + } + } + } From 0604cce21de69a3e3b0ea6a6634a6dac53ae39ea Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Fri, 16 Sep 2022 01:55:25 +0300 Subject: [PATCH 09/20] [PBCKP-236] draft, solution without macros cleanup --- src/utils/file.c | 4 +--- src/utils/remote.c | 48 +++++++++++++--------------------------------- 2 files changed, 14 insertions(+), 38 deletions(-) diff --git a/src/utils/file.c b/src/utils/file.c index b0dc39ae9..fa0983947 100644 --- a/src/utils/file.c +++ b/src/utils/file.c @@ -281,7 +281,7 @@ fio_get_agent_version(int* protocol, char* payload_buf, size_t payload_buf_size) IO_CHECK(fio_read_all(fio_stdin, &hdr, sizeof(hdr)), sizeof(hdr)); if (hdr.size > payload_buf_size) { - //TODO REVIEW XXX %zu is C99 but not ANSI S standard, should we cast to unsigned long? + //TODO REVIEW XXX %zu is C99 but not ANSI S compatible, should we use ints, %zu is also applied at data.c:501 data.c:1638?? elog(ERROR, "Corrupted remote compatibility protocol: insufficient payload_buf_size=%zu", payload_buf_size); } @@ -3330,8 +3330,6 @@ fio_communicate(int in, int out) IO_CHECK(fio_write_all(out, &hdr, sizeof(hdr)), sizeof(hdr)); IO_CHECK(fio_write_all(out, buf, payload_size), payload_size); - //TODO REVIEW XXX make INFO to LOG or VERBOSE - elog(INFO, "TODO REVIEW XXX sent agent compatibility\n %s", buf); break; } case FIO_STAT: /* Get information about file with specified path */ diff --git a/src/utils/remote.c b/src/utils/remote.c index af3e460c0..2babe79ae 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -277,41 +277,20 @@ static char* compatibility_params[] = { */ size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_buf_size) { - char tmp_buf[1024]; - int size_inc = 0; - size_t result_size = 1; + size_t result_size = 0; size_t compatibility_params_array_size = sizeof compatibility_params / sizeof compatibility_params[0];; *compatibility_buf = '\0'; Assert(compatibility_params_array_size % 2 == 0); - //TODO !!!! for (int i = 0; i < compatibility_params_array_size; i+=2) { - size_inc = snprintf(compatibility_buf + size_inc, compatibility_buf_size, - "%s" COMPATIBILITY_VAL_SEPARATOR "%s" COMPATIBILITY_LINE_SEPARATOR, - compatibility_params[i], compatibility_params[i+1]); - -// size_inc = snprintf(tmp_buf, sizeof tmp_buf, -// "%s" COMPATIBILITY_VAL_SEPARATOR "%s" COMPATIBILITY_LINE_SEPARATOR, -// compatibility_params[i], compatibility_params[i+1]); - if (size_inc >= sizeof tmp_buf) - { - //TODO make Assert - elog(ERROR, "Compatibility params from agent doesn't fit to %zu chars, %s=%s", - sizeof tmp_buf - 1, compatibility_params[i], compatibility_params[i+1] ); - } - - result_size += size_inc; - if (result_size > compatibility_buf_size) - { - //TODO make Assert - elog(ERROR, "Can't fit compatibility string size %zu to buffer size %zu:\n%s\n%s", - result_size, compatibility_buf_size, compatibility_buf, tmp_buf); - } - strcat(compatibility_buf, tmp_buf); + result_size += snprintf(compatibility_buf + result_size, compatibility_buf_size - result_size, + "%s" COMPATIBILITY_VAL_SEPARATOR "%s" COMPATIBILITY_LINE_SEPARATOR, + compatibility_params[i], compatibility_params[i+1]); + Assert(result_size < compatibility_buf_size); } - return result_size; + return result_size + 1; } /* @@ -319,7 +298,7 @@ size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_b */ void check_remote_agent_compatibility(int agent_version, char *compatibility_str, size_t compatibility_str_max_size) { - elog(LOG, "Agent version=%d", agent_version); + elog(LOG, "Agent version=%d\n", agent_version); if (agent_version != AGENT_PROTOCOL_VERSION) { @@ -331,25 +310,24 @@ void check_remote_agent_compatibility(int agent_version, char *compatibility_str elog(ERROR, "Remote agent protocol version %s does not match local program protocol version %s, " "consider to upgrade pg_probackup binary", - agent_version_str, AGENT_PROTOCOL_VERSION_STR); + agent_version_str, AGENT_PROTOCOL_VERSION_STR); } + /* checking compatibility params */ if (strnlen(compatibility_str, compatibility_str_max_size) == compatibility_str_max_size) { elog(ERROR, "Corrupted remote compatibility protocol: compatibility string has no terminating \\0"); } - elog(LOG, "Agent compatibility params: '%s'", compatibility_str); + elog(LOG, "Agent compatibility params:\n%s", compatibility_str); - /* checking compatibility params */ { - char *buf[compatibility_str_max_size]; + char buf[compatibility_str_max_size]; prepare_compatibility_str(buf, sizeof buf); - if(!strcmp(compatibility_str, buf)) + if(strcmp(compatibility_str, buf)) { - elog(ERROR, "Incompatible agent params, expected %s", buf); + elog(ERROR, "Incompatible remote agent params, expected:\n%s", buf); } } - } From f61be78e783a5948750e5fc77f31eeab54a82ae2 Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Fri, 16 Sep 2022 04:39:18 +0300 Subject: [PATCH 10/20] [PBCKP-236] working solution cleaned up --- src/utils/remote.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/utils/remote.c b/src/utils/remote.c index 2babe79ae..79456d9fb 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -249,26 +249,13 @@ bool launch_agent(void) return true; } -#define COMPATIBILITY_VAL(macro) #macro, macro -#define COMPATIBILITY_STR(macro) #macro -#define COMPATIBILITY_VAL_STR(macro) #macro, COMPATIBILITY_STR(macro) +#define COMPATIBILITY_VAL_STR(macro) #macro, macro +#define COMPATIBILITY_VAL_INT_HELPER(macro, helper_buf, buf_size) (snprintf(helper_buf, buf_size, "%d", macro), helper_buf) +#define COMPATIBILITY_VAL_INT(macro, helper_buf, buf_size) #macro, COMPATIBILITY_VAL_INT_HELPER(macro, helper_buf, buf_size) #define COMPATIBILITY_VAL_SEPARATOR "=" #define COMPATIBILITY_LINE_SEPARATOR "\n" -static char* compatibility_params[] = { - COMPATIBILITY_VAL(PG_MAJORVERSION), - //TODO remove? - //TODO doesn't work macro name check for ints!!!! - COMPATIBILITY_VAL_STR(SIZEOF_VOID_P), - //TODO REVIEW XXX can use edition.h/extract_pgpro_edition() -#ifdef PGPRO_EDN - //TODO add vanilla - //TODO make "1c" -> "vanilla" - COMPATIBILITY_VAL(PGPRO_EDN), -#endif -}; - /* * Compose compatibility string to be sent by pg_probackup agent * through ssh and to be verified by pg_probackup peer. @@ -277,6 +264,18 @@ static char* compatibility_params[] = { */ size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_buf_size) { + char compatibility_val_int_macro_helper_buf[32]; + char* compatibility_params[] = { + COMPATIBILITY_VAL_STR(PG_MAJORVERSION), +#ifdef PGPRO_EDN + //TODO REVIEW can use edition.h/extract_pgpro_edition() + COMPATIBILITY_VAL_STR(PGPRO_EDN), +#endif + //TODO REVIEW remove? no difference between 32/64 in global/pg_control. + COMPATIBILITY_VAL_INT(SIZEOF_VOID_P, + compatibility_val_int_macro_helper_buf, sizeof compatibility_val_int_macro_helper_buf), + }; + size_t result_size = 0; size_t compatibility_params_array_size = sizeof compatibility_params / sizeof compatibility_params[0];; From b2091cd2c277d28ec8bc4f1c7980b1c1798076ea Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Fri, 16 Sep 2022 05:51:11 +0300 Subject: [PATCH 11/20] [PBCKP-236] [skip] removed unnecessary TODOs --- src/utils/file.c | 2 -- src/utils/remote.c | 16 ++++++++-------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/utils/file.c b/src/utils/file.c index fa0983947..5a2aa61a8 100644 --- a/src/utils/file.c +++ b/src/utils/file.c @@ -268,7 +268,6 @@ fio_write_all(int fd, void const* buf, size_t size) return offs; } -//TODO REVIEW XXX move to remote.c???? /* Get version of remote agent */ void fio_get_agent_version(int* protocol, char* payload_buf, size_t payload_buf_size) @@ -3217,7 +3216,6 @@ fio_delete_impl(mode_t mode, char *buf) } /* Execute commands at remote host */ -//TODO REVIEW XXX move to remote.c? void fio_communicate(int in, int out) { diff --git a/src/utils/remote.c b/src/utils/remote.c index 79456d9fb..97c8f3d4a 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -117,9 +117,6 @@ bool launch_agent(void) int infd[2]; int errfd[2]; int agent_version; - //TODO REVIEW XXX review buf_size - int payload_buf_size = 1024 * 8; - char payload_buf[payload_buf_size]; ssh_argc = 0; #ifdef WIN32 @@ -241,10 +238,13 @@ bool launch_agent(void) fio_redirect(infd[0], outfd[1], errfd[0]); /* write to stdout */ } - /* Make sure that remote agent has the same version, fork and other features to be binary compatible - */ - fio_get_agent_version(&agent_version, payload_buf, payload_buf_size); - check_remote_agent_compatibility(agent_version, payload_buf, payload_buf_size); + + /* Make sure that remote agent has the same version, fork and other features to be binary compatible */ + { + char payload_buf[1024]; + fio_get_agent_version(&agent_version, payload_buf, sizeof payload_buf); + check_remote_agent_compatibility(agent_version, payload_buf, sizeof payload_buf); + } return true; } @@ -268,7 +268,7 @@ size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_b char* compatibility_params[] = { COMPATIBILITY_VAL_STR(PG_MAJORVERSION), #ifdef PGPRO_EDN - //TODO REVIEW can use edition.h/extract_pgpro_edition() + //TODO REVIEW can use edition.h/extract_pgpro_edition() or similar COMPATIBILITY_VAL_STR(PGPRO_EDN), #endif //TODO REVIEW remove? no difference between 32/64 in global/pg_control. From 56598848c260c1cae5a6fb9d739f840c105668ed Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Fri, 16 Sep 2022 06:43:16 +0300 Subject: [PATCH 12/20] [PBCKP-236] ANSI C fix --- src/utils/remote.c | 2 +- tests/compatibility.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/remote.c b/src/utils/remote.c index 97c8f3d4a..a5294c705 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -321,7 +321,7 @@ void check_remote_agent_compatibility(int agent_version, char *compatibility_str elog(LOG, "Agent compatibility params:\n%s", compatibility_str); { - char buf[compatibility_str_max_size]; + char buf[1024]; prepare_compatibility_str(buf, sizeof buf); if(strcmp(compatibility_str, buf)) diff --git a/tests/compatibility.py b/tests/compatibility.py index 0562b441c..b0a9b0e50 100644 --- a/tests/compatibility.py +++ b/tests/compatibility.py @@ -14,7 +14,7 @@ def setUp(self): self.fname = self.id().split('.')[3] # @unittest.expectedFailure - @unittest.skip("skip") + # @unittest.skip("skip") def test_catchup_with_different_remote_major_pg(self): """ Decription in jira issue PBCKP-236 From 35df5060d5e1a20bdfa45dbaa5aab429f0fd4aa6 Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Mon, 26 Sep 2022 06:00:03 +0300 Subject: [PATCH 13/20] [PBCKP-236] 1c+certified editions check --- src/utils/file.c | 1 - src/utils/remote.c | 58 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/utils/file.c b/src/utils/file.c index 5a2aa61a8..242810b5e 100644 --- a/src/utils/file.c +++ b/src/utils/file.c @@ -280,7 +280,6 @@ fio_get_agent_version(int* protocol, char* payload_buf, size_t payload_buf_size) IO_CHECK(fio_read_all(fio_stdin, &hdr, sizeof(hdr)), sizeof(hdr)); if (hdr.size > payload_buf_size) { - //TODO REVIEW XXX %zu is C99 but not ANSI S compatible, should we use ints, %zu is also applied at data.c:501 data.c:1638?? elog(ERROR, "Corrupted remote compatibility protocol: insufficient payload_buf_size=%zu", payload_buf_size); } diff --git a/src/utils/remote.c b/src/utils/remote.c index a5294c705..786b4bfb1 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -249,6 +249,57 @@ bool launch_agent(void) return true; } +/* PGPRO 10-13 check to be "(certified)", with exceptional case PGPRO_11 conforming to "(standard certified)" */ +static bool check_certified() +{ +#ifdef PGPRO_VERSION_STR + return strstr(PGPRO_VERSION_STR, "(certified)") || + strstr(PGPRO_VERSION_STR, ("(standard certified)")); +#endif + return false; +} + +//TODO REVIEW review coding standard https://jira.postgrespro.ru/browse/PBCKP-251 with @funny_falcon, newlines, braces etc +static char* extract_pg_edition_str() +{ + static char *vanilla = "vanilla"; + static char *std = "standard"; + static char *ent = "enterprise"; + static char *std_cert = "standard-certified"; + static char *ent_cert = "enterprise-certified"; + +#ifdef PGPRO_EDITION + if (strcasecmp(PGPRO_EDITION, "1C") == 0) + return vanilla; + + /* these "certified" checks are applicable to PGPRO from 9.6 up to 12 versions. + * 13+ certified versions are compatible to non-certified ones */ + if (PG_VERSION_NUM < 100000) + { + if (strcmp(PGPRO_EDITION, "standard-certified") == 0) + return std_cert; + else if (strcmp(PGPRO_EDITION, "enterprise-certified")) + return ent_cert; + else + Assert("Bad #define PGPRO_EDITION value" == 0); + } + + if (check_certified()) + { + if (strcmp(PGPRO_EDITION, "standard")) + return std_cert; + else if (strcmp(PGPRO_EDITION, "enterprise") == 0) + return ent_cert; + else + Assert("Bad #define PGPRO_EDITION value" == 0); + } + + return PGPRO_EDITION; +#else + return vanilla; +#endif +} + #define COMPATIBILITY_VAL_STR(macro) #macro, macro #define COMPATIBILITY_VAL_INT_HELPER(macro, helper_buf, buf_size) (snprintf(helper_buf, buf_size, "%d", macro), helper_buf) #define COMPATIBILITY_VAL_INT(macro, helper_buf, buf_size) #macro, COMPATIBILITY_VAL_INT_HELPER(macro, helper_buf, buf_size) @@ -267,11 +318,8 @@ size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_b char compatibility_val_int_macro_helper_buf[32]; char* compatibility_params[] = { COMPATIBILITY_VAL_STR(PG_MAJORVERSION), -#ifdef PGPRO_EDN - //TODO REVIEW can use edition.h/extract_pgpro_edition() or similar - COMPATIBILITY_VAL_STR(PGPRO_EDN), -#endif - //TODO REVIEW remove? no difference between 32/64 in global/pg_control. + "edition", extract_pg_edition_str(), + /* 32/64 bits compatibility */ COMPATIBILITY_VAL_INT(SIZEOF_VOID_P, compatibility_val_int_macro_helper_buf, sizeof compatibility_val_int_macro_helper_buf), }; From b3351b50d664b9a639537d4aea694d971c9cd7d5 Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Tue, 27 Sep 2022 06:00:57 +0300 Subject: [PATCH 14/20] [PBCKP-236] final update --- src/utils/remote.c | 32 ++++++++++++-------------------- tests/compatibility.py | 8 ++++---- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/utils/remote.c b/src/utils/remote.c index 786b4bfb1..7d86be4c1 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -249,19 +249,18 @@ bool launch_agent(void) return true; } -/* PGPRO 10-13 check to be "(certified)", with exceptional case PGPRO_11 conforming to "(standard certified)" */ +#ifdef PGPRO_EDITION +/* PGPRO 10-13 checks to be "(certified)", with exceptional case PGPRO_11 conforming to "(standard certified)" */ static bool check_certified() { -#ifdef PGPRO_VERSION_STR return strstr(PGPRO_VERSION_STR, "(certified)") || strstr(PGPRO_VERSION_STR, ("(standard certified)")); -#endif - return false; } +#endif -//TODO REVIEW review coding standard https://jira.postgrespro.ru/browse/PBCKP-251 with @funny_falcon, newlines, braces etc static char* extract_pg_edition_str() { + static char *_1C = "1C"; static char *vanilla = "vanilla"; static char *std = "standard"; static char *ent = "enterprise"; @@ -269,26 +268,19 @@ static char* extract_pg_edition_str() static char *ent_cert = "enterprise-certified"; #ifdef PGPRO_EDITION - if (strcasecmp(PGPRO_EDITION, "1C") == 0) + if (strcmp(PGPRO_EDITION, _1C) == 0) return vanilla; - /* these "certified" checks are applicable to PGPRO from 9.6 up to 12 versions. - * 13+ certified versions are compatible to non-certified ones */ if (PG_VERSION_NUM < 100000) - { - if (strcmp(PGPRO_EDITION, "standard-certified") == 0) - return std_cert; - else if (strcmp(PGPRO_EDITION, "enterprise-certified")) - return ent_cert; - else - Assert("Bad #define PGPRO_EDITION value" == 0); - } + return PGPRO_EDITION; - if (check_certified()) + /* these "certified" checks are applicable to PGPRO from 10 up to 12 versions. + * 13+ certified versions are compatible to non-certified ones */ + if (PG_VERSION_NUM < 130000 && check_certified()) { - if (strcmp(PGPRO_EDITION, "standard")) + if (strcmp(PGPRO_EDITION, std) == 0) return std_cert; - else if (strcmp(PGPRO_EDITION, "enterprise") == 0) + else if (strcmp(PGPRO_EDITION, ent) == 0) return ent_cert; else Assert("Bad #define PGPRO_EDITION value" == 0); @@ -374,7 +366,7 @@ void check_remote_agent_compatibility(int agent_version, char *compatibility_str prepare_compatibility_str(buf, sizeof buf); if(strcmp(compatibility_str, buf)) { - elog(ERROR, "Incompatible remote agent params, expected:\n%s", buf); + elog(ERROR, "Incompatible remote agent params, expected:\n%s, actual:\n:%s ", buf, compatibility_str); } } } diff --git a/tests/compatibility.py b/tests/compatibility.py index b0a9b0e50..04af1478f 100644 --- a/tests/compatibility.py +++ b/tests/compatibility.py @@ -14,7 +14,7 @@ def setUp(self): self.fname = self.id().split('.')[3] # @unittest.expectedFailure - # @unittest.skip("skip") + @unittest.skip("skip") def test_catchup_with_different_remote_major_pg(self): """ Decription in jira issue PBCKP-236 @@ -27,13 +27,13 @@ def test_catchup_with_different_remote_major_pg(self): calling probackup PGPROEE9_6 agent from PGPROEE11 probackup master for DELTA backup causes the PBCKP-236 problem - please correct path for agent's pg_path_ee_9_6 = '/home/avaness/postgres/postgres.build.ee.9.6/bin/' + please correct path for agent's pg_path_remote_version = '/home/avaness/postgres/postgres.build.ee.9.6/bin/' """ self.verbose = True self.remote = True # please use your own local path - pg_path_ee_9_6 = '/home/avaness/postgres/postgres.build.ee.9.6/bin/' + pg_path_remote_version = '/home/avaness/postgres/postgres.build.clean/bin' src_pg = self.make_simple_node( base_dir=os.path.join(module_name, self.fname, 'src'), @@ -70,7 +70,7 @@ def test_catchup_with_different_remote_major_pg(self): source_pgdata = src_pg.data_dir, destination_node = dst_pg, # here's substitution of --remoge-path pg_probackup agent compiled with another postgres version - options=['-d', 'postgres', '-p', str(src_pg.port), '--stream', '--remote-path=' + pg_path_ee_9_6] + options=['-d', 'postgres', '-p', str(src_pg.port), '--stream', '--remote-path=' + pg_path_remote_version] ) # Clean after yourself From 6e671232b8791ccb8bf6090c9dcf38081fc9c2dd Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Wed, 28 Sep 2022 04:32:47 +0300 Subject: [PATCH 15/20] [PBCKP-236] final update after review --- src/utils/remote.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/utils/remote.c b/src/utils/remote.c index 7d86be4c1..0f254d147 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -292,12 +292,8 @@ static char* extract_pg_edition_str() #endif } -#define COMPATIBILITY_VAL_STR(macro) #macro, macro -#define COMPATIBILITY_VAL_INT_HELPER(macro, helper_buf, buf_size) (snprintf(helper_buf, buf_size, "%d", macro), helper_buf) -#define COMPATIBILITY_VAL_INT(macro, helper_buf, buf_size) #macro, COMPATIBILITY_VAL_INT_HELPER(macro, helper_buf, buf_size) - -#define COMPATIBILITY_VAL_SEPARATOR "=" -#define COMPATIBILITY_LINE_SEPARATOR "\n" +#define COMPATIBILITY_VAL_STR(macro) { #macro, macro, 0 } +#define COMPATIBILITY_VAL_INT(macro) { #macro, NULL, macro } /* * Compose compatibility string to be sent by pg_probackup agent @@ -307,13 +303,10 @@ static char* extract_pg_edition_str() */ size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_buf_size) { - char compatibility_val_int_macro_helper_buf[32]; - char* compatibility_params[] = { + struct { const char* name; const char* strval; int intval; } compatibility_params[] = { COMPATIBILITY_VAL_STR(PG_MAJORVERSION), - "edition", extract_pg_edition_str(), - /* 32/64 bits compatibility */ - COMPATIBILITY_VAL_INT(SIZEOF_VOID_P, - compatibility_val_int_macro_helper_buf, sizeof compatibility_val_int_macro_helper_buf), + { "edition", extract_pg_edition_str(), 0 }, + COMPATIBILITY_VAL_INT(SIZEOF_VOID_P), }; size_t result_size = 0; @@ -324,9 +317,16 @@ size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_b for (int i = 0; i < compatibility_params_array_size; i+=2) { - result_size += snprintf(compatibility_buf + result_size, compatibility_buf_size - result_size, - "%s" COMPATIBILITY_VAL_SEPARATOR "%s" COMPATIBILITY_LINE_SEPARATOR, - compatibility_params[i], compatibility_params[i+1]); + if (compatibility_params[i].strval != NULL) + result_size += snprintf(compatibility_buf + result_size, compatibility_buf_size - result_size, + "%s=%s/n", + compatibility_params[i].name, + compatibility_params[i].strval); + else + result_size += snprintf(compatibility_buf + result_size, compatibility_buf_size - result_size, + "%s=%d/n", + compatibility_params[i].name, + compatibility_params[i].intval); Assert(result_size < compatibility_buf_size); } return result_size + 1; @@ -349,7 +349,7 @@ void check_remote_agent_compatibility(int agent_version, char *compatibility_str elog(ERROR, "Remote agent protocol version %s does not match local program protocol version %s, " "consider to upgrade pg_probackup binary", - agent_version_str, AGENT_PROTOCOL_VERSION_STR); + agent_version_str, AGENT_PROTOCOL_VERSION_STR); } /* checking compatibility params */ From 1ce38ed70cccabcdea5dbda7f1030424ceeeef03 Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Wed, 28 Sep 2022 04:52:11 +0300 Subject: [PATCH 16/20] [PBCKP-236] final update after review --- src/utils/remote.c | 7 ++----- tests/compatibility.py | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/utils/remote.c b/src/utils/remote.c index 0f254d147..f3608e566 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -310,12 +310,9 @@ size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_b }; size_t result_size = 0; - size_t compatibility_params_array_size = sizeof compatibility_params / sizeof compatibility_params[0];; - *compatibility_buf = '\0'; - Assert(compatibility_params_array_size % 2 == 0); - for (int i = 0; i < compatibility_params_array_size; i+=2) + for (int i = 0; i < sizeof compatibility_params; i+=2) { if (compatibility_params[i].strval != NULL) result_size += snprintf(compatibility_buf + result_size, compatibility_buf_size - result_size, @@ -366,7 +363,7 @@ void check_remote_agent_compatibility(int agent_version, char *compatibility_str prepare_compatibility_str(buf, sizeof buf); if(strcmp(compatibility_str, buf)) { - elog(ERROR, "Incompatible remote agent params, expected:\n%s, actual:\n:%s ", buf, compatibility_str); + elog(ERROR, "Incompatible remote agent params, expected:\n%s, actual:\n:%s", buf, compatibility_str); } } } diff --git a/tests/compatibility.py b/tests/compatibility.py index 04af1478f..4e5e27f0e 100644 --- a/tests/compatibility.py +++ b/tests/compatibility.py @@ -14,7 +14,7 @@ def setUp(self): self.fname = self.id().split('.')[3] # @unittest.expectedFailure - @unittest.skip("skip") + # @unittest.skip("skip") def test_catchup_with_different_remote_major_pg(self): """ Decription in jira issue PBCKP-236 From c52659791b91012b13a2a8ebec1367dddc60187b Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Thu, 29 Sep 2022 03:01:57 +0300 Subject: [PATCH 17/20] [PBCKP-236] assert fix --- src/utils/remote.c | 17 +++++++++++++---- tests/compatibility.py | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/utils/remote.c b/src/utils/remote.c index f3608e566..91468b54c 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -295,6 +295,9 @@ static char* extract_pg_edition_str() #define COMPATIBILITY_VAL_STR(macro) { #macro, macro, 0 } #define COMPATIBILITY_VAL_INT(macro) { #macro, NULL, macro } +#define COMPATIBILITY_VAL_SEPARATOR "=" +#define COMPATIBILITY_LINE_SEPARATOR "\n" + /* * Compose compatibility string to be sent by pg_probackup agent * through ssh and to be verified by pg_probackup peer. @@ -303,7 +306,13 @@ static char* extract_pg_edition_str() */ size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_buf_size) { - struct { const char* name; const char* strval; int intval; } compatibility_params[] = { + typedef struct compatibility_param_tag { + const char* name; + const char* strval; + int intval; + } compatibility_param; + + compatibility_param compatibility_params[] = { COMPATIBILITY_VAL_STR(PG_MAJORVERSION), { "edition", extract_pg_edition_str(), 0 }, COMPATIBILITY_VAL_INT(SIZEOF_VOID_P), @@ -312,16 +321,16 @@ size_t prepare_compatibility_str(char* compatibility_buf, size_t compatibility_b size_t result_size = 0; *compatibility_buf = '\0'; - for (int i = 0; i < sizeof compatibility_params; i+=2) + for (int i = 0; i < (sizeof compatibility_params / sizeof(compatibility_param)); i++) { if (compatibility_params[i].strval != NULL) result_size += snprintf(compatibility_buf + result_size, compatibility_buf_size - result_size, - "%s=%s/n", + "%s" COMPATIBILITY_VAL_SEPARATOR "%s" COMPATIBILITY_LINE_SEPARATOR, compatibility_params[i].name, compatibility_params[i].strval); else result_size += snprintf(compatibility_buf + result_size, compatibility_buf_size - result_size, - "%s=%d/n", + "%s" COMPATIBILITY_VAL_SEPARATOR "%d" COMPATIBILITY_LINE_SEPARATOR, compatibility_params[i].name, compatibility_params[i].intval); Assert(result_size < compatibility_buf_size); diff --git a/tests/compatibility.py b/tests/compatibility.py index 4e5e27f0e..04af1478f 100644 --- a/tests/compatibility.py +++ b/tests/compatibility.py @@ -14,7 +14,7 @@ def setUp(self): self.fname = self.id().split('.')[3] # @unittest.expectedFailure - # @unittest.skip("skip") + @unittest.skip("skip") def test_catchup_with_different_remote_major_pg(self): """ Decription in jira issue PBCKP-236 From 03d55d079b836d285716a3df67a213fa1674a50a Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Thu, 29 Sep 2022 05:03:51 +0300 Subject: [PATCH 18/20] [PBCKP-236] fix excessive warnings for vanilla --- src/utils/remote.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/remote.c b/src/utils/remote.c index 91468b54c..9feb44a9c 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -260,14 +260,14 @@ static bool check_certified() static char* extract_pg_edition_str() { - static char *_1C = "1C"; static char *vanilla = "vanilla"; +#ifdef PGPRO_EDITION + static char *_1C = "1C"; static char *std = "standard"; static char *ent = "enterprise"; static char *std_cert = "standard-certified"; static char *ent_cert = "enterprise-certified"; -#ifdef PGPRO_EDITION if (strcmp(PGPRO_EDITION, _1C) == 0) return vanilla; From d808a16640be611e363b66900b18b2b6f8a52747 Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Tue, 11 Oct 2022 16:09:57 +0300 Subject: [PATCH 19/20] [PBCKP-236] removed excessive brackets --- src/utils/remote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/remote.c b/src/utils/remote.c index 9feb44a9c..addd73dc8 100644 --- a/src/utils/remote.c +++ b/src/utils/remote.c @@ -254,7 +254,7 @@ bool launch_agent(void) static bool check_certified() { return strstr(PGPRO_VERSION_STR, "(certified)") || - strstr(PGPRO_VERSION_STR, ("(standard certified)")); + strstr(PGPRO_VERSION_STR, "(standard certified)"); } #endif From 26f9992b2ab484a0bdd605d9669b2c86a07c28ed Mon Sep 17 00:00:00 2001 From: Ivan Lazarev Date: Mon, 24 Oct 2022 18:28:30 +0300 Subject: [PATCH 20/20] [PBCKP-236] added PGPROBACKUP_MANUAL testing and PGPROBACKUP_SSH_AGENT_PATH flags. --- tests/compatibility.py | 53 +++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/tests/compatibility.py b/tests/compatibility.py index 04af1478f..6c2bc9204 100644 --- a/tests/compatibility.py +++ b/tests/compatibility.py @@ -8,32 +8,48 @@ module_name = 'compatibility' +def check_manual_tests_enabled(): + return 'PGPROBACKUP_MANUAL' in os.environ and os.environ['PGPROBACKUP_MANUAL'] == 'ON' + + +def check_ssh_agent_path_exists(): + return 'PGPROBACKUP_SSH_AGENT_PATH' in os.environ + + class CompatibilityTest(ProbackupTest, unittest.TestCase): def setUp(self): self.fname = self.id().split('.')[3] # @unittest.expectedFailure - @unittest.skip("skip") + @unittest.skipUnless(check_manual_tests_enabled(), 'skip manual test') + @unittest.skipUnless(check_ssh_agent_path_exists(), 'skip no ssh agent path exist') + # @unittest.skip("skip") def test_catchup_with_different_remote_major_pg(self): """ Decription in jira issue PBCKP-236 - This test requires builds both PGPROEE11 and PGPROEE9_6 + This test exposures ticket error using pg_probackup builds for both PGPROEE11 and PGPROEE9_6 + + Prerequisites: + - pg_probackup git tag for PBCKP 2.5.1 + - master pg_probackup build should be made for PGPROEE11 + - agent pg_probackup build should be made for PGPROEE9_6 - prerequisites: - - git tag for PBCKP 2.5.1 - - master probackup build should be inside PGPROEE11 - - agent probackup build is inside PGPROEE9_6 + Calling probackup PGPROEE9_6 pg_probackup agent from PGPROEE11 pg_probackup master for DELTA backup causes + the PBCKP-236 problem - calling probackup PGPROEE9_6 agent from PGPROEE11 probackup master for DELTA backup causes the PBCKP-236 problem + Please give env variables PROBACKUP_MANUAL=ON;PGPROBACKUP_SSH_AGENT_PATH= + for the test - please correct path for agent's pg_path_remote_version = '/home/avaness/postgres/postgres.build.ee.9.6/bin/' + Please make path for agent's pgprobackup_ssh_agent_path = '/home/avaness/postgres/postgres.build.ee.9.6/bin/' + without pg_probackup executable """ self.verbose = True self.remote = True - # please use your own local path - pg_path_remote_version = '/home/avaness/postgres/postgres.build.clean/bin' + # please use your own local path like + # pgprobackup_ssh_agent_path = '/home/avaness/postgres/postgres.build.clean/bin/' + pgprobackup_ssh_agent_path = os.environ['PGPROBACKUP_SSH_AGENT_PATH'] src_pg = self.make_simple_node( base_dir=os.path.join(module_name, self.fname, 'src'), @@ -47,14 +63,13 @@ def test_catchup_with_different_remote_major_pg(self): # do full catchup dst_pg = self.make_empty_node(os.path.join(module_name, self.fname, 'dst')) self.catchup_node( - backup_mode = 'FULL', - source_pgdata = src_pg.data_dir, - destination_node = dst_pg, + backup_mode='FULL', + source_pgdata=src_pg.data_dir, + destination_node=dst_pg, options=['-d', 'postgres', '-p', str(src_pg.port), '--stream'] ) - dst_options = {} - dst_options['port'] = str(dst_pg.port) + dst_options = {'port': str(dst_pg.port)} self.set_auto_conf(dst_pg, dst_options) dst_pg.slow_start() dst_pg.stop() @@ -66,11 +81,11 @@ def test_catchup_with_different_remote_major_pg(self): # do delta catchup with remote pg_probackup agent with another postgres major version # this DELTA backup should fail without PBCKP-236 patch. self.catchup_node( - backup_mode = 'DELTA', - source_pgdata = src_pg.data_dir, - destination_node = dst_pg, + backup_mode='DELTA', + source_pgdata=src_pg.data_dir, + destination_node=dst_pg, # here's substitution of --remoge-path pg_probackup agent compiled with another postgres version - options=['-d', 'postgres', '-p', str(src_pg.port), '--stream', '--remote-path=' + pg_path_remote_version] + options=['-d', 'postgres', '-p', str(src_pg.port), '--stream', '--remote-path=' + pgprobackup_ssh_agent_path] ) # Clean after yourself