From a732a4c9a22037329bca888f4b4d4d24d1386da3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 4 Jan 2018 11:33:06 +0200 Subject: [PATCH 01/20] Fix packaging when both RPM and DEB tools are installed If both RPM and DEB tools are installed, only DEB packages should be generated. --- cmake/package.cmake | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/cmake/package.cmake b/cmake/package.cmake index 20f88c52c..e228645ca 100644 --- a/cmake/package.cmake +++ b/cmake/package.cmake @@ -58,19 +58,25 @@ find_program(DEBBUILD dpkg-buildpackage) if(TARBALL) include(cmake/package_tgz.cmake) -elseif (NOT ( ${RPMBUILD} STREQUAL "RPMBUILD-NOTFOUND" ) OR NOT ( ${DEBBUILD} STREQUAL "DEBBUILD-NOTFOUND" )) - if(NOT ( ${RPMBUILD} STREQUAL "RPMBUILD-NOTFOUND" ) ) +elseif(${RPMBUILD} MATCHES "NOTFOUND" AND ${DEBBUILD} MATCHES "NOTFOUND") + message(FATAL_ERROR "Could not automatically resolve the package generator and no generators " + "defined on the command line. Please install distribution specific packaging software or " + "define -DTARBALL=Y to build tar.gz packages.") +else() + + if(${DEBBUILD} MATCHES "NOTFOUND") + # No DEB packaging tools found, must be an RPM system include(cmake/package_rpm.cmake) - endif() - if(NOT ( ${DEBBUILD} STREQUAL "DEBBUILD-NOTFOUND" ) ) + else() + # We have DEB packaging tools, must be a DEB system + if (NOT ${RPMBUILD} MATCHES "NOTFOUND") + # Debian based systems can have both RPM and DEB packaging tools + message(WARNING "Found both DEB and RPM packaging tools, generating DEB packages. If this is not correct, " + "remove the packaging tools for the package type you DO NOT want to create.") + endif() include(cmake/package_deb.cmake) endif() message(STATUS "You can install startup scripts and system configuration files for MaxScale by running the 'postinst' shell script located at ${CMAKE_INSTALL_PREFIX}.") message(STATUS "To remove these installed files, run the 'postrm' shell script located in the same folder.") - -else() - message(FATAL_ERROR "Could not automatically resolve the package generator and no generators " - "defined on the command line. Please install distribution specific packaging software or " - "define -DTARBALL=Y to build tar.gz packages.") endif() From e310bbbe53d7b1db8c4b007d62a2b169fac51752 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 4 Jan 2018 14:53:03 +0200 Subject: [PATCH 02/20] Initialize query classifier in housekeeper thread The query classifier was not initialized for the housekeeper thread. This means that tasks could not use the query classifier and as the avro conversion is done inside a task, it can't use it. --- server/core/housekeeper.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/server/core/housekeeper.c b/server/core/housekeeper.c index d7406d72f..2e68199b3 100644 --- a/server/core/housekeeper.c +++ b/server/core/housekeeper.c @@ -18,6 +18,7 @@ #include #include #include +#include /** * @file housekeeper.c Provide a mechanism to run periodic tasks @@ -57,21 +58,30 @@ static THREAD hk_thr_handle; static void hkthread(void *); +struct hkinit_result +{ + sem_t sem; + bool ok; +}; + bool hkinit() { - bool inited = false; + struct hkinit_result res; + sem_init(&res.sem, 0, 0); + res.ok = false; - if (thread_start(&hk_thr_handle, hkthread, NULL) != NULL) + if (thread_start(&hk_thr_handle, hkthread, &res) != NULL) { - inited = true; + sem_wait(&res.sem); } else { MXS_ALERT("Failed to start housekeeper thread."); } - return inited; + sem_destroy(&res.sem); + return res.ok; } /** @@ -262,6 +272,16 @@ hkthread(void *data) void *taskdata; int i; + struct hkinit_result* res = (struct hkinit_result*)data; + res->ok = qc_thread_init(QC_INIT_BOTH); + + if (!res->ok) + { + MXS_ERROR("Could not initialize housekeeper thread."); + } + + sem_post(&res->sem); + while (!do_shutdown) { for (i = 0; i < 10; i++) @@ -301,6 +321,7 @@ hkthread(void *data) spinlock_release(&tasklock); } + qc_thread_end(QC_INIT_BOTH); MXS_NOTICE("Housekeeper shutting down."); } From 95ad3aa7df33577914ae96e893df1bcc59899824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 4 Jan 2018 14:55:16 +0200 Subject: [PATCH 03/20] MXS-1543: Log a warning if STATEMENT or MIXED is used If avrorouter suspects that statement based or mixed replication formats are being used, it logs a warning. --- server/modules/routing/avrorouter/avro_file.c | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/server/modules/routing/avrorouter/avro_file.c b/server/modules/routing/avrorouter/avro_file.c index 73df4ef98..f7bcdaa09 100644 --- a/server/modules/routing/avrorouter/avro_file.c +++ b/server/modules/routing/avrorouter/avro_file.c @@ -31,6 +31,7 @@ */ #include "avrorouter.h" +#include #include #include @@ -1046,6 +1047,25 @@ void handle_query_event(AVRO_INSTANCE *router, REP_HEADER *hdr, int *pending_tra len = tmpsz; unify_whitespace(sql, len); + static bool warn_not_row_format = true; + + if (warn_not_row_format) + { + GWBUF* buffer = gwbuf_alloc(len + 5); + gw_mysql_set_byte3(GWBUF_DATA(buffer), len + 1); + GWBUF_DATA(buffer)[4] = 0x03; + memcpy(GWBUF_DATA(buffer) + 5, sql, len); + qc_query_op_t op = qc_get_operation(buffer); + gwbuf_free(buffer); + + if (op == QUERY_OP_UPDATE || op == QUERY_OP_INSERT || op == QUERY_OP_DELETE) + { + MXS_WARNING("Possible STATEMENT or MIXED format binary log. Check that " + "'binlog_format' is set to ROW on the master."); + warn_not_row_format = false; + } + } + if (is_create_table_statement(router, sql, len)) { TABLE_CREATE *created = NULL; From c64fd4f39f4d52fcb05518c24e6550b03bbe9fb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 5 Jan 2018 08:52:36 +0200 Subject: [PATCH 04/20] MXS-1543: Add test case Added test case that checks that statement based replication is detected. --- maxscale-system-test/CMakeLists.txt | 4 ++++ maxscale-system-test/mxs1543.cpp | 32 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 maxscale-system-test/mxs1543.cpp diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index 31e6c5a21..9177d7daf 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -522,6 +522,10 @@ add_test_executable(mxs1516.cpp mxs1516 replication LABELS REPL_BACKEND) # https://jira.mariadb.org/browse/MXS-1542 add_test_executable(mxs1542.cpp mxs1542 avro LABELS REPL_BACKEND) +# MXS-1543: Avrorouter doesn't detect MIXED or STATEMENT format replication +# https://jira.mariadb.org/browse/MXS-1543 +add_test_executable(mxs1543.cpp mxs1543 avro LABELS REPL_BACKEND) + # MXS-1585: Crash in MaxScale 2.1.12 # https://jira.mariadb.org/browse/MXS-1585 add_test_executable(mxs1585.cpp mxs1585 mxs1585 LABELS REPL_BACKEND) diff --git a/maxscale-system-test/mxs1543.cpp b/maxscale-system-test/mxs1543.cpp new file mode 100644 index 000000000..6d185f92d --- /dev/null +++ b/maxscale-system-test/mxs1543.cpp @@ -0,0 +1,32 @@ +/** + * MXS-1543: https://jira.mariadb.org/browse/MXS-1543 + * + * Avrorouter doesn't detect MIXED or STATEMENT format replication + */ + +#include "testconnections.h" + +int main(int argc, char** argv) +{ + TestConnections::skip_maxscale_start(true); + TestConnections::check_nodes(false); + TestConnections test(argc, argv); + + test.replicate_from_master(); + + test.repl->connect(); + execute_query(test.repl->nodes[0], "CREATE OR REPLACE TABLE t1 (data VARCHAR(30))"); + execute_query(test.repl->nodes[0], "INSERT INTO t1 VALUES ('ROW')"); + execute_query(test.repl->nodes[0], "SET binlog_format=STATEMENT"); + execute_query(test.repl->nodes[0], "FLUSH LOGS"); + execute_query(test.repl->nodes[0], "INSERT INTO t1 VALUES ('STATEMENT')"); + execute_query(test.repl->nodes[0], "SET binlog_format=ROW"); + execute_query(test.repl->nodes[0], "FLUSH LOGS"); + execute_query(test.repl->nodes[0], "INSERT INTO t1 VALUES ('ROW2')"); + + // Wait for the avrorouter to process the data + sleep(10); + test.check_log_err("Possible STATEMENT or MIXED", true); + + return test.global_result; +} From ce1c45828ae366c612935e3e748b470f712b4b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 5 Jan 2018 10:37:43 +0200 Subject: [PATCH 05/20] MXS-1575: Fix CREATE TABLE processing The CREATE TABLE processing failed to identify the explicit database names that were generated by mysqldump. --- server/modules/routing/avrorouter/avro_schema.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/server/modules/routing/avrorouter/avro_schema.c b/server/modules/routing/avrorouter/avro_schema.c index 9bdc47edb..fc61301d3 100644 --- a/server/modules/routing/avrorouter/avro_schema.c +++ b/server/modules/routing/avrorouter/avro_schema.c @@ -421,28 +421,33 @@ static bool get_database_name(const char* sql, char* dest) if (ptr) { ptr--; - while (*ptr == '`' || isspace(*ptr)) + while (ptr >= sql && (*ptr == '`' || isspace(*ptr))) { ptr--; } - while (*ptr != '`' && *ptr != '.' && !isspace(*ptr)) + while (ptr >= sql && *ptr != '`' && *ptr != '.' && !isspace(*ptr)) { ptr--; } - if (*ptr == '.') + while (ptr >= sql && (*ptr == '`' || isspace(*ptr))) + { + ptr--; + } + + if (ptr >= sql && *ptr == '.') { // The query defines an explicit database - while (*ptr == '`' || *ptr == '.' || isspace(*ptr)) + while (ptr >= sql && (*ptr == '`' || *ptr == '.' || isspace(*ptr))) { ptr--; } const char* end = ptr + 1; - while (*ptr != '`' && *ptr != '.' && !isspace(*ptr)) + while (ptr >= sql && *ptr != '`' && *ptr != '.' && !isspace(*ptr)) { ptr--; } @@ -1139,7 +1144,7 @@ void read_alter_identifier(const char *sql, const char *end, char *dest, int siz int len = 0; const char *tok = get_tok(sql, &len, end); // ALTER if (tok && (tok = get_tok(tok + len, &len, end)) // TABLE - && (tok = get_tok(tok + len, &len, end))) // Table identifier + && (tok = get_tok(tok + len, &len, end))) // Table identifier { snprintf(dest, size, "%.*s", len, tok); remove_backticks(dest); From e5b530313792ecf37e1dfea0c33df12173445543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 5 Jan 2018 12:57:09 +0200 Subject: [PATCH 06/20] Initialize the query classifier in tests The test initialization function now loads the query classifier. --- server/core/test/test_utils.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/core/test/test_utils.h b/server/core/test/test_utils.h index 464679bae..5c00c724c 100644 --- a/server/core/test/test_utils.h +++ b/server/core/test/test_utils.h @@ -20,6 +20,7 @@ #include #include #include +#include #include "../maxscale/poll.h" #include "../maxscale/statistics.h" @@ -36,6 +37,8 @@ void init_test_env(char *path) ts_stats_init(); mxs_log_init(NULL, logdir, MXS_LOG_TARGET_DEFAULT); dcb_global_init(); + qc_setup(NULL, NULL); + qc_process_init(QC_INIT_BOTH); poll_init(); hkinit(); } From 3f78dbc9230a438b07b7aa4f7080a83d5bbcf498 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 5 Jan 2018 13:34:46 +0200 Subject: [PATCH 07/20] Improve avrorouter assertion output When an assertion fails due to an overflow of the event buffer, all processed values for that event are dumped. This commit also enables the assertions even for non-debug builds which should speed up the elimination process for bugs in the avrorouter. The overhead of doing this is minimal as the output is already gathered for the INFO level logging. --- server/modules/routing/avrorouter/avro_rbr.c | 60 ++++++++++++++------ 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/server/modules/routing/avrorouter/avro_rbr.c b/server/modules/routing/avrorouter/avro_rbr.c index 3fcf1bf8f..be3907094 100644 --- a/server/modules/routing/avrorouter/avro_rbr.c +++ b/server/modules/routing/avrorouter/avro_rbr.c @@ -17,6 +17,7 @@ #include #include #include +#include #define WRITE_EVENT 0 #define UPDATE_EVENT 1 @@ -468,6 +469,19 @@ int get_metadata_len(uint8_t type) } } +// Make sure that both `i` and `trace` are defined before using this macro +#define check_overflow(t) do \ + { \ + if (!(t)) \ + { \ + for (long x = 0; x < i;x++) \ + { \ + MXS_ALERT("%s", trace[x]); \ + } \ + raise(SIGABRT); \ + } \ + }while(false) + /** * @brief Extract the values from a single row in a row event * @@ -484,7 +498,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value { int npresent = 0; avro_value_t field; - long ncolumns = map->columns; + long ncolumns = MXS_MIN(map->columns, create->columns); uint8_t *metadata = map->column_metadata; size_t metadata_offset = 0; @@ -497,7 +511,10 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value ptr += (ncolumns + 7) / 8; ss_dassert(ptr < end); - for (long i = 0; i < map->columns && i < create->columns && npresent < ncolumns; i++) + char trace[ncolumns][768]; + memset(trace, 0, sizeof(trace)); + + for (long i = 0; i < ncolumns && npresent < ncolumns; i++) { ss_debug(int rc = )avro_value_get_by_name(record, create->column_names[i], &field, NULL); ss_dassert(rc == 0); @@ -507,7 +524,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value npresent++; if (bit_is_set(null_bitmap, ncolumns, i)) { - MXS_INFO("[%ld] NULL", i); + sprintf(trace[i], "[%ld] NULL", i); if (column_is_blob(map->column_types[i])) { uint8_t nullvalue = 0; @@ -529,9 +546,9 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value char strval[bytes * 2 + 1]; gw_bin2hex(strval, val, bytes); avro_value_set_string(&field, strval); - MXS_INFO("[%ld] ENUM: %lu bytes", i, bytes); + sprintf(trace[i], "[%ld] ENUM: %lu bytes", i, bytes); ptr += bytes; - ss_dassert(ptr < end); + check_overflow(ptr < end); } else { @@ -561,13 +578,13 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value bytes = *ptr++; } - MXS_INFO("[%ld] CHAR: field: %d bytes, data: %d bytes", i, field_length, bytes); + sprintf(trace[i], "[%ld] CHAR: field: %d bytes, data: %d bytes", i, field_length, bytes); char str[bytes + 1]; memcpy(str, ptr, bytes); str[bytes] = '\0'; avro_value_set_string(&field, str); ptr += bytes; - ss_dassert(ptr < end); + check_overflow(ptr < end); } } else if (column_is_bit(map->column_types[i])) @@ -584,17 +601,17 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value MXS_WARNING("BIT is not currently supported, values are stored as 0."); } avro_value_set_int(&field, value); - MXS_INFO("[%ld] BIT", i); + sprintf(trace[i], "[%ld] BIT", i); ptr += bytes; - ss_dassert(ptr < end); + check_overflow(ptr < end); } else if (column_is_decimal(map->column_types[i])) { double f_value = 0.0; ptr += unpack_decimal_field(ptr, metadata + metadata_offset, &f_value); avro_value_set_double(&field, f_value); - MXS_INFO("[%ld] DOUBLE", i); - ss_dassert(ptr < end); + sprintf(trace[i], "[%ld] DECIMAL", i); + check_overflow(ptr < end); } else if (column_is_variable_string(map->column_types[i])) { @@ -611,13 +628,13 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value ptr++; } - MXS_INFO("[%ld] VARCHAR: field: %d bytes, data: %lu bytes", i, bytes, sz); + sprintf(trace[i], "[%ld] VARCHAR: field: %d bytes, data: %lu bytes", i, bytes, sz); char buf[sz + 1]; memcpy(buf, ptr, sz); buf[sz] = '\0'; ptr += sz; avro_value_set_string(&field, buf); - ss_dassert(ptr < end); + check_overflow(ptr < end); } else if (column_is_blob(map->column_types[i])) { @@ -625,7 +642,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value uint64_t len = 0; memcpy(&len, ptr, bytes); ptr += bytes; - MXS_INFO("[%ld] BLOB: field: %d bytes, data: %lu bytes", i, bytes, len); + sprintf(trace[i], "[%ld] BLOB: field: %d bytes, data: %lu bytes", i, bytes, len); if (len) { avro_value_set_bytes(&field, ptr, len); @@ -636,7 +653,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value uint8_t nullvalue = 0; avro_value_set_bytes(&field, &nullvalue, 1); } - ss_dassert(ptr < end); + check_overflow(ptr < end); } else if (column_is_temporal(map->column_types[i])) { @@ -647,8 +664,8 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value create->column_lengths[i], &tm); format_temporal_value(buf, sizeof(buf), map->column_types[i], &tm); avro_value_set_string(&field, buf); - MXS_INFO("[%ld] %s: %s", i, column_type_to_string(map->column_types[i]), buf); - ss_dassert(ptr < end); + sprintf(trace[i], "[%ld] %s: %s", i, column_type_to_string(map->column_types[i]), buf); + check_overflow(ptr < end); } /** All numeric types (INT, LONG, FLOAT etc.) */ else @@ -658,11 +675,18 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value ptr += unpack_numeric_field(ptr, map->column_types[i], &metadata[metadata_offset], lval); set_numeric_field_value(&field, map->column_types[i], &metadata[metadata_offset], lval); - ss_dassert(ptr < end); + sprintf(trace[i], "[%ld] %s", i, column_type_to_string(map->column_types[i])); + check_overflow(ptr < end); } ss_dassert(metadata_offset <= map->column_metadata_size); metadata_offset += get_metadata_len(map->column_types[i]); } + else + { + sprintf(trace[i], "[%ld] %s: Not present", i, column_type_to_string(map->column_types[i])); + } + + MXS_INFO("%s", trace[i]); } return ptr; From 579dca07502f4d09fea3cc71500187b66261366f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 5 Jan 2018 15:19:09 +0200 Subject: [PATCH 08/20] Log to stdout in unit tests The log manager will log to stdout to work around directory and file permissions. --- server/core/test/test_utils.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/server/core/test/test_utils.h b/server/core/test/test_utils.h index 5c00c724c..1de1f9059 100644 --- a/server/core/test/test_utils.h +++ b/server/core/test/test_utils.h @@ -22,20 +22,21 @@ #include #include +#include + #include "../maxscale/poll.h" #include "../maxscale/statistics.h" void init_test_env(char *path) { - int argc = 3; - - const char* logdir = path ? path : TEST_LOG_DIR; - config_get_global_options()->n_threads = 1; ts_stats_init(); - mxs_log_init(NULL, logdir, MXS_LOG_TARGET_DEFAULT); + if (!mxs_log_init(NULL, NULL, MXS_LOG_TARGET_STDOUT)) + { + exit(1); + } dcb_global_init(); qc_setup(NULL, NULL); qc_process_init(QC_INIT_BOTH); From 0416d66bcbe70b4f1ae5258f06067a95dc44725f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 5 Jan 2018 20:27:15 +0200 Subject: [PATCH 09/20] Set query classifier with an absolute path in tests Setting the query classifier with an absolute path makes it easier to manage test setup without having to manually resolve the relative path to the query classifier from the test source directory. --- server/core/test/test_utils.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/core/test/test_utils.h b/server/core/test/test_utils.h index 1de1f9059..01fca0667 100644 --- a/server/core/test/test_utils.h +++ b/server/core/test/test_utils.h @@ -21,6 +21,8 @@ #include #include #include +#include +#include #include @@ -38,6 +40,7 @@ void init_test_env(char *path) exit(1); } dcb_global_init(); + set_libdir(MXS_STRDUP(TEST_DIR "/query_classifier/qc_sqlite/")); qc_setup(NULL, NULL); qc_process_init(QC_INIT_BOTH); poll_init(); From a8876fbcb8fa2ba3d29030eb9c4d936e2ab9308f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 8 Jan 2018 12:50:45 +0200 Subject: [PATCH 10/20] Add CONTRIBUTING document and pull request template These templates allow for easier pull requests. --- CONTRIBUTING.md | 27 +++++++++++++++++++++++++++ PULL_REQUEST_TEMPLATE.md | 3 +++ 2 files changed, 30 insertions(+) create mode 100644 CONTRIBUTING.md create mode 100644 PULL_REQUEST_TEMPLATE.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..69af32211 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,27 @@ +# Contributing to Maxscale + +## Prerequisites + +Basically, in order for us to be able to accept a contribution, it must be +licensed under [BSD-new](http://en.wikipedia.org/wiki/BSD_licenses). Upon +request, we can also provide a _contributor agreement_ for you to sign. + +When you submit a pull request, add the following comment to your pull request. + +> I am contributing the new code of the whole pull request, including one or + several files that are either new files or modified ones, under the BSD-new + license. + +Without this comment, the pull request will not be accepted. + +## Practicalities + +* Please ensure that your pull-request has been made against the correct + branch. For bug fixes or minor improvements, use the default branch and for + new code, use the `develop` branch (e.g. for a patch to 2.1.x, use the `2.1` + branch). + +* Please ensure that your code follows our [Coding Style](https://github.com/mariadb-corporation/MaxScale/wiki/Coding-Style-and-Guidelines). + All new code should be formatted with the + [Astyle configuration](https://github.com/mariadb-corporation/MaxScale/wiki/Coding-Style-and-Guidelines#tldr) + provided with the MaxScale source code. diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 000000000..ce348d0f0 --- /dev/null +++ b/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,3 @@ +I am contributing the new code of the whole pull request, including one or +several files that are either new files or modified ones, under the BSD-new +license. From 398dbbc66dc0d537aeaa4498a5059d0dea21adfc Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Mon, 8 Jan 2018 14:28:13 +0200 Subject: [PATCH 11/20] Update 2.1 version number --- VERSION21.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION21.cmake b/VERSION21.cmake index bba5e1b04..4b2078c0d 100644 --- a/VERSION21.cmake +++ b/VERSION21.cmake @@ -5,7 +5,7 @@ set(MAXSCALE_VERSION_MAJOR "2" CACHE STRING "Major version") set(MAXSCALE_VERSION_MINOR "1" CACHE STRING "Minor version") -set(MAXSCALE_VERSION_PATCH "13" CACHE STRING "Patch version") +set(MAXSCALE_VERSION_PATCH "14" CACHE STRING "Patch version") # This should only be incremented if a package is rebuilt set(MAXSCALE_BUILD_NUMBER 1 CACHE STRING "Release number") From f2771d5ad89f864c826661f6cb63088dd9a28bca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 11 Jan 2018 07:47:25 +0200 Subject: [PATCH 12/20] Remove obsolete message The warning that a schema already exists is obsolete as mapped tables are now always opened instead of being reused. This causes the schema checks to be done for each mapped table. --- server/modules/routing/avrorouter/avro_schema.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/server/modules/routing/avrorouter/avro_schema.c b/server/modules/routing/avrorouter/avro_schema.c index fc61301d3..79ef43bd2 100644 --- a/server/modules/routing/avrorouter/avro_schema.c +++ b/server/modules/routing/avrorouter/avro_schema.c @@ -311,10 +311,6 @@ void save_avro_schema(const char *path, const char* schema, TABLE_MAP *map) } } } - else - { - MXS_NOTICE("Schema version %d already exists: %s", map->version, filepath); - } } /** From 5e4b7ae7c760b32db70aa85d7fc49c0ac7afbfe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 11 Jan 2018 08:15:01 +0200 Subject: [PATCH 13/20] Fix memory leak in avrorouter The TABLE_MAP freeing function leaked memory. --- server/modules/routing/avrorouter/avro_schema.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/modules/routing/avrorouter/avro_schema.c b/server/modules/routing/avrorouter/avro_schema.c index 79ef43bd2..ef1e41c27 100644 --- a/server/modules/routing/avrorouter/avro_schema.c +++ b/server/modules/routing/avrorouter/avro_schema.c @@ -1446,6 +1446,8 @@ void table_map_free(TABLE_MAP *map) if (map) { MXS_FREE(map->column_types); + MXS_FREE(map->column_metadata); + MXS_FREE(map->null_bitmap); MXS_FREE(map->database); MXS_FREE(map->table); MXS_FREE(map); From dbad6b737d693b5cb1e795c6fabfa219d209fe78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 11 Jan 2018 08:15:58 +0200 Subject: [PATCH 14/20] Detect redundant table map events When two identical tables are mapped into the same slot, the newer one is ignored. --- server/modules/routing/avrorouter/avro_rbr.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/server/modules/routing/avrorouter/avro_rbr.c b/server/modules/routing/avrorouter/avro_rbr.c index be3907094..8af455076 100644 --- a/server/modules/routing/avrorouter/avro_rbr.c +++ b/server/modules/routing/avrorouter/avro_rbr.c @@ -91,6 +91,15 @@ bool handle_table_map_event(AVRO_INSTANCE *router, REP_HEADER *hdr, uint8_t *ptr TABLE_MAP *old = hashtable_fetch(router->table_maps, table_ident); TABLE_MAP *map = table_map_alloc(ptr, ev_len, create); MXS_ABORT_IF_NULL(map); // Fatal error at this point + + if (old && old->id == map->id && old->version == map->version && + strcmp(old->table, map->table) == 0 && + strcmp(old->database, map->database) == 0) + { + table_map_free(map); + return true; + } + char* json_schema = json_new_schema_from_table(map); if (json_schema) From 9ca6d586d1521c42b19a919b650a1d9d933dbece Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 11 Jan 2018 11:43:27 +0200 Subject: [PATCH 15/20] MXS-1575: Fix Avro schema versioning The versions were not updated if the table was dropped and created again. --- server/modules/routing/avrorouter/avro_schema.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/server/modules/routing/avrorouter/avro_schema.c b/server/modules/routing/avrorouter/avro_schema.c index ef1e41c27..24fe0adf8 100644 --- a/server/modules/routing/avrorouter/avro_schema.c +++ b/server/modules/routing/avrorouter/avro_schema.c @@ -703,6 +703,21 @@ TABLE_CREATE* table_create_from_schema(const char* file, const char* db, return newtable; } +int resolve_table_version(const char* db, const char* table) +{ + int version = 0; + char buf[PATH_MAX + 1]; + + do + { + version++; + snprintf(buf, sizeof(buf), "%s.%s.%06d.avsc", db, table, version); + } + while (access(buf, F_OK) == 0); + + return version; +} + /** * @brief Handle a query event which contains a CREATE TABLE statement * @param sql Query SQL @@ -758,7 +773,7 @@ TABLE_CREATE* table_create_alloc(const char* sql, int len, const char* db) { if ((rval = MXS_MALLOC(sizeof(TABLE_CREATE)))) { - rval->version = 1; + rval->version = resolve_table_version(db, table); rval->was_used = false; rval->column_names = names; rval->column_lengths = lengths; From ab44a941ab0c04f4d1153652c1363700e7327bbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 11 Jan 2018 11:44:33 +0200 Subject: [PATCH 16/20] MXS-1575: Fix large DECIMAL value handling DECIMAL types that were larger than 8 bytes were not handled correctly. The current implementation only prints the lowest 8 bytes of the integer part of the decimal. --- server/core/mysql_binlog.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/server/core/mysql_binlog.c b/server/core/mysql_binlog.c index ae9b6b844..33f6dd395 100644 --- a/server/core/mysql_binlog.c +++ b/server/core/mysql_binlog.c @@ -259,7 +259,7 @@ size_t datetime_sizes[] = */ static void unpack_datetime(uint8_t *ptr, int length, struct tm *dest) { - int64_t val = 0; + uint64_t val = 0; uint32_t second, minute, hour, day, month, year; if (length == -1) @@ -717,6 +717,7 @@ size_t unpack_decimal_field(uint8_t *ptr, uint8_t *metadata, double *val_float) int fpart2 = decimals - fpart1 * dec_dig; int ibytes = ipart1 * 4 + dig_bytes[ipart2]; int fbytes = fpart1 * 4 + dig_bytes[fpart2]; + int field_size = ibytes + fbytes; /** Remove the sign bit and store it locally */ bool negative = (ptr[0] & 0x80) == 0; @@ -735,7 +736,17 @@ size_t unpack_decimal_field(uint8_t *ptr, uint8_t *metadata, double *val_float) } } - int64_t val_i = unpack_bytes(ptr, ibytes); + int64_t val_i = 0; + + if (ibytes > 8) + { + int extra = ibytes - 8; + ptr += extra; + ibytes -= extra; + ss_dassert(ibytes == 8); + } + + val_i = unpack_bytes(ptr, ibytes); int64_t val_f = fbytes ? unpack_bytes(ptr + ibytes, fbytes) : 0; if (negative) @@ -746,5 +757,5 @@ size_t unpack_decimal_field(uint8_t *ptr, uint8_t *metadata, double *val_float) *val_float = (double)val_i + ((double)val_f / (pow(10.0, decimals))); - return ibytes + fbytes; + return field_size; } From 847dd2c1204d4e0a779d802755f8ad8587eb87fb Mon Sep 17 00:00:00 2001 From: Timofey Turenko Date: Mon, 15 Jan 2018 16:47:15 +0200 Subject: [PATCH 17/20] fix typo in read_env() and set_env.sh --- maxscale-system-test/mariadb_nodes.cpp | 2 +- maxscale-system-test/mdbci/set_env.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/maxscale-system-test/mariadb_nodes.cpp b/maxscale-system-test/mariadb_nodes.cpp index 8e43d151b..1a51292a8 100644 --- a/maxscale-system-test/mariadb_nodes.cpp +++ b/maxscale-system-test/mariadb_nodes.cpp @@ -227,7 +227,7 @@ int Mariadb_nodes::read_env() } else { - sprintf(start_db_command[i], "%s", "service mysql stop"); + sprintf(stop_db_command[i], "%s", "service mysql stop"); } //reading cleanup_db_command diff --git a/maxscale-system-test/mdbci/set_env.sh b/maxscale-system-test/mdbci/set_env.sh index da1608a19..86459e8d1 100644 --- a/maxscale-system-test/mdbci/set_env.sh +++ b/maxscale-system-test/mdbci/set_env.sh @@ -70,7 +70,7 @@ do else ${mdbci_dir}/mdbci ssh --command 'echo \"/usr/sbin/mysqld \$* 2> stderr.log > stdout.log &\" > mysql_start.sh; echo \"sleep 20\" >> mysql_start.sh; echo \"disown\" >> mysql_start.sh; chmod a+x mysql_start.sh' $config_name/node_$num --silent eval 'export $start_cmd_var="/home/$au/mysql_start.sh "' - eval 'export $start_cmd_var="killall mysqld "' + eval 'export $stop_cmd_var="killall mysqld "' fi else eval 'export $start_cmd_var="$mysql_exe start "' From 2b653887a34bcb47845182b8c52bdfd11dc5ecf7 Mon Sep 17 00:00:00 2001 From: Dapeng Huang <7xerocha@gmail.com> Date: Sat, 13 Jan 2018 12:41:05 +0800 Subject: [PATCH 18/20] fix crash at execute empty flush command --- server/modules/routing/maxinfo/maxinfo_exec.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/modules/routing/maxinfo/maxinfo_exec.c b/server/modules/routing/maxinfo/maxinfo_exec.c index f4a37f967..bebe940fd 100644 --- a/server/modules/routing/maxinfo/maxinfo_exec.c +++ b/server/modules/routing/maxinfo/maxinfo_exec.c @@ -354,6 +354,13 @@ exec_flush(DCB *dcb, MAXINFO_TREE *tree) int i; char errmsg[120]; + sprintf(errmsg, "Unsupported flush command '%s'", tree->value); + if(!tree) + { + maxinfo_send_error(dcb, 0, errmsg); + MXS_ERROR("%s", errmsg); + return; + } for (i = 0; flush_commands[i].name; i++) { if (strcasecmp(flush_commands[i].name, tree->value) == 0) @@ -366,7 +373,6 @@ exec_flush(DCB *dcb, MAXINFO_TREE *tree) { tree->value[80] = 0; } - sprintf(errmsg, "Unsupported flush command '%s'", tree->value); maxinfo_send_error(dcb, 0, errmsg); MXS_ERROR("%s", errmsg); } From 454f195cb03d05e5692f18170fc26db0ed36b27d Mon Sep 17 00:00:00 2001 From: Dapeng Huang <7xerocha@gmail.com> Date: Sat, 13 Jan 2018 12:54:12 +0800 Subject: [PATCH 19/20] fix:cannot connect to maxinfo with python client --- .gitignore | 9 +++++++++ server/modules/routing/maxinfo/maxinfo.c | 10 +++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 70154a506..f47acb69a 100644 --- a/.gitignore +++ b/.gitignore @@ -51,3 +51,12 @@ CMakeFiles/* */*/*/*/CMakeFiles/* Makefile /.DS_Store + +# Netbeans Project files +nbproject/ +/build/ + +# RBCommons +.reviewboardrc +# vscode +.vscode diff --git a/server/modules/routing/maxinfo/maxinfo.c b/server/modules/routing/maxinfo/maxinfo.c index 63c84e3bf..81646620a 100644 --- a/server/modules/routing/maxinfo/maxinfo.c +++ b/server/modules/routing/maxinfo/maxinfo.c @@ -61,7 +61,7 @@ static int maxinfo_statistics(INFO_INSTANCE *, INFO_SESSION *, GWBUF *); static int maxinfo_ping(INFO_INSTANCE *, INFO_SESSION *, GWBUF *); static int maxinfo_execute_query(INFO_INSTANCE *, INFO_SESSION *, char *); static int handle_url(INFO_INSTANCE *instance, INFO_SESSION *router_session, GWBUF *queue); - +static int maxinfo_send_ok(DCB *dcb); /* The router entry points */ static MXS_ROUTER *createInstance(SERVICE *service, char **options); @@ -356,7 +356,7 @@ execute(MXS_ROUTER *rinstance, MXS_ROUTER_SESSION *router_session, GWBUF *queue) switch (MYSQL_COMMAND(queue)) { case COM_PING: - rc = maxinfo_ping(instance, session, queue); + rc = maxinfo_send_ok(session->dcb); break; case COM_STATISTICS: rc = maxinfo_statistics(instance, session, queue); @@ -618,7 +618,7 @@ maxinfo_execute_query(INFO_INSTANCE *instance, INFO_SESSION *session, char *sql) respond_starttime(session->dcb); return 1; } - if (strcasecmp(sql, "set names 'utf8'") == 0) + if (strncasecmp(sql, "set names", 9) == 0) { return maxinfo_send_ok(session->dcb); } @@ -626,6 +626,10 @@ maxinfo_execute_query(INFO_INSTANCE *instance, INFO_SESSION *session, char *sql) { return maxinfo_send_ok(session->dcb); } + if (strncasecmp(sql, "set @@session", 13) == 0) + { + return maxinfo_send_ok(session->dcb); + } if (strncasecmp(sql, "set autocommit", 14) == 0) { return maxinfo_send_ok(session->dcb); From 76e53f2081d816af7aed13de77e44a2fc39d6a04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 16 Jan 2018 08:42:40 +0200 Subject: [PATCH 20/20] Make contribution instructions more concise The instructions now separate the two parts more clearly so that bug fixes are always done to the oldest possible version. --- CONTRIBUTING.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 69af32211..61f666a74 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,9 +17,8 @@ Without this comment, the pull request will not be accepted. ## Practicalities * Please ensure that your pull-request has been made against the correct - branch. For bug fixes or minor improvements, use the default branch and for - new code, use the `develop` branch (e.g. for a patch to 2.1.x, use the `2.1` - branch). + branch. For bug fixes or minor improvements, use the default branch (at the + time of writing `2.1`). For new features, use the `develop` branch. * Please ensure that your code follows our [Coding Style](https://github.com/mariadb-corporation/MaxScale/wiki/Coding-Style-and-Guidelines). All new code should be formatted with the