diff --git a/BUILD/mdbci/build.sh b/BUILD/mdbci/build.sh index abdb0f4b2..a18c16f92 100755 --- a/BUILD/mdbci/build.sh +++ b/BUILD/mdbci/build.sh @@ -66,9 +66,7 @@ $(<${script_dir}/templates/build.json.template) # destroying existing box if [ -d "$MDBCI_VM_PATH/${name}" ]; then - cd $MDBCI_VM_PATH/${name} - vagrant destroy -f - cd ${dir} + ${mdbci_dir}/mdbci destroy $name fi # starting VM for build @@ -78,9 +76,7 @@ $(<${script_dir}/templates/build.json.template) ${mdbci_dir}/mdbci up --attempts=1 $name if [ $? != 0 ] ; then echo "Error starting VM" - cd $MDBCI_VM_PATH/${name} rm ~/vagrant_lock - cd $dir exit 1 fi echo "copying public keys to VM" @@ -122,11 +118,7 @@ if [ "$try_already_running" == "yes" ] ; then fi if [[ "$do_not_destroy_vm" != "yes" && "$try_already_running" != "yes" ]] ; then echo "Destroying VM" - vagrant destroy -f - cd .. - rm -rf $name - rm -rf ${name}.json - rm -rf ${name}_netwotk_config + ${mdbci_dir}/mdbci destroy $name fi cd $dir diff --git a/BUILD/mdbci/upgrade_test.sh b/BUILD/mdbci/upgrade_test.sh index 530969a5c..45b6974b1 100755 --- a/BUILD/mdbci/upgrade_test.sh +++ b/BUILD/mdbci/upgrade_test.sh @@ -30,9 +30,7 @@ echo $JOB_NAME-$BUILD_NUMBER >> ~/vagrant_lock # destroying existing box if [ -d "install_$box" ]; then - cd $MDBCI_VM_PATH/$name - vagrant destroy -f - cd $dir + ${mdbci_dir}/mdbci destroy $name fi ${mdbci_dir}/repository-config/generate_all.sh repo.d @@ -44,11 +42,9 @@ ${mdbci_dir}/mdbci up $name --attempts=1 if [ $? != 0 ] ; then if [ $? != 0 ] ; then echo "Error starting VM" - cd ${MDBCI_VM_PATH}/$name if [ "x$do_not_destroy_vm" != "xyes" ] ; then - vagrant destroy -f + ${mdbci_dir}/mdbci destroy $name fi - cd $dir rm ~/vagrant_lock exit 1 fi @@ -126,9 +122,7 @@ scp $scpopt $sshuser@$IP:/var/log/maxscale/* $logs_publish_dir chmod a+r $logs_publish_dir/* if [ "x$do_not_destroy_vm" != "xyes" ] ; then - cd $MDBCI_VM_PATH/$name - vagrant destroy -f - cd $dir + ${mdbci_dir}/mdbci destroy $name fi kill $pid_to_kill exit $res diff --git a/Documentation/Changelog.md b/Documentation/Changelog.md index e7e0109dc..ccf10bb10 100644 --- a/Documentation/Changelog.md +++ b/Documentation/Changelog.md @@ -28,6 +28,7 @@ the master. There is also limited capability for rejoining nodes. For more details, please refer to: +* [MariaDB MaxScale 2.2.2 Release Notes](Release-Notes/MaxScale-2.2.2-Release-Notes.md) * [MariaDB MaxScale 2.2.1 Release Notes](Release-Notes/MaxScale-2.2.1-Release-Notes.md) * [MariaDB MaxScale 2.2.0 Release Notes](Release-Notes/MaxScale-2.2.0-Release-Notes.md) diff --git a/Documentation/Filters/Database-Firewall-Filter.md b/Documentation/Filters/Database-Firewall-Filter.md index 18eb8dd2a..704b585c4 100644 --- a/Documentation/Filters/Database-Firewall-Filter.md +++ b/Documentation/Filters/Database-Firewall-Filter.md @@ -355,6 +355,12 @@ After the matching part comes the rules keyword after which a list of rule names is expected. This allows reusing of the rules and enables varying levels of query restriction. +If a particular _NAME_ appears on several `users` lines, then when an +actual user matches that name, the rules of each line are checked +independently until there is a match for the statement in question. That +is, the rules of each `users` line are treated in an _OR_ fashion with +respect to each other. + ## Module commands Read [Module Commands](../Reference/Module-Commands.md) documentation for diff --git a/Documentation/Getting-Started/Configuration-Guide.md b/Documentation/Getting-Started/Configuration-Guide.md index 696b7101a..d72e19867 100644 --- a/Documentation/Getting-Started/Configuration-Guide.md +++ b/Documentation/Getting-Started/Configuration-Guide.md @@ -688,6 +688,17 @@ Note that MariaDB MaxScale is **not** explicitly aware of the sql mode of the server, so the value of `sql_mode` should reflect the sql mode used when the server is started. +#### `local_address` + +What specific local address/interface to use when connecting to servers. + +This can be used for ensuring that MaxScale uses a particular interface +when connecting to servers, in case the computer MaxScale is running on +has multiple interfaces. +``` +local_address=192.168.1.254 +``` + ### Service A service represents the database service that MariaDB MaxScale offers to the diff --git a/Documentation/Release-Notes/MaxScale-2.1.14-Release-Notes.md b/Documentation/Release-Notes/MaxScale-2.1.14-Release-Notes.md new file mode 100644 index 000000000..eb9f4426f --- /dev/null +++ b/Documentation/Release-Notes/MaxScale-2.1.14-Release-Notes.md @@ -0,0 +1,63 @@ +# MariaDB MaxScale 2.1.14 Release Notes + +Release 2.1.14 is a GA release. + +This document describes the changes in release 2.1.14, when compared +to release [2.1.13](MaxScale-2.1.13-Release-Notes.md). + +If you are upgrading from release 2.0, please also read the following +release notes: + +* [2.1.13](./MaxScale-2.1.13-Release-Notes.md) +* [2.1.12](./MaxScale-2.1.12-Release-Notes.md) +* [2.1.11](./MaxScale-2.1.11-Release-Notes.md) +* [2.1.10](./MaxScale-2.1.10-Release-Notes.md) +* [2.1.9](./MaxScale-2.1.9-Release-Notes.md) +* [2.1.8](./MaxScale-2.1.8-Release-Notes.md) +* [2.1.7](./MaxScale-2.1.7-Release-Notes.md) +* [2.1.6](./MaxScale-2.1.6-Release-Notes.md) +* [2.1.5](./MaxScale-2.1.5-Release-Notes.md) +* [2.1.4](./MaxScale-2.1.4-Release-Notes.md) +* [2.1.3](./MaxScale-2.1.3-Release-Notes.md) +* [2.1.2](./MaxScale-2.1.2-Release-Notes.md) +* [2.1.1](./MaxScale-2.1.1-Release-Notes.md) +* [2.1.0](./MaxScale-2.1.0-Release-Notes.md) + +For any problems you encounter, please consider submitting a bug report at +[Jira](https://jira.mariadb.org). + +## New Features + +### Local Address + +It is now possible to specify what local address MaxScale should +use when connecting to servers. Please refer to the documentation +for [details](../Getting-Started/Configuration-Guide.md#local_address). + +## Bug fixes + +[Here is a list of bugs fixed in MaxScale 2.1.14.](https://jira.mariadb.org/issues/?jql=project%20%3D%20MXS%20AND%20issuetype%20%3D%20Bug%20AND%20status%20%3D%20Closed%20AND%20fixVersion%20%3D%202.1.14) + +* [MXS-1627](https://jira.mariadb.org/browse/MXS-1627) MySQLAuth loads users that use authentication plugins +* [MXS-1620](https://jira.mariadb.org/browse/MXS-1620) CentOS package symbols are stripped +* [MXS-1602](https://jira.mariadb.org/browse/MXS-1602) cannot connect to maxinfo with python client +* [MXS-1601](https://jira.mariadb.org/browse/MXS-1601) maxinfo crash at execute query 'flush;' +* [MXS-1600](https://jira.mariadb.org/browse/MXS-1600) maxscale it seen to not coop well with lower-case-table-names=1 on cnf +* [MXS-1576](https://jira.mariadb.org/browse/MXS-1576) Maxscale crashes when starting if .avro and .avsc files are present +* [MXS-1543](https://jira.mariadb.org/browse/MXS-1543) Avrorouter doesn't detect MIXED or STATEMENT format replication +* [MXS-1416](https://jira.mariadb.org/browse/MXS-1416) maxscale should not try to do anything when started with --config-check + +## Packaging + +RPM and Debian packages are provided for the Linux distributions supported by +MariaDB Enterprise. + +Packages can be downloaded [here](https://mariadb.com/resources/downloads). + +## Source Code + +The source code of MaxScale is tagged at GitHub with a tag, which is identical +with the version of MaxScale. For instance, the tag of version X.Y.Z of MaxScale +is maxscale-X.Y.Z. + +The source code is available [here](https://github.com/mariadb-corporation/MaxScale). diff --git a/Documentation/Release-Notes/MaxScale-2.2.2-Release-Notes.md b/Documentation/Release-Notes/MaxScale-2.2.2-Release-Notes.md new file mode 100644 index 000000000..a27d13a46 --- /dev/null +++ b/Documentation/Release-Notes/MaxScale-2.2.2-Release-Notes.md @@ -0,0 +1,48 @@ +# MariaDB MaxScale 2.2.2 Release Notes + +Release 2.2.2 is a XXX release. + +This document describes the changes in release 2.2.2, when compared to +release 2.2.1. + +For any problems you encounter, please consider submitting a bug +report at [Jira](https://jira.mariadb.org). + +## Changed Features + +## Dropped Features + +## New Features + +## Bug fixes + +[Here is a list of bugs fixed in MaxScale 2.2.2.](https://jira.mariadb.org/issues/?jql=project%20%3D%20MXS%20AND%20issuetype%20%3D%20Bug%20AND%20status%20%3D%20Closed%20AND%20fixVersion%20%3D%202.2.2) + +* [MXS-1630](https://jira.mariadb.org/browse/MXS-1630) MaxCtrl binary are not included by default in MaxScale package +* [MXS-1620](https://jira.mariadb.org/browse/MXS-1620) CentOS package symbols are stripped +* [MXS-1615](https://jira.mariadb.org/browse/MXS-1615) Masking filter accesses wrong command argument. +* [MXS-1614](https://jira.mariadb.org/browse/MXS-1614) MariaDBMon yet adding mysqlbackend as the protocol instead of mariadbbackend +* [MXS-1606](https://jira.mariadb.org/browse/MXS-1606) After enabling detect_replication_lag MariaDBMon does not create the maxscale_schema.replication_heartbeat database and table +* [MXS-1604](https://jira.mariadb.org/browse/MXS-1604) PamAuth Default Authentication is Different from MariaDB +* [MXS-1591](https://jira.mariadb.org/browse/MXS-1591) Adding get_lock and release_lock support +* [MXS-1539](https://jira.mariadb.org/browse/MXS-1539) Authentication data should be thread specific + +## Known Issues and Limitations + +There are some limitations and known issues within this version of MaxScale. +For more information, please refer to the [Limitations](../About/Limitations.md) document. + +## Packaging + +RPM and Debian packages are provided for the Linux distributions supported +by MariaDB Enterprise. + +Packages can be downloaded [here](https://mariadb.com/resources/downloads). + +## Source Code + +The source code of MaxScale is tagged at GitHub with a tag, which is identical +with the version of MaxScale. For instance, the tag of version X.Y.Z of MaxScale +is X.Y.Z. Further, *master* always refers to the latest released non-beta version. + +The source code is available [here](https://github.com/mariadb-corporation/MaxScale). diff --git a/VERSION22.cmake b/VERSION22.cmake index 30e294c04..ff9093763 100644 --- a/VERSION22.cmake +++ b/VERSION22.cmake @@ -5,7 +5,7 @@ set(MAXSCALE_VERSION_MAJOR "2" CACHE STRING "Major version") set(MAXSCALE_VERSION_MINOR "2" CACHE STRING "Minor version") -set(MAXSCALE_VERSION_PATCH "1" CACHE STRING "Patch version") +set(MAXSCALE_VERSION_PATCH "2" CACHE STRING "Patch version") # This should only be incremented if a package is rebuilt set(MAXSCALE_BUILD_NUMBER 1 CACHE STRING "Release number") diff --git a/connectors/cdc-connector/cdc_connector.cpp b/connectors/cdc-connector/cdc_connector.cpp index fa9628f70..be751488a 100644 --- a/connectors/cdc-connector/cdc_connector.cpp +++ b/connectors/cdc-connector/cdc_connector.cpp @@ -254,28 +254,32 @@ bool Connection::connect(const std::string& table, const std::string& gtid) m_error = "Failed to set socket non-blocking: "; m_error += strerror_r(errno, err, sizeof(err)); } - else if (do_auth() && do_registration()) + else { - std::string req_msg(REQUEST_MSG); - req_msg += table; + m_fd = c_fd.release(); + m_connected = true; - if (gtid.length()) + if (do_auth() && do_registration()) { - req_msg += " "; - req_msg += gtid; - } + std::string req_msg(REQUEST_MSG); + req_msg += table; - if (nointr_write(req_msg.c_str(), req_msg.length()) == -1) - { - char err[ERRBUF_SIZE]; - m_error = "Failed to write request: "; - m_error += strerror_r(errno, err, sizeof(err)); - } - else if ((m_first_row = read())) - { - rval = true; - m_connected = true; - m_fd = c_fd.release(); + if (gtid.length()) + { + req_msg += " "; + req_msg += gtid; + } + + if (nointr_write(req_msg.c_str(), req_msg.length()) == -1) + { + char err[ERRBUF_SIZE]; + m_error = "Failed to write request: "; + m_error += strerror_r(errno, err, sizeof(err)); + } + else if ((m_first_row = read())) + { + rval = true; + } } } } @@ -438,11 +442,12 @@ bool Connection::do_auth() std::string auth_str = generateAuthString(m_user, m_password); /** Send the auth string */ - if (nointr_write(auth_str.c_str(), auth_str.length()) == -1) + int rc = nointr_write(auth_str.c_str(), auth_str.length()); + if (rc <= 0) { char err[ERRBUF_SIZE]; m_error = "Failed to write authentication data: "; - m_error += strerror_r(errno, err, sizeof(err)); + m_error += rc == -1 ? strerror_r(errno, err, sizeof(err)) : "Write timeout"; } else { @@ -460,7 +465,8 @@ bool Connection::do_auth() { buf[bytes] = '\0'; m_error = "Authentication failed: "; - m_error += buf; + m_error += bytes > 0 ? buf : "Request timed out"; + } else { @@ -687,11 +693,12 @@ int Connection::nointr_read(void *dest, size_t size) int Connection::nointr_write(const void *src, size_t size) { int rc = 0; - int n_bytes = 0; + size_t n_bytes = 0; + const uint8_t* ptr = static_cast(src); - if (wait_for_event(POLLOUT) > 0) + do { - while ((rc = ::write(m_fd, src, size)) < 0 && errno == EINTR) + while ((rc = ::write(m_fd, ptr, size)) < 0 && errno == EINTR) { ; } @@ -706,8 +713,11 @@ int Connection::nointr_write(const void *src, size_t size) else if (rc > 0) { n_bytes += rc; + ptr += rc; + size -= rc; } } + while (n_bytes < size && wait_for_event(POLLOUT) > 0); return n_bytes; } diff --git a/include/maxscale/config.h b/include/maxscale/config.h index 42f7c5bbf..91ae6e924 100644 --- a/include/maxscale/config.h +++ b/include/maxscale/config.h @@ -231,6 +231,7 @@ typedef struct int query_retries; /**< Number of times a interrupted query is retried */ time_t query_retry_timeout; /**< Timeout for query retries */ bool substitute_variables; /**< Should environment variables be substituted */ + char* local_address; /**< Local address to use when connecting */ } MXS_CONFIG; /** diff --git a/include/maxscale/monitor.h b/include/maxscale/monitor.h index 676f4a494..b8d9450d8 100644 --- a/include/maxscale/monitor.h +++ b/include/maxscale/monitor.h @@ -375,4 +375,13 @@ void store_server_journal(MXS_MONITOR *monitor, MXS_MONITORED_SERVER *master); */ void load_server_journal(MXS_MONITOR *monitor, MXS_MONITORED_SERVER **master); +/** + * Find the monitored server representing the server. + * + * @param mon Cluster monitor + * @param search_server Server to search for + * @return Found monitored server or NULL if not found + */ +MXS_MONITORED_SERVER* mon_get_monitored_server(MXS_MONITOR* mon, SERVER* search_server); + MXS_END_DECLS diff --git a/maxctrl/CMakeLists.txt b/maxctrl/CMakeLists.txt index 6516a82eb..3105cb14e 100644 --- a/maxctrl/CMakeLists.txt +++ b/maxctrl/CMakeLists.txt @@ -20,5 +20,5 @@ if (BUILD_MAXCTRL) message(FATAL_ERROR "Not building MaxCtrl: npm or Node.js >= 6.0.0 not found. Add the following to skip MaxCtrl: -DBUILD_MAXCTRL=N") endif() else() - messages(STATUS "Not building MaxCtrl: BUILD_MAXCTRL=N") + message(STATUS "Not building MaxCtrl: BUILD_MAXCTRL=N") endif() diff --git a/maxscale-system-test/.gitignore b/maxscale-system-test/.gitignore index 5d831b984..561b7c601 100644 --- a/maxscale-system-test/.gitignore +++ b/maxscale-system-test/.gitignore @@ -106,6 +106,7 @@ kill_master large_insert_hang load_balancing load_balancing_galera +local_address long_sysbench longblob lots_of_rows @@ -143,6 +144,7 @@ mxs1457_ignore_deleted mxs1468 mxs1476 mxs1509 +mxs1583_fwf mxs244_prepared_stmt_loop mxs280_select_outfile mxs314 diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index f497a20fc..c934225dc 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -50,10 +50,15 @@ add_library(testcore SHARED testconnections.cpp nodes.cpp mariadb_nodes.cpp maxs mariadb_func.cpp get_com_select_insert.cpp maxadmin_operations.cpp big_transaction.cpp sql_t1.cpp test_binlog_fnc.cpp get_my_ip.cpp big_load.cpp get_com_select_insert.cpp different_size.cpp fw_copy_rules maxinfo_func.cpp config_operations.cpp rds_vpc.cpp execute_cmd.cpp - blob_test.cpp) -target_link_libraries(testcore ${MYSQL_CLIENT} ${CDC_CONNECTOR_LIBRARIES} ${JANSSON_LIBRARIES} z nsl m pthread ssl dl rt crypto crypt) + blob_test.cpp + # Include the CDC connector in the core library + ${CMAKE_SOURCE_DIR}/../connectors/cdc-connector/cdc_connector.cpp) +target_link_libraries(testcore ${MYSQL_CLIENT} ${JANSSON_LIBRARIES} z nsl m pthread ssl dl rt crypto crypt) install(TARGETS testcore DESTINATION system-test) -add_dependencies(testcore connector-c cdc_connector jansson) +add_dependencies(testcore connector-c jansson) + +# Include the CDC connector headers +include_directories(${CMAKE_SOURCE_DIR}/../connectors/cdc-connector/) # Tool used to check backend state add_test_executable_notest(check_backend.cpp check_backend check_backend LABELS CONFIG) @@ -249,6 +254,9 @@ add_test_executable(different_size_rwsplit.cpp different_size_rwsplit replicatio # Tries to use 'maxkeys', 'maxpasswrd' add_test_executable(encrypted_passwords.cpp encrypted_passwords replication LABELS maxscale LIGHT REPL_BACKEND) +# Basic MaxCtrl test +add_test_executable(maxctrl_basic.cpp maxctrl_basic replication LABELS maxctrl REPL_BACKEND) + # MySQL Monitor Failover Test add_test_executable(mysqlmon_detect_standalone_master.cpp mysqlmon_detect_standalone_master mysqlmon_detect_standalone_master LABELS mysqlmon REPL_BACKEND) @@ -313,6 +321,7 @@ add_test_executable(fwf_logging.cpp fwf_logging fwf_logging LABELS dbfwfilter RE add_test_executable(fwf_reload.cpp fwf_reload fwf LABELS dbfwfilter REPL_BACKEND) add_test_executable(fwf_syntax.cpp fwf_syntax fwf_syntax LABELS dbfwfilter REPL_BACKEND) add_test_executable(fwf_com_ping.cpp fwf_com_ping fwf_com_ping LABELS dbfwfilter REPL_BACKEND) +add_test_executable(mxs1583_fwf.cpp mxs1583_fwf mxs1583_fwf LABELS dbfwfilter REPL_BACKEND) # Galera node priority test add_test_executable(galera_priority.cpp galera_priority galera_priority LABELS galeramon LIGHT GALERA_BACKEND) @@ -474,7 +483,7 @@ add_test_script(mxs791_galera.sh mxs791_galera.sh galera LABELS UNSTABLE HEAVY G add_test_executable(mxs812_1.cpp mxs812_1 longblob LABELS readwritesplit REPL_BACKEND) # Checks "Current no. of conns" maxadmin output after long blob inserting -add_test_executable(mxs812_2.cpp mxs812_2 longblob LABELS readwritesplit REPL_BACKEND) +add_test_executable(mxs812_2.cpp mxs812_2 replication LABELS readwritesplit REPL_BACKEND) # Execute prepared statements while master is blocked, checks "Current no. of conns" after the test add_test_executable(mxs822_maxpasswd.cpp mxs822_maxpasswd maxpasswd LABELS maxscale REPL_BACKEND) @@ -831,6 +840,9 @@ add_test_executable_notest(create_rds.cpp create_rds replication LABELS EXTERN_B # start sysbench ageints RWSplit for infinite execution add_test_executable_notest(long_sysbench.cpp long_sysbench replication LABELS readwritesplit REPL_BACKEND) +# test effect of local_address in configuration file +add_test_executable(local_address.cpp local_address local_address LABELS REPL_BACKEND) + configure_file(templates.h.in templates.h @ONLY) include(CTest) diff --git a/maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp b/maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp index 65ee60ae6..9ce63da30 100644 --- a/maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp +++ b/maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp @@ -28,6 +28,7 @@ static const char* integer_values[] = "-1", "20", "-20", + "NULL", NULL }; @@ -36,6 +37,7 @@ static const char* decimal_types[] = "FLOAT", "DOUBLE", "DECIMAL(10, 2)", + "DECIMAL(32, 2)", NULL }; @@ -46,6 +48,7 @@ static const char* decimal_values[] = "-1.5", "20.5", "-20.5", + "NULL", NULL }; @@ -64,7 +67,7 @@ static const char* string_values[] = { "\"Hello world!\"", "\"The quick brown fox jumps over the lazy dog\"", -// "\"The Unicode should work: äöåǢ\"", + "NULL", NULL }; @@ -84,8 +87,59 @@ static const char* binary_values[] = "\"Hello world!\"", "\"The quick brown fox jumps over the lazy dog\"", "NULL", -// "\"The Unicode should work: äöåǢ\"", -// "\"These should work for binary types: ⦿☏☃☢😤😂\"", + NULL +}; + +static const char* datetime_types[] = +{ + "DATETIME", + "DATETIME(1)", + "DATETIME(2)", + "DATETIME(3)", + "DATETIME(4)", + "DATETIME(5)", + "DATETIME(6)", + // TODO: Fix test setup to use same timezone + // "TIMESTAMP", + NULL +}; + +static const char* datetime_values[] = +{ + "'2018-01-01 11:11:11'", + "NULL", + NULL +}; + +static const char* date_types[] = +{ + "DATE", + NULL +}; + +static const char* date_values[] = +{ + "'2018-01-01'", + "NULL", + NULL +}; + +static const char* time_types[] = +{ + "TIME", + "TIME(1)", + "TIME(2)", + "TIME(3)", + "TIME(4)", + "TIME(5)", + "TIME(6)", + NULL +}; + +static const char* time_values[] = +{ + "'12:00:00'", + "NULL", NULL }; @@ -95,10 +149,13 @@ struct const char** values; } test_set[] { - { integer_types, integer_values }, - { decimal_types, decimal_values }, - { string_types, string_values }, - { binary_types, binary_values }, + { integer_types, integer_values }, + { decimal_types, decimal_values }, + { string_types, string_values }, + { binary_types, binary_values }, + { datetime_types, datetime_values }, + { date_types, date_values }, + { time_types, time_values }, { 0 } }; diff --git a/maxscale-system-test/cdc_datatypes/cdc_result.h b/maxscale-system-test/cdc_datatypes/cdc_result.h index 2475230d6..03c209855 100644 --- a/maxscale-system-test/cdc_datatypes/cdc_result.h +++ b/maxscale-system-test/cdc_datatypes/cdc_result.h @@ -39,8 +39,10 @@ public: bool operator ==(const TestOutput& output) const { return m_value == output.getValue() || - (m_type.find("BLOB") != std::string::npos && - output.getValue().length() == 0); + (m_type.find("BLOB") != std::string::npos && output.getValue().length() == 0) || + // A NULL timestamp appears to be inserted as NOW() by default in 10.2, a NULL INT is + // inserted as 0 and a NULL string gets converted into an empty string by the CDC system + (m_value == "NULL" && (output.getValue().empty() || m_type == "TIMESTAMP" || output.getValue() == "0")); } bool operator !=(const TestOutput& output) const diff --git a/maxscale-system-test/check_backend.cpp b/maxscale-system-test/check_backend.cpp index 85b8fa07d..f9a62372c 100644 --- a/maxscale-system-test/check_backend.cpp +++ b/maxscale-system-test/check_backend.cpp @@ -8,11 +8,17 @@ int main(int argc, char *argv[]) { - int exit_code; + TestConnections * Test = new TestConnections(argc, argv); - /*Test->maxscales->restart_maxscale(0); - sleep(5);*/ + // Reset server settings by replacing the config files + Test->repl->reset_server_settings(); + + std::string src = std::string(test_dir) + "/mdbci/add_core_cnf.sh"; + Test->maxscales->copy_to_node(0, src.c_str(), Test->maxscales->access_homedir[0]); + Test->maxscales->ssh_node_f(0, true, "%s/add_core_cnf.sh %s", Test->maxscales->access_homedir[0], + Test->verbose ? "verbose" : ""); + Test->set_timeout(10); Test->tprintf("Connecting to Maxscale maxscales->routers[0] with Master/Slave backend\n"); @@ -40,6 +46,7 @@ int main(int argc, char *argv[]) Test->maxscales->close_maxscale_connections(0); Test->check_maxscale_alive(0); + int exit_code = 0; char * ver = Test->maxscales->ssh_node_output(0, "maxscale --version-full", false, &exit_code); Test->tprintf("Maxscale_full_version_start:\n%s\nMaxscale_full_version_end\n", ver); diff --git a/maxscale-system-test/cnf/maxscale.cnf.template.local_address b/maxscale-system-test/cnf/maxscale.cnf.template.local_address new file mode 100644 index 000000000..44bf3ae38 --- /dev/null +++ b/maxscale-system-test/cnf/maxscale.cnf.template.local_address @@ -0,0 +1,90 @@ +[maxscale] +threads=###threads### +###local_address### + +[MySQL Monitor] +type=monitor +module=mysqlmon +###repl51### +servers=server1,server2,server3,server4 +user=maxskysql +passwd=skysql +monitor_interval=1000 +detect_stale_master=false + +[RW Split Router] +type=service +router=readwritesplit +servers=server1,server2,server3,server4 +user=maxskysql +passwd=skysql +router_options=slave_selection_criteria=LEAST_GLOBAL_CONNECTIONS +max_slave_connections=1 + +[Read Connection Router Slave] +type=service +router=readconnroute +router_options=slave +servers=server1,server2,server3,server4 +user=maxskysql +passwd=skysql + +[Read Connection Router Master] +type=service +router=readconnroute +router_options=master +servers=server1,server2,server3,server4 +user=maxskysql +passwd=skysql + +[RW Split Listener] +type=listener +service=RW Split Router +protocol=MySQLClient +port=4006 + +[Read Connection Listener Slave] +type=listener +service=Read Connection Router Slave +protocol=MySQLClient +port=4009 + +[Read Connection Listener Master] +type=listener +service=Read Connection Router Master +protocol=MySQLClient +port=4008 + +[CLI] +type=service +router=cli + +[CLI Listener] +type=listener +service=CLI +protocol=maxscaled +socket=default + +[server1] +type=server +address=###node_server_IP_1### +port=###node_server_port_1### +protocol=MySQLBackend + +[server2] +type=server +address=###node_server_IP_2### +port=###node_server_port_2### +protocol=MySQLBackend + +[server3] +type=server +address=###node_server_IP_3### +port=###node_server_port_3### +protocol=MySQLBackend + +[server4] +type=server +address=###node_server_IP_4### +port=###node_server_port_4### +protocol=MySQLBackend diff --git a/maxscale-system-test/cnf/maxscale.cnf.template.mxs1583_fwf b/maxscale-system-test/cnf/maxscale.cnf.template.mxs1583_fwf new file mode 100644 index 000000000..8c9e1213b --- /dev/null +++ b/maxscale-system-test/cnf/maxscale.cnf.template.mxs1583_fwf @@ -0,0 +1,77 @@ +[maxscale] +threads=###threads### +query_classifier_args=log_unrecognized_statements=3 + +[MySQL Monitor] +type=monitor +module=mysqlmon +###repl51### +servers=server1 +user=maxskysql +passwd=skysql +monitor_interval=100 + +[Database Firewall] +type=filter +module=dbfwfilter +rules=/###access_homedir###/rules/rules.txt +log_match=true +log_no_match=true + +[RW Split Router] +type=service +router=readconnroute +servers=server1 +user=maxskysql +passwd=skysql +filters=Database Firewall + +[Read Connection Router Slave] +type=service +router=readconnroute +router_options=slave +servers=server1 +user=maxskysql +passwd=skysql + +[Read Connection Router Master] +type=service +router=readconnroute +router_options=master +servers=server1 +user=maxskysql +passwd=skysql + +[RW Split Listener] +type=listener +service=RW Split Router +protocol=MySQLClient +port=4006 + +[Read Connection Listener Slave] +type=listener +service=Read Connection Router Slave +protocol=MySQLClient +port=4009 + +[Read Connection Listener Master] +type=listener +service=Read Connection Router Master +protocol=MySQLClient +port=4008 + +[CLI] +type=service +router=cli + +[CLI Listener] +type=listener +service=CLI +protocol=maxscaled +socket=default + +[server1] +type=server +address=###node_server_IP_1### +port=###node_server_port_1### +protocol=MySQLBackend diff --git a/maxscale-system-test/different_size_rwsplit.cpp b/maxscale-system-test/different_size_rwsplit.cpp index 33400406b..a3a1b804a 100644 --- a/maxscale-system-test/different_size_rwsplit.cpp +++ b/maxscale-system-test/different_size_rwsplit.cpp @@ -19,6 +19,7 @@ int main(int argc, char *argv[]) different_packet_size(Test, false); + Test->repl->sync_slaves(); Test->check_maxscale_alive(0); int rval = Test->global_result; delete Test; diff --git a/maxscale-system-test/fw/rules_mxs1583 b/maxscale-system-test/fw/rules_mxs1583 new file mode 100644 index 000000000..ce87945c9 --- /dev/null +++ b/maxscale-system-test/fw/rules_mxs1583 @@ -0,0 +1,7 @@ +rule deny_functions_on_a match uses_function a + +users %@% match all rules deny_functions_on_a + +rule deny_functions_on_b match uses_function b + +users %@% match all rules deny_functions_on_b diff --git a/maxscale-system-test/kerberos_setup.cpp b/maxscale-system-test/kerberos_setup.cpp index 8ceb024fb..5ba310888 100644 --- a/maxscale-system-test/kerberos_setup.cpp +++ b/maxscale-system-test/kerberos_setup.cpp @@ -60,6 +60,8 @@ int main(int argc, char *argv[]) Test->maxscales->ssh_node(0, (char *) "yum clean all", true); Test->maxscales->ssh_node(0, (char *) "yum install rng-tools -y", true); Test->maxscales->ssh_node(0, (char *) "rngd -r /dev/urandom -o /dev/random", true); + Test->maxscales->ssh_node(0, (char *) + "yum install -y MariaDB-gssapi-server MariaDB-gssapi-client krb5-server krb5-workstation pam_krb5", true); Test->maxscales->ssh_node_f(0, true, (char *) "yum install -y MariaDB-gssapi-server MariaDB-gssapi-client krb5-server krb5-workstation pam_krb5", true); diff --git a/maxscale-system-test/local_address.cpp b/maxscale-system-test/local_address.cpp new file mode 100644 index 000000000..0013a0f0c --- /dev/null +++ b/maxscale-system-test/local_address.cpp @@ -0,0 +1,333 @@ +/* + * Copyright (c) 2018 MariaDB Corporation Ab + * + * Use of this software is governed by the Business Source License included + * in the LICENSE.TXT file and at www.mariadb.com/bsl11. + * + * Change Date: 2019-07-01 + * + * On the date above, in accordance with the Business Source License, use + * of this software will be governed by version 2 or later of the General + * Public License. + */ + +#include "testconnections.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace std; + +namespace +{ + +template +void to_collection(string s, const string& delimiter, T* pT) +{ + size_t pos; + + while ((pos = s.find(delimiter)) != std::string::npos) + { + pT->push_back(s.substr(0, pos)); + s.erase(0, pos + delimiter.length()); + } + + if (s.length() != 0) + { + pT->push_back(s); + } +} + +string& ltrim(std::string& s) +{ + s.erase(s.begin(), std::find_if(s.begin(), s.end(), + std::not1(std::ptr_fun(std::isspace)))); + return s; +} + +string& rtrim(std::string& s) +{ + s.erase(std::find_if(s.rbegin(), s.rend(), + std::not1(std::ptr_fun(std::isspace))).base(), s.end()); + return s; +} + +string& trim(std::string& s) +{ + return ltrim(rtrim(s)); +} + +string extract_ip(string s) +{ + // 's' looks something like: " inet 127.0.0.1/..."; + s = s.substr(9); // => "127.0.0.1/..."; + s = s.substr(0, s.find_first_of('/')); // => "127.0.0.1" + return s; +} + +void get_maxscale_ips(TestConnections& test, vector* pIps) +{ + int exit_code; + string output(test.maxscales->ssh_node_output(0, "ip addr|fgrep inet|fgrep -v ::", false, &exit_code)); + + to_collection(output, "\n", pIps); + transform(pIps->begin(), pIps->end(), pIps->begin(), extract_ip); + + pIps->erase(find(pIps->begin(), pIps->end(), "127.0.0.1")); +} + +} + +namespace +{ + +void drop_user(TestConnections& test, const string& user, const string& host) +{ + string stmt("DROP USER IF EXISTS "); + + stmt += "'"; + stmt += user; + stmt += "'@'"; + stmt += host; + stmt += "'"; + test.try_query(test.maxscales->conn_rwsplit[0], stmt.c_str()); +} + +void create_user(TestConnections& test, const string& user, const string& password, const string& host) +{ + string stmt("CREATE USER "); + + stmt += "'"; + stmt += user; + stmt += "'@'"; + stmt += host; + stmt += "'"; + stmt += " IDENTIFIED BY "; + stmt += "'"; + stmt += password; + stmt += "'"; + test.try_query(test.maxscales->conn_rwsplit[0], stmt.c_str()); +} + +void grant_access(TestConnections& test, const string& user, const string& host) +{ + string stmt("GRANT SELECT, INSERT, UPDATE ON *.* TO "); + + stmt += "'"; + stmt += user; + stmt += "'@'"; + stmt += host; + stmt += "'"; + test.try_query(test.maxscales->conn_rwsplit[0], stmt.c_str()); + + test.try_query(test.maxscales->conn_rwsplit[0], "FLUSH PRIVILEGES"); +} + +void create_user_and_grants(TestConnections& test, + const string& user, const string& password, const string& host) +{ + test.tprintf("Creating user: %s@%s", user.c_str(), host.c_str()); + + drop_user(test, user, host); + create_user(test, user, password, host); + grant_access(test, user, host); +} + +bool select_user(MYSQL* pMysql, string* pUser) +{ + bool rv = false; + + if (mysql_query(pMysql, "SELECT USER()") == 0) + { + MYSQL_RES* pRes = mysql_store_result(pMysql); + + if (mysql_num_rows(pRes) == 1) + { + MYSQL_ROW row = mysql_fetch_row(pRes); + *pUser = row[0]; + rv = true; + } + + mysql_free_result(pRes); + + while (mysql_next_result(pMysql) == 0) + { + MYSQL_RES* pRes = mysql_store_result(pMysql); + mysql_free_result(pRes); + } + } + + return rv; +} + +bool can_connect_to_maxscale(const char* zHost, int port, const char* zUser, const char* zPassword) +{ + bool could_connect = false; + + MYSQL* pMysql = mysql_init(NULL); + + if (pMysql) + { + unsigned int timeout = 5; + mysql_options(pMysql, MYSQL_OPT_CONNECT_TIMEOUT, &timeout); + mysql_options(pMysql, MYSQL_OPT_READ_TIMEOUT, &timeout); + mysql_options(pMysql, MYSQL_OPT_WRITE_TIMEOUT, &timeout); + + if (mysql_real_connect(pMysql, zHost, zUser, zPassword, NULL, port, NULL, 0)) + { + string user; + if (select_user(pMysql, &user)) + { + could_connect = true; + } + else + { + cout << "Could not 'SELECT USER()' as '" << zUser << "': " << mysql_error(pMysql) << endl; + } + } + else + { + cout << "Could not connect as '" << zUser << "': " << mysql_error(pMysql) << endl; + } + + mysql_close(pMysql); + } + + return could_connect; +} + +string get_local_ip(TestConnections& test) +{ + int exit_code; + string output(test.maxscales->ssh_node_output(0, "nslookup maxscale|fgrep Server:|sed s/Server://", false, &exit_code)); + return trim(output); +} + +void start_maxscale_with_local_address(TestConnections& test, + const string& replace, + const string& with) +{ + string command("sed -i s/"); + command += replace; + command += "/"; + command += with; + command += "/ "; + command += "/etc/maxscale.cnf"; + + test.maxscales->ssh_node(0, command.c_str(), true); + + test.start_maxscale(); +} + +void test_connecting(TestConnections& test, + const char* zUser, const char* zPassword, const char* zHost, + bool should_be_able_to) +{ + bool could_connect = can_connect_to_maxscale(test.maxscales->IP[0], test.maxscales->rwsplit_port[0], zUser, zPassword); + + if (!could_connect && should_be_able_to) + { + test.assert(false, "%s@%s should have been able to connect, but wasn't.", zUser, zHost); + } + else if (could_connect && !should_be_able_to) + { + test.assert(false, "%s@%s should NOT have been able to connect, but was.", zUser, zHost); + } + else + { + if (could_connect) + { + test.tprintf("%s@%s could connect, as expected.", zUser, zHost); + } + else + { + test.tprintf("%s@%s could NOT connect, as expected.", zUser, zHost); + } + } +} + +void run_test(TestConnections& test, const string& ip1, const string& ip2) +{ + test.maxscales->connect(); + + string local_ip = get_local_ip(test); + + const char* zUser1 = "alice"; + const char* zUser2 = "bob"; + const char* zPassword1 = "alicepwd"; + const char* zPassword2 = "bobpwd"; + + create_user_and_grants(test, zUser1, zPassword1, ip1); + create_user_and_grants(test, zUser1, zPassword1, local_ip); + create_user_and_grants(test, zUser2, zPassword2, ip2); + create_user_and_grants(test, zUser2, zPassword2, local_ip); + + test.tprintf("\n"); + test.tprintf("Testing default; alice should be able to access, bob not."); + + test_connecting(test, zUser1, zPassword1, ip1.c_str(), true); + test_connecting(test, zUser2, zPassword2, ip2.c_str(), false); + + test.maxscales->disconnect(); + test.stop_maxscale(); + + test.tprintf("\n"); + test.tprintf("Testing with local_address=%s; alice should be able to access, bob not.", + ip1.c_str()); + + string local_address_ip1 = "local_address=" + ip1; + start_maxscale_with_local_address(test, "###local_address###", local_address_ip1); + test.maxscales->connect(); + + test_connecting(test, zUser1, zPassword1, ip1.c_str(), true); + test_connecting(test, zUser2, zPassword2, ip2.c_str(), false); + + test.maxscales->disconnect(); + test.stop_maxscale(); + + test.tprintf("\n"); + test.tprintf("WARNING: Other IP-address not tested, as usable IP-address not available."); + +#ifdef USABLE_SECOND_IP_ADDRESS_ON_MAXSCALE_NODE_IS_AVAILABLE + test.tprintf("\n"); + test.tprintf("\nTesting with local_address=%s, bob should be able to access, alice not.", + ip2.c_str()); + + string local_address_ip2 = "local_address=" + ip2; + start_maxscale_with_local_address(test, local_address_ip1, local_address_ip2); + test.connect_maxscale(); + + test_connecting(test, zUser1, zPassword1, ip1.c_str(), false); + test_connecting(test, zUser2, zPassword2, ip2.c_str(), true); + + test.maxscales->disconnect(); + test.stop_maxscale(); +#endif +} + +} + +int main(int argc, char** argv) +{ + TestConnections test(argc, argv); + + vector ips; + get_maxscale_ips(test, &ips); + + if (ips.size() >= 2) + { + run_test(test, ips[0], ips[1]); + } + else + { + test.assert(false, "MaxScale node does not have at least two IP-addresses."); + } + + return test.global_result; +} diff --git a/maxscale-system-test/mariadb_nodes.cpp b/maxscale-system-test/mariadb_nodes.cpp index 8d417376f..6071f7a63 100644 --- a/maxscale-system-test/mariadb_nodes.cpp +++ b/maxscale-system-test/mariadb_nodes.cpp @@ -1324,6 +1324,25 @@ void Mariadb_nodes::add_server_setting(int node, const char* setting) ssh_node_f(node, true, "sudo sed -i '$a %s' /etc/my.cnf.d/server.cnf", setting); } +void Mariadb_nodes::reset_server_settings(int node) +{ + std::stringstream ss; + ss << test_dir << "/mdbci/cnf/server" << node + 1 << ".cnf"; + + // Note: This is a CentOS specific path + ssh_node(node, "sudo rm -rf /etc/my.cnf.d/*", true); + copy_to_node(node, ss.str().c_str(), "~/"); + ssh_node_f(node, false, "sudo mv ~/server%d.cnf /etc/my.cnf.d/", node + 1); +} + +void Mariadb_nodes::reset_server_settings() +{ + for (int i = 0; i < N; i++) + { + reset_server_settings(i); + } +} + /** * @brief extract_version_from_string Tries to find MariaDB server version number in the output of 'mysqld --version' * Function does not allocate any memory diff --git a/maxscale-system-test/mariadb_nodes.h b/maxscale-system-test/mariadb_nodes.h index 1cf00c1ae..fb0517ae9 100644 --- a/maxscale-system-test/mariadb_nodes.h +++ b/maxscale-system-test/mariadb_nodes.h @@ -396,6 +396,18 @@ public: */ void add_server_setting(int node, const char* setting); + /** + * Restore the original configuration for this server + * + * @param node Node to restore + */ + void reset_server_settings(int node); + + /** + * Restore the original configuration for all servers + */ + void reset_server_settings(); + /** * @brief revert_nodes_snapshot Execute MDBCI snapshot revert command for all nodes * @return true in case of success diff --git a/maxscale-system-test/maxctrl_basic.cpp b/maxscale-system-test/maxctrl_basic.cpp new file mode 100644 index 000000000..6322d989a --- /dev/null +++ b/maxscale-system-test/maxctrl_basic.cpp @@ -0,0 +1,31 @@ +/** + * Minimal MaxCtrl sanity check + */ + +#include "testconnections.h" + +int main(int argc, char** argv) +{ + TestConnections test(argc, argv); + + int rc = test.maxscales->ssh_node_f(0, false, "maxctrl help list servers"); + test.assert(rc == 0, "`help list servers` should work"); + + rc = test.maxscales->ssh_node_f(0, false, "maxctrl --tsv list servers|grep 'Master, Running'"); + test.assert(rc == 0, "`list servers` should return at least one row with: Master, Running"); + + rc = test.maxscales->ssh_node_f(0, false, "maxctrl set server server1 maintenance"); + test.assert(rc == 0, "`set server` should work"); + + rc = test.maxscales->ssh_node_f(0, false, "maxctrl --tsv list servers|grep 'Maintenance'"); + test.assert(rc == 0, "`list servers` should return at least one row with: Maintanance"); + + rc = test.maxscales->ssh_node_f(0, false, "maxctrl clear server server1 maintenance"); + test.assert(rc == 0, "`clear server` should work"); + + rc = test.maxscales->ssh_node_f(0, false, "maxctrl --tsv list servers|grep 'Maintenance'"); + test.assert(rc != 0, "`list servers` should have no rows with: Maintanance"); + + test.check_maxscale_alive(); + return test.global_result; +} diff --git a/maxscale-system-test/mdbci/add_core_cnf.sh b/maxscale-system-test/mdbci/add_core_cnf.sh index aa5db1828..21a69f460 100755 --- a/maxscale-system-test/mdbci/add_core_cnf.sh +++ b/maxscale-system-test/mdbci/add_core_cnf.sh @@ -1,4 +1,8 @@ -set -x +if [ "$1" == "verbose" ] +then + set -x +fi + chmod 777 /tmp/ echo 2 > /proc/sys/fs/suid_dumpable sed -i "s/start() {/start() { \n export DAEMON_COREFILE_LIMIT='unlimited'; ulimit -c unlimited; /" /etc/init.d/maxscale diff --git a/maxscale-system-test/mdbci/configure_core.sh b/maxscale-system-test/mdbci/configure_core.sh deleted file mode 100755 index 2c72b5ac9..000000000 --- a/maxscale-system-test/mdbci/configure_core.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/bin/bash -set -x - -ssh -i $maxscale_sshkey -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no $maxscale_access_user@$maxscale_IP '$maxscale_access_sudo service iptables stop' - -ssh -i $maxscale_sshkey -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no $maxscale_access_user@$maxscale_IP "$maxscale_access_sudo mkdir ccore; $maxscale_access_sudo chown $maxscale_access_user ccore" - -scp -i $maxscale_sshkey -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no ${script_dir}/add_core_cnf.sh $maxscale_access_user@$maxscale_IP:./ccore/ -ssh -i $maxscale_sshkey -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no $maxscale_access_user@$maxscale_IP "$maxscale_access_sudo /home/$maxscale_access_user/ccore/add_core_cnf.sh" - -set +x diff --git a/maxscale-system-test/mdbci/create_config.sh b/maxscale-system-test/mdbci/create_config.sh index e35874938..14cbb3f42 100755 --- a/maxscale-system-test/mdbci/create_config.sh +++ b/maxscale-system-test/mdbci/create_config.sh @@ -16,10 +16,12 @@ export repo_dir=$dir/repo.d/ export provider=`${mdbci_dir}/mdbci show provider $box --silent 2> /dev/null` export backend_box=${backend_box:-"centos_7_"$provider} +if [ "$product" == "mysql" ] ; then + export cnf_path=${script_dir}/cnf/mysql56 +fi + +${mdbci_dir}/mdbci destroy $name mkdir -p ${MDBCI_VM_PATH}/$name -cd ${MDBCI_VM_PATH}/$name -vagrant destroy -f -cd $dir export cnf_path="${MDBCI_VM_PATH}/$name/cnf/" if [ "$product" == "mysql" ] ; then diff --git a/maxscale-system-test/mdbci/destroy.sh b/maxscale-system-test/mdbci/destroy.sh deleted file mode 100755 index 8d9597732..000000000 --- a/maxscale-system-test/mdbci/destroy.sh +++ /dev/null @@ -1,10 +0,0 @@ -#!/bin/bash - -dir=`pwd` -cd ${MDBCI_VM_PATH}/${name} -vagrant destroy -f -cd $dir - -rm -rf ${MDBCI_VM_PATH}/${name} -rm -rf ${MDBCI_VM_PATH}/${name}.json -rm -rf ${MDBCI_VM_PATH}/${name}_network_config diff --git a/maxscale-system-test/mdbci/run_test.sh b/maxscale-system-test/mdbci/run_test.sh index c74ab8dc9..609d35e7c 100755 --- a/maxscale-system-test/mdbci/run_test.sh +++ b/maxscale-system-test/mdbci/run_test.sh @@ -63,7 +63,6 @@ res=$? ulimit -c unlimited if [ $res == 0 ] ; then -# . ${script_dir}/configure_backend.sh . ${script_dir}/set_env.sh $name cd ${script_dir}/.. cmake . -DBUILDNAME=$name -DCMAKE_BUILD_TYPE=Debug @@ -76,7 +75,7 @@ if [ $res == 0 ] ; then if [ $? != 0 ]; then echo "Backend broken!" if [ "${do_not_destroy_vm}" != "yes" ] ; then - ${script_dir}/destroy.sh + ${mdbci_dir}/mdbci destroy $name fi rm ~/vagrant_lock exit 1 @@ -90,13 +89,13 @@ if [ $res == 0 ] ; then else echo "Failed to create VMs, exiting" if [ "${do_not_destroy_vm}" != "yes" ] ; then - ${script_dir}/destroy.sh + ${mdbci_dir}/mdbci destroy $name fi rm ~/vagrant_lock exit 1 fi if [ "${do_not_destroy_vm}" != "yes" ] ; then - ${script_dir}/destroy.sh + ${mdbci_dir}/mdbci destroy $name echo "clean up done!" fi diff --git a/maxscale-system-test/mdbci/run_test_snapshot.sh b/maxscale-system-test/mdbci/run_test_snapshot.sh index aa7655320..fa3398f3f 100755 --- a/maxscale-system-test/mdbci/run_test_snapshot.sh +++ b/maxscale-system-test/mdbci/run_test_snapshot.sh @@ -48,15 +48,14 @@ export repo_dir=$dir/repo.d/ ${mdbci_dir}/mdbci snapshot revert --path-to-nodes $name --snapshot-name $snapshot_name if [ $? != 0 ]; then - ${script_dir}/destroy.sh + ${mdbci_dir}/mdbci destroy $name ${MDBCI_VM_PATH}/scripts/clean_vms.sh $name ${script_dir}/create_config.sh - checkExitStatus $? "Error creating configuration" $snapshot_lock_file - . ${script_dir}/configure_backend.sh + checkExitStatus $? "Error creating configuration" $snapshot_lock_file echo "Creating snapshot from new config" - $HOME/mdbci/mdbci snapshot take --path-to-nodes $name --snapshot-name $snapshot_name + ${mdbci_dir}/mdbci snapshot take --path-to-nodes $name --snapshot-name $snapshot_name fi . ${script_dir}/set_env.sh "$name" diff --git a/maxscale-system-test/mxs1516.cpp b/maxscale-system-test/mxs1516.cpp index c1081b4e2..2be6db2ac 100644 --- a/maxscale-system-test/mxs1516.cpp +++ b/maxscale-system-test/mxs1516.cpp @@ -17,7 +17,11 @@ int main(int argc, char** argv) test.repl->connect(); test.repl->change_master(1, 0); - test.add_result(execute_query_silent(test.maxscales->conn_master[0], "SELECT 1") == 0, "Query should fail"); + // Give the monitor some time to detect it + sleep(5); + + test.add_result(execute_query_silent(test.maxscales->conn_master[0], "SELECT 1") == 0, + "Query should fail"); // Change the master back to the original one test.repl->change_master(0, 1); diff --git a/maxscale-system-test/mxs1583_fwf.cpp b/maxscale-system-test/mxs1583_fwf.cpp new file mode 100644 index 000000000..fa6c2e186 --- /dev/null +++ b/maxscale-system-test/mxs1583_fwf.cpp @@ -0,0 +1,65 @@ +/** + * Firewall filter multiple matching users + * + * Test it multiple matching user rows are handled in OR fashion. + */ + + +#include +#include +#include "testconnections.h" +#include "fw_copy_rules.h" + +int main(int argc, char** argv) +{ + TestConnections::skip_maxscale_start(true); + char rules_dir[4096]; + + TestConnections test(argc, argv); + test.stop_timeout(); + + test.tprintf("Creating rules\n"); + test.maxscales->stop_maxscale(0); + + sprintf(rules_dir, "%s/fw/", test_dir); + copy_rules(&test, (char*) "rules_mxs1583", rules_dir); + + test.set_timeout(60); + test.maxscales->start_maxscale(0); + + test.set_timeout(30); + test.maxscales->connect_maxscale(0); + + test.try_query(test.maxscales->conn_rwsplit[0], "drop table if exists t"); + test.try_query(test.maxscales->conn_rwsplit[0], "create table t (a text, b text)"); + + test.tprintf("Trying query that matches one 'user' row, expecting failure\n"); + test.set_timeout(30); + test.add_result(!execute_query(test.maxscales->conn_rwsplit[0], "select concat(a) from t"), + "Query that matches one 'user' row should fail.\n"); + + test.tprintf("Trying query that matches other 'user' row, expecting failure\n"); + test.set_timeout(30); + test.add_result(!execute_query(test.maxscales->conn_rwsplit[0], "select concat(b) from t"), + "Query that matches other 'user' row should fail.\n"); + + test.tprintf("Trying query that matches both 'user' rows, expecting failure\n"); + test.set_timeout(30); + test.add_result(!execute_query_silent(test.maxscales->conn_rwsplit[0], "select concat(a), concat(b) from t"), + "Query that matches both 'user' rows should fail.\n"); + + test.tprintf("Trying non-matching query to blacklisted RWSplit, expecting success\n"); + test.set_timeout(30); + test.add_result(execute_query_silent(test.maxscales->conn_rwsplit[0], "show status"), + "Non-matching query to blacklist service should succeed.\n"); + + test.stop_timeout(); + test.tprintf("Checking if MaxScale is alive\n"); + test.check_maxscale_processes(0, 1); + test.maxscales->stop_maxscale(0); + sleep(10); + test.tprintf("Checking if MaxScale was succesfully terminated\n"); + test.check_maxscale_processes(0, 0); + + return test.global_result; +} diff --git a/maxscale-system-test/mxs1585.cpp b/maxscale-system-test/mxs1585.cpp index d6656846d..ee259830a 100644 --- a/maxscale-system-test/mxs1585.cpp +++ b/maxscale-system-test/mxs1585.cpp @@ -63,6 +63,7 @@ int main(int argc, char** argv) } running = false; + test.set_timeout(120); for (auto& a: threads) { diff --git a/maxscale-system-test/mxs431.cpp b/maxscale-system-test/mxs431.cpp index 62e0dcb5d..f179f26ef 100644 --- a/maxscale-system-test/mxs431.cpp +++ b/maxscale-system-test/mxs431.cpp @@ -10,37 +10,26 @@ int main(int argc, char *argv[]) { TestConnections test(argc, argv); - char str[256]; - int iterations = 100; - test.repl->execute_query_all_nodes((char *) "set global max_connections = 600;"); - test.set_timeout(200); - test.repl->stop_slaves(); - test.set_timeout(200); - test.maxscales->restart_maxscale(0); - test.set_timeout(200); + test.repl->connect(); - test.stop_timeout(); /** Create a database on each node */ for (int i = 0; i < test.repl->N; i++) { test.set_timeout(20); - sprintf(str, "DROP DATABASE IF EXISTS shard_db%d", i); - test.tprintf("%s\n", str); - execute_query(test.repl->nodes[i], str); - test.set_timeout(20); - sprintf(str, "CREATE DATABASE shard_db%d", i); - test.tprintf("%s\n", str); - execute_query(test.repl->nodes[i], str); + execute_query(test.repl->nodes[i], "set global max_connections = 600"); + execute_query(test.repl->nodes[i], "DROP DATABASE IF EXISTS shard_db%d", i); + execute_query(test.repl->nodes[i], "CREATE DATABASE shard_db%d", i); test.stop_timeout(); } - test.repl->close_connections(); + int iterations = 100; for (int j = 0; j < iterations && test.global_result == 0; j++) { for (int i = 0; i < test.repl->N && test.global_result == 0; i++) { + char str[256]; sprintf(str, "shard_db%d", i); test.set_timeout(30); MYSQL *conn = open_conn_db(test.maxscales->rwsplit_port[0], test.maxscales->IP[0], @@ -52,5 +41,13 @@ int main(int argc, char *argv[]) } } + /** Create a database on each node */ + for (int i = 0; i < test.repl->N; i++) + { + test.set_timeout(20); + execute_query(test.repl->nodes[i], "DROP DATABASE shard_db%d", i); + test.stop_timeout(); + } + return test.global_result; } diff --git a/maxscale-system-test/utilities.cmake b/maxscale-system-test/utilities.cmake index a683b7fd0..a8f274166 100644 --- a/maxscale-system-test/utilities.cmake +++ b/maxscale-system-test/utilities.cmake @@ -103,10 +103,6 @@ ExternalProject_Add(connector-c include_directories(${CMAKE_BINARY_DIR}/include) set(MYSQL_CLIENT ${CMAKE_BINARY_DIR}/lib/mariadb/libmariadbclient.a CACHE INTERNAL "") -# Build the CDC connector -add_library(cdc_connector ${CMAKE_SOURCE_DIR}/../connectors/cdc-connector/cdc_connector.cpp) -include_directories(${CMAKE_SOURCE_DIR}/../connectors/cdc-connector/) - # # Check that all required components are present. To build even without them, # add e.g. -DHAVE_PHP=Y to the CMake invocation diff --git a/query_classifier/qc_sqlite/builtin_functions.c b/query_classifier/qc_sqlite/builtin_functions.c index af42e1f65..7839441ed 100644 --- a/query_classifier/qc_sqlite/builtin_functions.c +++ b/query_classifier/qc_sqlite/builtin_functions.c @@ -250,27 +250,33 @@ static const char* BUILTIN_FUNCTIONS[] = * https://mariadb.com/kb/en/mariadb/miscellaneous-functions/ */ "default", - "get_lock", "inet6_aton", "inet6_ntoa", "inet_aton", "inet_ntoa", - "is_free_lock", "is_ipv4", "is_ipv4_compat", "is_ipv4_mapped", "is_ipv6", - "is_used_lock", "last_value", "master_gtid_wait", "master_pos_wait", "name_const", - "release_lock", "sleep", "uuid", "uuid_short", "values", + /** + * Although conceptually non-updating, we classify these as WRITE as + * that will force them to be sent to _master_ outside transactions. + * + * "get_lock", + * "is_free_lock", + * "is_used_lock", + * "release_lock", + */ + /* * Numeric Functions * https://mariadb.com/kb/en/mariadb/numeric-functions/ diff --git a/query_classifier/test/expected.sql b/query_classifier/test/expected.sql index dbff01229..f57402db2 100644 --- a/query_classifier/test/expected.sql +++ b/query_classifier/test/expected.sql @@ -19,3 +19,7 @@ QUERY_TYPE_READ|QUERY_TYPE_SYSVAR_READ QUERY_TYPE_READ|QUERY_TYPE_WRITE QUERY_TYPE_READ|QUERY_TYPE_WRITE QUERY_TYPE_READ|QUERY_TYPE_WRITE +QUERY_TYPE_READ|QUERY_TYPE_WRITE +QUERY_TYPE_READ|QUERY_TYPE_WRITE +QUERY_TYPE_READ|QUERY_TYPE_WRITE +QUERY_TYPE_READ|QUERY_TYPE_WRITE diff --git a/query_classifier/test/input.sql b/query_classifier/test/input.sql index 48c5d6276..643e2c2d0 100644 --- a/query_classifier/test/input.sql +++ b/query_classifier/test/input.sql @@ -19,3 +19,7 @@ select if(@@hostname='box02','prod_mariadb02','n'); select next value for seq1; select nextval(seq1); select seq1.nextval; +SELECT GET_LOCK('lock1',10); +SELECT IS_FREE_LOCK('lock1'); +SELECT IS_USED_LOCK('lock1'); +SELECT RELEASE_LOCK('lock1'); diff --git a/server/core/CMakeLists.txt b/server/core/CMakeLists.txt index a76c021dc..a6c6e979d 100644 --- a/server/core/CMakeLists.txt +++ b/server/core/CMakeLists.txt @@ -73,6 +73,7 @@ target_link_libraries(maxscale-common z rt m + sqlite3 stdc++ gnutls gcrypt diff --git a/server/core/backend.cc b/server/core/backend.cc index 3dd915ca6..6ffa3777d 100644 --- a/server/core/backend.cc +++ b/server/core/backend.cc @@ -43,6 +43,8 @@ Backend::~Backend() void Backend::close(close_type type) { + ss_dassert(m_dcb->n_close == 0); + if (!m_closed) { m_closed = true; diff --git a/server/core/config.cc b/server/core/config.cc index 22d706b5a..ef68d81df 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -116,6 +116,7 @@ const char CN_QUERY_RETRIES[] = "query_retries"; const char CN_QUERY_RETRY_TIMEOUT[] = "query_retry_timeout"; const char CN_RELATIONSHIPS[] = "relationships"; const char CN_LINKS[] = "links"; +const char CN_LOCAL_ADDRESS[] = "local_address"; const char CN_REQUIRED[] = "required"; const char CN_RETRY_ON_FAILURE[] = "retry_on_failure"; const char CN_ROUTER[] = "router"; @@ -1624,6 +1625,10 @@ handle_global_item(const char *name, const char *value) { gateway.passive = config_truth_value((char*)value); } + else if (strcmp(name, CN_LOCAL_ADDRESS) == 0) + { + gateway.local_address = MXS_STRDUP_A(value); + } else { for (i = 0; lognames[i].name; i++) diff --git a/server/core/dcb.cc b/server/core/dcb.cc index 161df9dad..151e990eb 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -1123,6 +1123,7 @@ void dcb_close(DCB *dcb) ++dcb->n_close; // TODO: Will this happen on a regular basis? MXS_WARNING("dcb_close(%p) called %u times.", dcb, dcb->n_close); + ss_dassert(!true); } } @@ -3208,6 +3209,26 @@ static uint32_t dcb_poll_handler(MXS_POLL_DATA *data, int thread_id, uint32_t ev return dcb_handler(dcb, events); } +static bool dcb_is_still_valid(DCB* target, int id) +{ + bool rval = false; + + for (DCB *dcb = this_unit.all_dcbs[id]; + dcb; dcb = dcb->thread.next) + { + if (target == dcb) + { + if (dcb->n_close == 0) + { + rval = true; + } + break; + } + } + + return rval; +} + class FakeEventTask: public mxs::WorkerDisposableTask { FakeEventTask(const FakeEventTask&); @@ -3223,8 +3244,15 @@ public: void execute(Worker& worker) { - m_dcb->fakeq = m_buffer; - dcb_handler(m_dcb, m_ev); + if (dcb_is_still_valid(m_dcb, worker.get_current_id())) + { + m_dcb->fakeq = m_buffer; + dcb_handler(m_dcb, m_ev); + } + else + { + gwbuf_free(m_buffer); + } } private: diff --git a/server/core/gateway.cc b/server/core/gateway.cc index 1d900d42f..d2caf5695 100644 --- a/server/core/gateway.cc +++ b/server/core/gateway.cc @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -189,6 +190,7 @@ static void disable_module_unloading(const char* arg); static void enable_module_unloading(const char* arg); static void redirect_output_to_file(const char* arg); static bool user_is_acceptable(const char* specified_user); +static bool init_sqlite3(); struct DEBUG_ARGUMENT { @@ -1957,6 +1959,13 @@ int main(int argc, char **argv) } } + if (!init_sqlite3()) + { + MXS_ERROR("Could not initialize sqlite3, exiting."); + rc = MAXSCALE_INTERNALERROR; + goto return_main; + } + MXS_NOTICE("MariaDB MaxScale %s started", MAXSCALE_VERSION); MXS_NOTICE("MaxScale is running in process %i", getpid()); @@ -3304,3 +3313,33 @@ static bool user_is_acceptable(const char* specified_user) return acceptable; } + +static bool init_sqlite3() +{ + bool rv = true; + + // Collecting the memstatus introduces locking that, according to customer reports, + // has a significant impact on the performance. + if (sqlite3_config(SQLITE_CONFIG_MEMSTATUS, (int)0) == SQLITE_OK) // 0 turns off. + { + MXS_NOTICE("The collection of SQLite memory allocation statistics turned off."); + } + else + { + MXS_WARNING("Could not turn off the collection of SQLite memory allocation statistics."); + // Non-fatal, we simply will take a small performance hit. + } + + if (sqlite3_config(SQLITE_CONFIG_MULTITHREAD) == SQLITE_OK) + { + MXS_NOTICE("Threading mode of SQLite set to Multi-thread."); + } + else + { + MXS_ERROR("Could not set the threading mode of SQLite to Multi-thread. " + "MaxScale will terminate."); + rv = false; + } + + return rv; +} diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 7c9936e0f..c8f0b450b 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -2427,3 +2427,16 @@ static bool journal_is_stale(MXS_MONITOR *monitor, time_t max_age) return is_stale; } + +MXS_MONITORED_SERVER* mon_get_monitored_server(MXS_MONITOR* mon, SERVER* search_server) +{ + ss_dassert(mon && search_server); + for (MXS_MONITORED_SERVER* iter = mon->monitored_servers; iter != NULL; iter = iter->next) + { + if (iter->server == search_server) + { + return iter; + } + } + return NULL; +} \ No newline at end of file diff --git a/server/core/mysql_binlog.cc b/server/core/mysql_binlog.cc index 476460314..07392d812 100644 --- a/server/core/mysql_binlog.cc +++ b/server/core/mysql_binlog.cc @@ -262,47 +262,18 @@ static void unpack_datetime(uint8_t *ptr, int length, struct tm *dest) uint64_t val = 0; uint32_t second, minute, hour, day, month, year; - if (length == -1) - { - val = gw_mysql_get_byte8(ptr); - second = val - ((val / 100) * 100); - val /= 100; - minute = val - ((val / 100) * 100); - val /= 100; - hour = val - ((val / 100) * 100); - val /= 100; - day = val - ((val / 100) * 100); - val /= 100; - month = val - ((val / 100) * 100); - val /= 100; - year = val; - } - else - { - // TODO: Figure out why DATETIME(0) doesn't work like it others do - val = unpack_bytes(ptr, datetime_sizes[length]); - val *= log_10_values[6 - length]; - - if (val < 0) - { - val = -val; - } - - int subsecond = val % 1000000; - val /= 1000000; - - second = val % 60; - val /= 60; - minute = val % 60; - val /= 60; - hour = val % 24; - val /= 24; - day = val % 32; - val /= 32; - month = val % 13; - val /= 13; - year = val; - } + val = gw_mysql_get_byte8(ptr); + second = val - ((val / 100) * 100); + val /= 100; + minute = val - ((val / 100) * 100); + val /= 100; + hour = val - ((val / 100) * 100); + val /= 100; + day = val - ((val / 100) * 100); + val /= 100; + month = val - ((val / 100) * 100); + val /= 100; + year = val; memset(dest, 0, sizeof(struct tm)); dest->tm_year = year - 1900; @@ -502,7 +473,7 @@ static size_t temporal_field_size(uint8_t type, uint8_t decimals, int length) return 3 + ((decimals + 1) / 2); case TABLE_COL_TYPE_DATETIME: - return length < 0 || length > 6 ? 8 : datetime_sizes[length]; + return 8; case TABLE_COL_TYPE_TIMESTAMP: return 4; @@ -649,6 +620,7 @@ size_t unpack_numeric_field(uint8_t *src, uint8_t type, uint8_t *metadata, uint8 break; } + ss_dassert(size > 0); memcpy(dest, src, size); return size; } diff --git a/server/core/session.cc b/server/core/session.cc index 23af09b38..ad10d9b24 100644 --- a/server/core/session.cc +++ b/server/core/session.cc @@ -320,10 +320,7 @@ void session_close(MXS_SESSION *session) { if (session->router_session) { - if (session->state != SESSION_STATE_STOPPING) - { - session->state = SESSION_STATE_STOPPING; - } + session->state = SESSION_STATE_STOPPING; MXS_ROUTER_OBJECT* router = session->service->router; MXS_ROUTER* router_instance = session->service->router_instance; diff --git a/server/core/utils.cc b/server/core/utils.cc index af9ab1756..ccab2f6be 100644 --- a/server/core/utils.cc +++ b/server/core/utils.cc @@ -41,6 +41,7 @@ #include #include +#include #include #include #include @@ -1019,6 +1020,8 @@ int open_network_socket(enum mxs_socket_type type, struct sockaddr_storage *addr memcpy(addr, ai->ai_addr, ai->ai_addrlen); set_port(addr, port); + freeaddrinfo(ai); + if ((type == MXS_SOCKET_NETWORK && !configure_network_socket(so)) || (type == MXS_SOCKET_LISTENER && !configure_listener_socket(so))) { @@ -1032,9 +1035,39 @@ int open_network_socket(enum mxs_socket_type type, struct sockaddr_storage *addr close(so); so = -1; } - } + else if (type == MXS_SOCKET_NETWORK) + { + MXS_CONFIG* config = config_get_global_options(); - freeaddrinfo(ai); + if (config->local_address) + { + if ((rc = getaddrinfo(config->local_address, NULL, &hint, &ai)) == 0) + { + struct sockaddr_storage local_address = {}; + + memcpy(&local_address, ai->ai_addr, ai->ai_addrlen); + freeaddrinfo(ai); + + if (bind(so, (struct sockaddr*)&local_address, sizeof(local_address)) == 0) + { + MXS_INFO("Bound connecting socket to \"%s\".", config->local_address); + } + else + { + MXS_ERROR("Could not bind connecting socket to local address \"%s\", " + "connecting to server using default local address: %s", + config->local_address, mxs_strerror(errno)); + } + } + else + { + MXS_ERROR("Could not get address information for local address \"%s\", " + "connecting to server using default local address: %s", + config->local_address, mxs_strerror(errno)); + } + } + } + } } return so; diff --git a/server/modules/authenticator/MySQLAuth/CMakeLists.txt b/server/modules/authenticator/MySQLAuth/CMakeLists.txt index a4aa424f1..d6ac92947 100644 --- a/server/modules/authenticator/MySQLAuth/CMakeLists.txt +++ b/server/modules/authenticator/MySQLAuth/CMakeLists.txt @@ -2,7 +2,7 @@ if(SQLITE_VERSION VERSION_LESS 3.3) message(FATAL_ERROR "SQLite version 3.3 or higher is required") else() add_library(mysqlauth SHARED mysql_auth.c dbusers.c) - target_link_libraries(mysqlauth maxscale-common mysqlcommon sqlite3) + target_link_libraries(mysqlauth maxscale-common mysqlcommon) set_target_properties(mysqlauth PROPERTIES VERSION "1.0.0") install_module(mysqlauth core) endif() diff --git a/server/modules/authenticator/MySQLAuth/dbusers.c b/server/modules/authenticator/MySQLAuth/dbusers.c index 434e7422b..ecc55331f 100644 --- a/server/modules/authenticator/MySQLAuth/dbusers.c +++ b/server/modules/authenticator/MySQLAuth/dbusers.c @@ -44,11 +44,11 @@ #define NEW_LOAD_DBUSERS_QUERY "SELECT u.user, u.host, d.db, u.select_priv, u.%s \ FROM mysql.user AS u LEFT JOIN mysql.db AS d \ - ON (u.user = d.user AND u.host = d.host) %s \ + ON (u.user = d.user AND u.host = d.host) WHERE u.plugin = '' %s \ UNION \ SELECT u.user, u.host, t.db, u.select_priv, u.%s \ FROM mysql.user AS u LEFT JOIN mysql.tables_priv AS t \ - ON (u.user = t.user AND u.host = t.host) %s" + ON (u.user = t.user AND u.host = t.host) WHERE u.plugin = '' %s" static int get_users(SERV_LISTENER *listener, bool skip_local); static MYSQL *gw_mysql_init(void); @@ -59,7 +59,7 @@ static bool get_hostname(DCB *dcb, char *client_hostname, size_t size); static char* get_new_users_query(const char *server_version, bool include_root) { const char* password = strstr(server_version, "5.7.") ? MYSQL57_PASSWORD : MYSQL_PASSWORD; - const char *with_root = include_root ? "" : "WHERE u.user NOT IN ('root')"; + const char *with_root = include_root ? "" : " AND u.user NOT IN ('root')"; size_t n_bytes = snprintf(NULL, 0, NEW_LOAD_DBUSERS_QUERY, password, with_root, password, with_root); char *rval = MXS_MALLOC(n_bytes + 1); diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.cc b/server/modules/filter/dbfwfilter/dbfwfilter.cc index fd5e85fb6..1c0aaa95e 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.cc +++ b/server/modules/filter/dbfwfilter/dbfwfilter.cc @@ -1029,7 +1029,7 @@ static bool process_user_templates(UserMap& users, const TemplateList& templates if (newrules.size() > 0) { - user->append_rules(ut->type, newrules); + user->add_rules(ut->type, newrules); } } diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.hh b/server/modules/filter/dbfwfilter/dbfwfilter.hh index e7181159d..738ef128d 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.hh +++ b/server/modules/filter/dbfwfilter/dbfwfilter.hh @@ -18,6 +18,7 @@ #include #include #include +#include #include #include diff --git a/server/modules/filter/dbfwfilter/user.cc b/server/modules/filter/dbfwfilter/user.cc index 0792283ce..ee225333c 100644 --- a/server/modules/filter/dbfwfilter/user.cc +++ b/server/modules/filter/dbfwfilter/user.cc @@ -31,20 +31,20 @@ const char* User::name() const return m_name.c_str(); } -void User::append_rules(match_type mode, const RuleList& rules) +void User::add_rules(match_type mode, const RuleList& rules) { switch (mode) { case FWTOK_MATCH_ANY: - rules_or.insert(rules_or.end(), rules.begin(), rules.end()); + rules_or_vector.push_back(rules); break; case FWTOK_MATCH_ALL: - rules_and.insert(rules_and.end(), rules.begin(), rules.end()); + rules_and_vector.push_back(rules); break; case FWTOK_MATCH_STRICT_ALL: - rules_strict_and.insert(rules_strict_and.end(), rules.begin(), rules.end()); + rules_strict_and_vector.push_back(rules); break; default: @@ -73,28 +73,39 @@ bool User::match_any(Dbfw* my_instance, DbfwSession* my_session, bool rval = false; - if (rules_or.size() > 0 && should_match(queue)) + for (RuleListVector::iterator i = rules_or_vector.begin(); i != rules_or_vector.end(); ++i) { - char *fullquery = modutil_get_SQL(queue); + RuleList& rules_or = *i; - if (fullquery) + if (rules_or.size() > 0 && should_match(queue)) { - for (RuleList::iterator it = rules_or.begin(); it != rules_or.end(); it++) + char *fullquery = modutil_get_SQL(queue); + + if (fullquery) { - if (rule_is_active(*it)) + for (RuleList::iterator j = rules_or.begin(); j != rules_or.end(); j++) { - if (rule_matches(my_instance, my_session, queue, *it, fullquery)) + if (rule_is_active(*j)) { - *rulename = MXS_STRDUP_A((*it)->name().c_str()); - rval = true; - break; + if (rule_matches(my_instance, my_session, queue, *j, fullquery)) + { + *rulename = MXS_STRDUP_A((*j)->name().c_str()); + rval = true; + break; + } } } - } - MXS_FREE(fullquery); + MXS_FREE(fullquery); + } + } + + if (rval) + { + break; } } + return rval; } @@ -116,44 +127,54 @@ bool User::do_match(Dbfw* my_instance, DbfwSession* my_session, bool rval = false; bool have_active_rule = false; std::string matching_rules; - RuleList& rules = mode == User::ALL ? rules_and : rules_strict_and; + RuleListVector& rules_vector = (mode == User::ALL ? rules_and_vector : rules_strict_and_vector); - if (rules.size() > 0 && should_match(queue)) + for (RuleListVector::iterator i = rules_vector.begin(); i != rules_vector.end(); ++i) { - char *fullquery = modutil_get_SQL(queue); + RuleList& rules = *i; - if (fullquery) + if (rules.size() > 0 && should_match(queue)) { - rval = true; - for (RuleList::iterator it = rules.begin(); it != rules.end(); it++) + char *fullquery = modutil_get_SQL(queue); + + if (fullquery) { - if (rule_is_active(*it)) + rval = true; + for (RuleList::iterator j = rules.begin(); j != rules.end(); j++) { - have_active_rule = true; - - if (rule_matches(my_instance, my_session, queue, *it, fullquery)) + if (rule_is_active(*j)) { - matching_rules += (*it)->name(); - matching_rules += " "; - } - else - { - rval = false; + have_active_rule = true; - if (mode == User::STRICT) + if (rule_matches(my_instance, my_session, queue, *j, fullquery)) { - break; + matching_rules += (*j)->name(); + matching_rules += " "; + } + else + { + rval = false; + + if (mode == User::STRICT) + { + break; + } } } } - } - if (!have_active_rule) - { - /** No active rules */ - rval = false; + if (!have_active_rule) + { + /** No active rules */ + rval = false; + } + MXS_FREE(fullquery); } - MXS_FREE(fullquery); + } + + if (rval) + { + break; } } diff --git a/server/modules/filter/dbfwfilter/user.hh b/server/modules/filter/dbfwfilter/user.hh index 5c537389b..f80eb5254 100644 --- a/server/modules/filter/dbfwfilter/user.hh +++ b/server/modules/filter/dbfwfilter/user.hh @@ -57,12 +57,12 @@ public: const char* name() const; /** - * Append new rules to existing rules + * Add new rules to existing rules * * @param mode Matching mode for the rule * @param rules Rules to append */ - void append_rules(match_type mode, const RuleList& rules); + void add_rules(match_type mode, const RuleList& rules); /** * Check if a query matches some rule @@ -84,11 +84,13 @@ private: STRICT }; - RuleList rules_or; /*< If any of these rules match the action is triggered */ - RuleList rules_and; /*< All of these rules must match for the action to trigger */ - RuleList rules_strict_and; /*< rules that skip the rest of the rules if one of them - * fails. This is only for rules paired with 'match strict_all'. */ - std::string m_name; /*< Name of the user */ + typedef std::vector RuleListVector; + + RuleListVector rules_or_vector; /*< If any of these rules match the action is triggered */ + RuleListVector rules_and_vector; /*< All of these rules must match for the action to trigger */ + RuleListVector rules_strict_and_vector;/*< rules that skip the rest of the rules if one of them + * fails. This is only for rules paired with 'match strict_all'. */ + std::string m_name; /*< Name of the user */ /** * Functions for matching rules diff --git a/server/modules/filter/qlafilter/qlafilter.cc b/server/modules/filter/qlafilter/qlafilter.cc index dc46f42e2..bd54d80ad 100644 --- a/server/modules/filter/qlafilter/qlafilter.cc +++ b/server/modules/filter/qlafilter/qlafilter.cc @@ -833,6 +833,10 @@ diagnostic(MXS_FILTER *instance, MXS_FILTER_SESSION *fsession, DCB *dcb) dcb_printf(dcb, "\t\tExclude queries that match %s\n", my_instance->exclude.c_str()); } + dcb_printf(dcb, "\t\tColumn separator %s\n", + my_instance->separator.c_str()); + dcb_printf(dcb, "\t\tNewline replacement %s\n", + my_instance->query_newline.c_str()); } /** @@ -876,6 +880,8 @@ static json_t* diagnostic_json(const MXS_FILTER *instance, const MXS_FILTER_SESS { json_object_set_new(rval, PARAM_EXCLUDE, json_string(my_instance->exclude.c_str())); } + json_object_set_new(rval, PARAM_SEPARATOR, json_string(my_instance->separator.c_str())); + json_object_set_new(rval, PARAM_NEWLINE, json_string(my_instance->query_newline.c_str())); return rval; } diff --git a/server/modules/monitor/mariadbmon/mysql_mon.cc b/server/modules/monitor/mariadbmon/mysql_mon.cc index ef42aadea..e4dd9c5f2 100644 --- a/server/modules/monitor/mariadbmon/mysql_mon.cc +++ b/server/modules/monitor/mariadbmon/mysql_mon.cc @@ -18,10 +18,11 @@ #define MXS_MODULE_NAME "mariadbmon" #include "../mysqlmon.h" +#include +#include #include #include #include -#include #include #include #include @@ -121,6 +122,7 @@ static bool can_replicate_from(MYSQL_MONITOR* mon, static bool wait_cluster_stabilization(MYSQL_MONITOR* mon, MXS_MONITORED_SERVER* new_master, const ServerVector& slaves, int seconds_remaining); static string get_connection_errors(const ServerVector& servers); +static int64_t scan_server_id(const char* id_string); static bool report_version_err = true; static const char* hb_table_name = "maxscale_schema.replication_heartbeat"; @@ -148,152 +150,271 @@ static const char CN_REPLICATION_PASSWORD[] = "replication_password"; /** Default master failure verification timeout */ #define DEFAULT_MASTER_FAILURE_TIMEOUT "10" +/** Server id default value */ +static const int64_t SERVER_ID_UNKNOWN = -1; + +class Gtid +{ +public: + uint32_t domain; + int64_t server_id; // Is actually 32bit unsigned. 0 is only used by server versions <= 10.1 + uint64_t sequence; + Gtid() + : domain(0) + , server_id(SERVER_ID_UNKNOWN) + , sequence(0) + {} + + /** + * Parse a Gtid-triplet from a string. In case of a multi-triplet value, only the triplet with + * the given domain is returned. + * + * @param str Gtid string + * @param search_domain The Gtid domain whose triplet should be returned. Negative domain stands for + * autoselect, which is only allowed when the string contains one triplet. + */ + Gtid(const char* str, int64_t search_domain = -1) + : domain(0) + , server_id(SERVER_ID_UNKNOWN) + , sequence(0) + { + // Autoselect only allowed with one triplet + ss_dassert(search_domain >= 0 || strchr(str, ',') == NULL); + parse_triplet(str); + if (search_domain >= 0 && domain != search_domain) + { + // Search for the correct triplet. + bool found = false; + for (const char* next_triplet = strchr(str, ','); + next_triplet != NULL && !found; + next_triplet = strchr(next_triplet, ',')) + { + parse_triplet(++next_triplet); + if (domain == search_domain) + { + found = true; + } + } + ss_dassert(found); + } + } + bool operator == (const Gtid& rhs) const + { + return domain == rhs.domain && + server_id != SERVER_ID_UNKNOWN && server_id == rhs.server_id && + sequence == rhs.sequence; + } + string to_string() const + { + std::stringstream ss; + if (server_id != SERVER_ID_UNKNOWN) + { + ss << domain << "-" << server_id << "-" << sequence; + } + return ss.str(); + } +private: + void parse_triplet(const char* str) + { + ss_debug(int rv = ) sscanf(str, "%" PRIu32 "-%" PRId64 "-%" PRIu64, &domain, &server_id, &sequence); + ss_dassert(rv == 3); + } +}; + +// Contains data returned by one row of SHOW ALL SLAVES STATUS +class SlaveStatusInfo +{ +public: + int64_t master_server_id; /**< The master's server_id value. Valid ids are 32bit unsigned. -1 is + * unread/error. */ + string master_host; /**< Master server host name. */ + int master_port; /**< Master server port. */ + bool slave_io_running; /**< Whether the slave I/O thread is running and connected. */ + bool slave_sql_running; /**< Whether or not the SQL thread is running. */ + string master_log_file; /**< Name of the master binary log file that the I/O thread is currently + * reading from. */ + uint64_t read_master_log_pos; /**< Position up to which the I/O thread has read in the current master + * binary log file. */ + Gtid gtid_io_pos; /**< Gtid I/O position of the slave thread. Only shows the triplet with + * the current master domain. */ + string last_error; /**< Last IO or SQL error encountered. */ + + SlaveStatusInfo() + : master_server_id(SERVER_ID_UNKNOWN) + , master_port(0) + , slave_io_running(false) + , slave_sql_running(false) + , read_master_log_pos(0) + {} +}; + +// This class groups some miscellaneous replication related settings together. +class ReplicationSettings +{ +public: + bool gtid_strict_mode; /**< Enable additional checks for replication */ + bool log_bin; /**< Is binary logging enabled */ + bool log_slave_updates; /**< Does the slave log replicated events to binlog */ + ReplicationSettings() + : gtid_strict_mode(false) + , log_bin(false) + , log_slave_updates(false) + {} +}; + /** - * Check whether specified current master is acceptable. + * Monitor specific information about a server * - * @param current_master The specified current master. - * @param monitored_server The server to check against. - * @param monitored_current_master On output, @c monitored_server, if @c monitored_server - * is the same server as @c current_master. - * @param error On output, error object if function failed. + * Note: These are initialized in @c init_server_info + */ +class MySqlServerInfo +{ +public: + int64_t server_id; /**< Value of @@server_id. Valid values are 32bit unsigned. */ + int group; /**< Multi-master group where this server belongs, + * 0 for servers not in groups */ + bool read_only; /**< Value of @@read_only */ + bool slave_configured; /**< Whether SHOW SLAVE STATUS returned rows */ + bool binlog_relay; /** Server is a Binlog Relay */ + int n_slaves_configured; /**< Number of configured slave connections*/ + int n_slaves_running; /**< Number of running slave connections */ + int slave_heartbeats; /**< Number of received heartbeats */ + double heartbeat_period; /**< The time interval between heartbeats */ + time_t latest_event; /**< Time when latest event was received from the master */ + int64_t gtid_domain_id; /**< The value of gtid_domain_id, the domain which is used for + * new non-replicated events. */ + Gtid gtid_current_pos; /**< Gtid of latest event. Only shows the triplet + * with the current master domain. */ + Gtid gtid_binlog_pos; /**< Gtid of latest event written to binlog. Only shows + * the triplet with the current master domain. */ + SlaveStatusInfo slave_status; /**< Data returned from SHOW SLAVE STATUS */ + ReplicationSettings rpl_settings; /**< Miscellaneous replication related settings */ + mysql_server_version version; /**< Server version, 10.X, 5.5 or 5.1 */ + + MySqlServerInfo() + : server_id(SERVER_ID_UNKNOWN) + , group(0) + , read_only(false) + , slave_configured(false) + , binlog_relay(false) + , n_slaves_configured(0) + , n_slaves_running(0) + , slave_heartbeats(0) + , heartbeat_period(0) + , latest_event(0) + , gtid_domain_id(-1) + , version(MYSQL_SERVER_VERSION_51) + {} + + /** + * Calculate how many events are left in the relay log. If gtid_current_pos is ahead of Gtid_IO_Pos, + * or a server_id is unknown, an error value is returned. + * + * @return Number of events in relay log according to latest queried info. A negative value signifies + * an error in the gtid-values. + */ + int64_t relay_log_events() + { + if (slave_status.gtid_io_pos.server_id != SERVER_ID_UNKNOWN && + gtid_current_pos.server_id != SERVER_ID_UNKNOWN && + slave_status.gtid_io_pos.domain == gtid_current_pos.domain && + slave_status.gtid_io_pos.sequence >= gtid_current_pos.sequence) + { + return slave_status.gtid_io_pos.sequence - gtid_current_pos.sequence; + } + return -1; + } +}; + +bool uses_gtid(MYSQL_MONITOR* mon, MXS_MONITORED_SERVER* mon_server, json_t** error_out) +{ + bool rval = false; + const MySqlServerInfo* info = get_server_info(mon, mon_server); + if (info->slave_status.gtid_io_pos.server_id == SERVER_ID_UNKNOWN) + { + string slave_not_gtid_msg = string("Slave server ") + mon_server->server->unique_name + + " is not using gtid replication."; + PRINT_MXS_JSON_ERROR(error_out, "%s", slave_not_gtid_msg.c_str()); + } + else + { + rval = true; + } + return rval; +} +/** + * Check that the given server is a master and it's the only master. + * + * @param mon Cluster monitor. + * @param suggested_curr_master The server to check, given by user. + * @param error_out On output, error object if function failed. * * @return False, if there is some error with the specified current master, * True otherwise. */ -bool mysql_switchover_check_current(SERVER* current_master, - MXS_MONITORED_SERVER* monitored_server, - MXS_MONITORED_SERVER** monitored_current_master, - json_t** error) +bool mysql_switchover_check_current(const MYSQL_MONITOR* mon, + const MXS_MONITORED_SERVER* suggested_curr_master, + json_t** error_out) { - bool rv = true; - bool is_master = SERVER_IS_MASTER(monitored_server->server); - - if (current_master == monitored_server->server) + bool server_is_master = false; + MXS_MONITORED_SERVER* extra_master = NULL; // A master server which is not the suggested one + for (MXS_MONITORED_SERVER* mon_serv = mon->monitor->monitored_servers; + mon_serv != NULL && extra_master == NULL; + mon_serv = mon_serv->next) { - if (is_master) + if (SERVER_IS_MASTER(mon_serv->server)) { - *monitored_current_master = monitored_server; - } - else - { - *error = mxs_json_error("Specified %s is a server, but not the current master.", - current_master->unique_name); - rv = false; + if (mon_serv == suggested_curr_master) + { + server_is_master = true; + } + else + { + extra_master = mon_serv; + } } } - else if (is_master) - { - *error = mxs_json_error("Current master not specified, even though there is " - "a master (%s).", monitored_server->server->unique_name); - rv = false; - } - return rv; + if (!server_is_master) + { + PRINT_MXS_JSON_ERROR(error_out, "Server '%s' is not the current master or it's in maintenance.", + suggested_curr_master->server->unique_name); + } + else if (extra_master) + { + PRINT_MXS_JSON_ERROR(error_out, "Cluster has an additional master server '%s'.", + extra_master->server->unique_name); + } + return server_is_master && !extra_master; } /** * Check whether specified new master is acceptable. * - * @param new_master The specified new master. * @param monitored_server The server to check against. - * @param monitored_new_master On output, @c monitored_server, if @c monitored_server - * is the same server as @c new_master. * @param error On output, error object if function failed. * - * @return False, if there is some error with the specified current master, - * True otherwise. + * @return True, if suggested new master is a viable promotion candidate. */ -bool mysql_switchover_check_new(SERVER* new_master, - MXS_MONITORED_SERVER* monitored_server, - MXS_MONITORED_SERVER** monitored_new_master, - json_t** error) +bool mysql_switchover_check_new(const MXS_MONITORED_SERVER* monitored_server, json_t** error) { - bool rv = true; - bool is_master = SERVER_IS_MASTER(monitored_server->server); + SERVER* server = monitored_server->server; + const char* name = server->unique_name; + bool is_master = SERVER_IS_MASTER(server); + bool is_slave = SERVER_IS_SLAVE(server); - if (new_master == monitored_server->server) + if (is_master) { - if (!is_master) - { - *monitored_new_master = monitored_server; - } - else - { - *error = mxs_json_error("Specified new master %s is already the current master.", - new_master->unique_name); - rv = false; - } + const char IS_MASTER[] = "Specified new master '%s' is already the current master."; + PRINT_MXS_JSON_ERROR(error, IS_MASTER, name); + } + else if (!is_slave) + { + const char NOT_SLAVE[] = "Specified new master '%s' is not a slave."; + PRINT_MXS_JSON_ERROR(error, NOT_SLAVE, name); } - return rv; -} - -/** - * Check whether specified current and new master are acceptable. - * - * @param mon The monitor. - * @param new_master The specified new master. - * @param current_master The specifiec current master (may be NULL). - * @param monitored_new_master On output, the monitored server corresponding to - * @c new_master. - * @param monitored_current_master On output, the monitored server corresponding to - * @c current_master. - * @param error On output, error object if function failed. - * - * @return True if switchover can proceeed, false otherwise. - */ -bool mysql_switchover_check(MXS_MONITOR* mon, - SERVER* new_master, - SERVER* current_master, - MXS_MONITORED_SERVER** monitored_new_master, - MXS_MONITORED_SERVER** monitored_current_master, - json_t** error) -{ - bool rv = true; - - *monitored_new_master = NULL; - *monitored_current_master = NULL; - *error = NULL; - - MXS_MONITORED_SERVER* monitored_server = mon->monitored_servers; - - while (rv && monitored_server && (!*monitored_current_master || !*monitored_new_master)) - { - if (!*monitored_current_master) - { - rv = mysql_switchover_check_current(current_master, - monitored_server, - monitored_current_master, - error); - } - - if (rv) - { - rv = mysql_switchover_check_new(new_master, monitored_server, monitored_new_master, error); - } - - monitored_server = monitored_server->next; - } - - if (rv && ((current_master && !*monitored_current_master) || !*monitored_new_master)) - { - if (current_master && !*monitored_current_master) - { - *error = mxs_json_error("Specified current master %s is not found amongst " - "existing servers.", current_master->unique_name); - } - - if (!*monitored_new_master) - { - *error = mxs_json_error_append(*error, - "Specified new master %s is not found amongst " - "existing servers.", new_master->unique_name); - } - - rv = false; - } - - return rv; + return !is_master && is_slave; } /** @@ -303,10 +424,13 @@ bool mysql_switchover_check(MXS_MONITOR* mon, * @param error_out JSON error out * @return True if failover may proceed */ -bool mysql_failover_check(MYSQL_MONITOR* mon, json_t** error_out) +bool failover_check(MYSQL_MONITOR* mon, json_t** error_out) { // Check that there is no running master and that there is at least one running server in the cluster. + // Also, all slaves must be using gtid-replication. int slaves = 0; + bool error = false; + for (MXS_MONITORED_SERVER* mon_server = mon->monitor->monitored_servers; mon_server != NULL; mon_server = mon_server->next) @@ -322,19 +446,31 @@ bool mysql_failover_check(MYSQL_MONITOR* mon, json_t** error_out) master_up_msg += ", although in maintenance mode"; } master_up_msg += "."; - PRINT_MXS_JSON_ERROR(error_out, "%s Failover not allowed.", master_up_msg.c_str()); - return false; + PRINT_MXS_JSON_ERROR(error_out, "%s", master_up_msg.c_str()); + error = true; } else if (SERVER_IS_SLAVE(mon_server->server)) { - slaves++; + if (uses_gtid(mon, mon_server, error_out)) + { + slaves++; + } + else + { + error = true; + } } } - if (slaves == 0) + + if (error) + { + PRINT_MXS_JSON_ERROR(error_out, "Failover not allowed due to errors."); + } + else if (slaves == 0) { PRINT_MXS_JSON_ERROR(error_out, "No running slaves, cannot failover."); } - return slaves > 0; + return !error && slaves > 0; } /** @@ -347,16 +483,9 @@ bool mysql_failover_check(MYSQL_MONITOR* mon, json_t** error_out) * * @return True, if switchover was performed, false otherwise. */ -bool mysql_switchover(MXS_MONITOR* mon, SERVER* new_master, SERVER* current_master, json_t** output) +bool mysql_switchover(MXS_MONITOR* mon, MXS_MONITORED_SERVER* new_master, MXS_MONITORED_SERVER* current_master, json_t** error_out) { - bool rv = true; - - MYSQL_MONITOR *handle = static_cast(mon->handle); - - *output = NULL; - bool stopped = stop_monitor(mon); - if (stopped) { MXS_NOTICE("Stopped the monitor %s for the duration of switchover.", mon->name); @@ -366,62 +495,55 @@ bool mysql_switchover(MXS_MONITOR* mon, SERVER* new_master, SERVER* current_mast MXS_NOTICE("Monitor %s already stopped, switchover can proceed.", mon->name); } - MXS_MONITORED_SERVER* monitored_new_master = NULL; - MXS_MONITORED_SERVER* monitored_current_master = NULL; + bool rval = false; + MYSQL_MONITOR* handle = static_cast(mon->handle); - rv = mysql_switchover_check(mon, - new_master, current_master, - &monitored_new_master, &monitored_current_master, - output); - - if (rv) + bool current_ok = mysql_switchover_check_current(handle, current_master, error_out); + bool new_ok = mysql_switchover_check_new(new_master, error_out); + // Check that all slaves are using gtid-replication + bool gtid_ok = true; + for (MXS_MONITORED_SERVER* mon_serv = mon->monitored_servers; mon_serv != NULL; mon_serv = mon_serv->next) { - bool failover = config_get_bool(mon->parameters, CN_AUTO_FAILOVER); - rv = do_switchover(handle, monitored_current_master, monitored_new_master, output); - - if (rv) + if (SERVER_IS_SLAVE(mon_serv->server)) { - MXS_NOTICE("Switchover %s -> %s performed.", - current_master->unique_name ? current_master->unique_name : "none", - new_master->unique_name); - - if (stopped) + if (!uses_gtid(handle, mon_serv, error_out)) { - startMonitor(mon, mon->parameters); + gtid_ok = false; } } + } + + if (current_ok && new_ok && gtid_ok) + { + bool switched = do_switchover(handle, current_master, new_master, error_out); + + const char* curr_master_name = current_master->server->unique_name; + const char* new_master_name = new_master->server->unique_name; + + if (switched) + { + MXS_NOTICE("Switchover %s -> %s performed.", curr_master_name, new_master_name); + rval = true; + } else { + string format = "Switchover %s -> %s failed"; + bool failover = config_get_bool(mon->parameters, CN_AUTO_FAILOVER); if (failover) { - // TODO: There could be a more convenient way for this. - MXS_CONFIG_PARAMETER p = {}; - p.name = const_cast(CN_AUTO_FAILOVER); - p.value = const_cast("false"); - - monitorAddParameters(mon, &p); - - MXS_ALERT("Switchover %s -> %s failed, failover has been disabled.", - current_master->unique_name ? current_master->unique_name : "none", - new_master->unique_name); - } - else - { - MXS_ERROR("Switchover %s -> %s failed.", - current_master->unique_name ? current_master->unique_name : "none", - new_master->unique_name); + disable_setting(handle, CN_AUTO_FAILOVER); + format += ", failover has been disabled."; } + format += "."; + PRINT_MXS_JSON_ERROR(error_out, format.c_str(), curr_master_name, new_master_name); } } - else + + if (stopped) { - if (stopped) - { - startMonitor(mon, mon->parameters); - } + startMonitor(mon, mon->parameters); } - - return rv; + return rval; } /** @@ -432,37 +554,68 @@ bool mysql_switchover(MXS_MONITOR* mon, SERVER* new_master, SERVER* current_mast * * @return True, if the command was executed, false otherwise. */ -bool mysql_handle_switchover(const MODULECMD_ARG* args, json_t** output) +bool mysql_handle_switchover(const MODULECMD_ARG* args, json_t** error_out) { ss_dassert((args->argc == 2) || (args->argc == 3)); ss_dassert(MODULECMD_GET_TYPE(&args->argv[0].type) == MODULECMD_ARG_MONITOR); ss_dassert(MODULECMD_GET_TYPE(&args->argv[1].type) == MODULECMD_ARG_SERVER); - ss_dassert((args->argc == 2) || - (MODULECMD_GET_TYPE(&args->argv[2].type) == MODULECMD_ARG_SERVER)); + ss_dassert((args->argc == 2) || (MODULECMD_GET_TYPE(&args->argv[2].type) == MODULECMD_ARG_SERVER)); MXS_MONITOR* mon = args->argv[0].value.monitor; - MYSQL_MONITOR* mysql_mon = static_cast(mon->handle); SERVER* new_master = args->argv[1].value.server; SERVER* current_master = (args->argc == 3) ? args->argv[2].value.server : NULL; + bool error = false; - bool rv = false; - - if (!config_get_global_options()->passive) + const char NO_SERVER[] = "Server '%s' is not a member of monitor '%s'."; + MXS_MONITORED_SERVER* mon_new_master = mon_get_monitored_server(mon, new_master); + if (mon_new_master == NULL) { - rv = mysql_switchover(mon, new_master, current_master, output); + PRINT_MXS_JSON_ERROR(error_out, NO_SERVER, new_master->unique_name, mon->name); + error = true; + } + + MXS_MONITORED_SERVER* mon_curr_master = NULL; + if (current_master) + { + mon_curr_master = mon_get_monitored_server(mon, current_master); + if (mon_curr_master == NULL) + { + PRINT_MXS_JSON_ERROR(error_out, NO_SERVER, current_master->unique_name, mon->name); + error = true; + } } else { - MXS_WARNING("Attempt to perform switchover %s -> %s, even though " - "MaxScale is in passive mode.", - current_master ? current_master->unique_name : "none", - new_master->unique_name); - *output = mxs_json_error("Switchover %s -> %s not performed, as MaxScale is in passive mode.", - current_master ? current_master->unique_name : "none", - new_master->unique_name); + // Autoselect current master + MYSQL_MONITOR* handle = static_cast(mon->handle); + if (handle->master) + { + mon_curr_master = handle->master; + } + else + { + const char NO_MASTER[] = "Monitor '%s' has no master server."; + PRINT_MXS_JSON_ERROR(error_out, NO_MASTER, mon->name); + error = true; + } + } + if (error) + { + return false; } - return rv; + bool rval = false; + if (!config_get_global_options()->passive) + { + rval = mysql_switchover(mon, mon_new_master, mon_curr_master, error_out); + } + else + { + const char MSG[] = "Switchover attempted but not performed, as MaxScale is in passive mode."; + PRINT_MXS_JSON_ERROR(error_out, MSG); + } + + return rval; } /** @@ -474,8 +627,6 @@ bool mysql_handle_switchover(const MODULECMD_ARG* args, json_t** output) */ bool mysql_failover(MXS_MONITOR* mon, json_t** output) { - bool rv = true; - MYSQL_MONITOR *handle = static_cast(mon->handle); bool stopped = stop_monitor(mon); if (stopped) { @@ -486,29 +637,25 @@ bool mysql_failover(MXS_MONITOR* mon, json_t** output) MXS_NOTICE("Monitor %s already stopped, failover can proceed.", mon->name); } - rv = mysql_failover_check(handle, output); + bool rv = true; + MYSQL_MONITOR *handle = static_cast(mon->handle); + rv = failover_check(handle, output); if (rv) { rv = do_failover(handle, output); if (rv) { MXS_NOTICE("Failover performed."); - if (stopped) - { - startMonitor(mon, mon->parameters); - } } else { PRINT_MXS_JSON_ERROR(output, "Failover failed."); } } - else + + if (stopped) { - if (stopped) - { - startMonitor(mon, mon->parameters); - } + startMonitor(mon, mon->parameters); } return rv; } @@ -549,7 +696,6 @@ bool mysql_handle_failover(const MODULECMD_ARG* args, json_t** output) */ bool mysql_rejoin(MXS_MONITOR* mon, SERVER* rejoin_server, json_t** output) { - MYSQL_MONITOR *handle = static_cast(mon->handle); bool stopped = stop_monitor(mon); if (stopped) { @@ -561,20 +707,10 @@ bool mysql_rejoin(MXS_MONITOR* mon, SERVER* rejoin_server, json_t** output) } bool rval = false; + MYSQL_MONITOR *handle = static_cast(mon->handle); if (cluster_can_be_joined(handle)) { - MXS_MONITORED_SERVER* mon_server = NULL; - // Search for the MONITORED_SERVER. Could this be a general monitor function? - for (MXS_MONITORED_SERVER* iterator = mon->monitored_servers; - iterator != NULL && mon_server == NULL; - iterator = iterator->next) - { - if (iterator->server == rejoin_server) - { - mon_server = iterator; - } - } - + MXS_MONITORED_SERVER* mon_server = mon_get_monitored_server(mon, rejoin_server); if (mon_server) { MXS_MONITORED_SERVER* master = handle->master; @@ -673,7 +809,7 @@ extern "C" ARG_MONITOR_DESC }, { MODULECMD_ARG_SERVER, "New master" }, - { MODULECMD_ARG_SERVER | MODULECMD_ARG_OPTIONAL, "Current master (obligatory if exists)" } + { MODULECMD_ARG_SERVER | MODULECMD_ARG_OPTIONAL, "Current master (optional)" } }; modulecmd_register_command(MXS_MODULE_NAME, "switchover", MODULECMD_TYPE_ACTIVE, @@ -766,170 +902,6 @@ extern "C" } -class Gtid -{ -public: - uint32_t domain; - uint32_t server_id; - uint64_t sequence; - Gtid() - : domain(0) - , server_id(0) - , sequence(0) - {} - - /** - * Parse a Gtid-triplet from a string. In case of a multi-triplet value, only the triplet with - * the given domain is returned. - * - * @param str Gtid string - * @param search_domain The Gtid domain whose triplet should be returned. Negative domain stands for - * autoselect, which is only allowed when the string contains one triplet. - */ - Gtid(const char* str, int64_t search_domain = -1) - : domain(0) - , server_id(0) - , sequence(0) - { - // Autoselect only allowed with one triplet - ss_dassert(search_domain >= 0 || strchr(str, ',') == NULL); - parse_triplet(str); - if (search_domain >= 0 && domain != search_domain) - { - // Search for the correct triplet. - bool found = false; - for (const char* next_triplet = strchr(str, ','); - next_triplet != NULL && !found; - next_triplet = strchr(next_triplet, ',')) - { - parse_triplet(++next_triplet); - if (domain == search_domain) - { - found = true; - } - } - ss_dassert(found); - } - } - bool operator == (const Gtid& rhs) const - { - return domain == rhs.domain && server_id == rhs.server_id && sequence == rhs.sequence; - } - string to_string() const - { - std::stringstream ss; - ss << domain << "-" << server_id << "-" << sequence; - return ss.str(); - } -private: - void parse_triplet(const char* str) - { - ss_debug(int rv = ) sscanf(str, "%" PRIu32 "-%" PRIu32 "-%" PRIu64, &domain, &server_id, &sequence); - ss_dassert(rv == 3); - } -}; -// Contains data returned by one row of SHOW ALL SLAVES STATUS -class SlaveStatusInfo -{ -public: - int master_server_id; /**< The master's server_id value. */ - string master_host; /**< Master server host name. */ - int master_port; /**< Master server port. */ - bool slave_io_running; /**< Whether the slave I/O thread is running and connected. */ - bool slave_sql_running; /**< Whether or not the SQL thread is running. */ - string master_log_file; /**< Name of the master binary log file that the I/O thread is currently - * reading from. */ - uint64_t read_master_log_pos; /**< Position up to which the I/O thread has read in the current master - * binary log file. */ - Gtid gtid_io_pos; /**< Gtid I/O position of the slave thread. Only shows the triplet with - * the current master domain. */ - string last_error; /**< Last IO or SQL error encountered. */ - - SlaveStatusInfo() - : master_server_id(0), - master_port(0), - slave_io_running(false), - slave_sql_running(false), - read_master_log_pos(0) - {} -}; - -// This class groups some miscellaneous replication related settings together. -class ReplicationSettings -{ -public: - bool gtid_strict_mode; /**< Enable additional checks for replication */ - bool log_bin; /**< Is binary logging enabled */ - bool log_slave_updates;/**< Does the slave log replicated events to binlog */ - ReplicationSettings() - : gtid_strict_mode(false) - , log_bin(false) - , log_slave_updates(false) - {} -}; - -/** - * Monitor specific information about a server - * - * Note: These are initialized in @c init_server_info - */ -class MySqlServerInfo -{ -public: - int server_id; /**< Value of @@server_id */ - int group; /**< Multi-master group where this server belongs, 0 for servers not in groups */ - bool read_only; /**< Value of @@read_only */ - bool slave_configured; /**< Whether SHOW SLAVE STATUS returned rows */ - bool binlog_relay; /** Server is a Binlog Relay */ - int n_slaves_configured; /**< Number of configured slave connections*/ - int n_slaves_running; /**< Number of running slave connections */ - int slave_heartbeats; /**< Number of received heartbeats */ - double heartbeat_period; /**< The time interval between heartbeats */ - time_t latest_event; /**< Time when latest event was received from the master */ - int64_t gtid_domain_id; /**< The value of gtid_domain_id, the domain which is used for new non- - * replicated events. */ - Gtid gtid_current_pos; /**< Gtid of latest event. Only shows the triplet - * with the current master domain. */ - Gtid gtid_binlog_pos; /**< Gtid of latest event written to binlog. Only shows the triplet - * with the current master domain. */ - SlaveStatusInfo slave_status; /**< Data returned from SHOW SLAVE STATUS */ - ReplicationSettings rpl_settings; /**< Miscellaneous replication related settings */ - mysql_server_version version; /**< Server version, 10.X, 5.5 or 5.1 */ - - MySqlServerInfo() - : server_id(0), - group(0), - read_only(false), - slave_configured(false), - binlog_relay(false), - n_slaves_configured(0), - n_slaves_running(0), - slave_heartbeats(0), - heartbeat_period(0), - latest_event(0), - gtid_domain_id(-1), - version(MYSQL_SERVER_VERSION_51) - {} - - /** - * Calculate how many events are left in the relay log. If gtid_current_pos is ahead of Gtid_IO_Pos, - * or a server_id is zero, an error value is returned. - * - * @return Number of events in relay log according to latest queried info. A negative value signifies - * an error in the gtid-values. - */ - int64_t relay_log_events() - { - if (slave_status.gtid_io_pos.server_id != 0 && gtid_current_pos.server_id != 0 && - slave_status.gtid_io_pos.domain == gtid_current_pos.domain && - slave_status.gtid_io_pos.sequence >= gtid_current_pos.sequence) - { - return slave_status.gtid_io_pos.sequence - gtid_current_pos.sequence; - } - return -1; - } -}; - void* info_copy_func(const void *val) { ss_dassert(val); @@ -1187,12 +1159,12 @@ static void diagnostics(DCB *dcb, const MXS_MONITOR *mon) { MySqlServerInfo *serv_info = get_server_info(handle, db); dcb_printf(dcb, "Server: %s\n", db->server->unique_name); - dcb_printf(dcb, "Server ID: %d\n", serv_info->server_id); + dcb_printf(dcb, "Server ID: %" PRId64 "\n", serv_info->server_id); dcb_printf(dcb, "Read only: %s\n", serv_info->read_only ? "ON" : "OFF"); dcb_printf(dcb, "Slave configured: %s\n", serv_info->slave_configured ? "YES" : "NO"); dcb_printf(dcb, "Slave IO running: %s\n", serv_info->slave_status.slave_io_running ? "YES" : "NO"); dcb_printf(dcb, "Slave SQL running: %s\n", serv_info->slave_status.slave_sql_running ? "YES" : "NO"); - dcb_printf(dcb, "Master ID: %d\n", serv_info->slave_status.master_server_id); + dcb_printf(dcb, "Master ID: %" PRId64 "\n", serv_info->slave_status.master_server_id); dcb_printf(dcb, "Master binlog file: %s\n", serv_info->slave_status.master_log_file.c_str()); dcb_printf(dcb, "Master binlog position: %lu\n", serv_info->slave_status.read_master_log_pos); @@ -1320,7 +1292,7 @@ static bool do_show_slave_status(MYSQL_MONITOR* mon, } MYSQL_RES* result; - int master_server_id = -1; + int64_t master_server_id = SERVER_ID_UNKNOWN; int nconfigured = 0; int nrunning = 0; @@ -1376,11 +1348,7 @@ static bool do_show_slave_status(MYSQL_MONITOR* mon, if (serv_info->slave_status.slave_io_running && server_version != MYSQL_SERVER_VERSION_51) { /* Get Master_Server_Id */ - master_server_id = atoi(row[i_master_server_id]); - if (master_server_id == 0) - { - master_server_id = -1; - } + master_server_id = scan_server_id(row[i_master_server_id]); } if (server_version == MYSQL_SERVER_VERSION_100) @@ -1458,7 +1426,7 @@ static bool slave_receiving_events(MYSQL_MONITOR* handle) { ss_dassert(handle->master); bool received_event = false; - long master_id = handle->master->server->node_id; + int64_t master_id = handle->master->server->node_id; for (MXS_MONITORED_SERVER* server = handle->monitor->monitored_servers; server; server = server->next) { MySqlServerInfo* info = get_server_info(handle, server); @@ -3258,7 +3226,7 @@ bool mon_process_failover(MYSQL_MONITOR* monitor, uint32_t failover_timeout, boo MXS_NOTICE("Performing automatic failover to replace failed master '%s'.", failed_master->server->unique_name); failed_master->new_event = false; - rval = mysql_failover_check(monitor, NULL) && do_failover(monitor, NULL); + rval = failover_check(monitor, NULL) && do_failover(monitor, NULL); if (rval) { *cluster_modified_out = true; @@ -3395,7 +3363,7 @@ MXS_MONITORED_SERVER* select_new_master(MYSQL_MONITOR* mon, if (check_replication_settings(cand, cand_info)) { bool select_this = false; - // If no candidate yet, accept any. + // If no candidate yet, accept any slave. Slaves have already been checked to use gtid. if (new_master == NULL) { select_this = true; @@ -4270,7 +4238,7 @@ static bool do_switchover(MYSQL_MONITOR* mon, MXS_MONITORED_SERVER* current_mast */ static void read_server_variables(MXS_MONITORED_SERVER* database, MySqlServerInfo* serv_info) { - string query = "SELECT @@server_id, @@read_only;"; + string query = "SELECT @@global.server_id, @@read_only;"; int columns = 2; if (serv_info->version == MYSQL_SERVER_VERSION_100) { @@ -4285,16 +4253,16 @@ static void read_server_variables(MXS_MONITORED_SERVER* database, MySqlServerInf StringVector row; if (query_one_row(database, query.c_str(), columns, &row)) { - uint32_t server_id = 0; - ss_debug(int rv = ) sscanf(row[ind_id].c_str(), "%" PRIu32, &server_id); - ss_dassert(rv == 1 && (row[ind_ro] == "0" || row[ind_ro] == "1")); + int64_t server_id = scan_server_id(row[ind_id].c_str()); database->server->node_id = server_id; serv_info->server_id = server_id; + + ss_dassert(row[ind_ro] == "0" || row[ind_ro] == "1"); serv_info->read_only = (row[ind_ro] == "1"); if (columns == 3) { uint32_t domain = 0; - ss_debug(rv = ) sscanf(row[ind_domain].c_str(), "%" PRIu32, &domain); + ss_debug(int rv = ) sscanf(row[ind_domain].c_str(), "%" PRIu32, &domain); ss_dassert(rv == 1); serv_info->gtid_domain_id = domain; } @@ -4324,7 +4292,7 @@ static bool can_replicate_from(MYSQL_MONITOR* mon, // The following are not sufficient requirements for replication to work, they only cover the basics. // If the servers have diverging histories, the redirection will seem to succeed but the slave IO // thread will stop in error. - if (slave_gtid.server_id != 0 && master_gtid.server_id != 0 && + if (slave_gtid.server_id != SERVER_ID_UNKNOWN && master_gtid.server_id != SERVER_ID_UNKNOWN && slave_gtid.domain == master_gtid.domain && slave_gtid.sequence <= master_info->gtid_current_pos.sequence) { @@ -4526,3 +4494,25 @@ static bool cluster_can_be_joined(MYSQL_MONITOR* mon) { return (mon->master != NULL && SERVER_IS_MASTER(mon->master->server) && mon->master_gtid_domain >= 0); } + +/** + * Scan a server id from a string. + * + * @param id_string + * @return Server id, or -1 if scanning fails + */ +static int64_t scan_server_id(const char* id_string) +{ + int64_t server_id = SERVER_ID_UNKNOWN; + ss_debug(int rv = ) sscanf(id_string, "%" PRId64, &server_id); + ss_dassert(rv == 1); + // Server id can be 0, which was even the default value until 10.2.1. + // KB is a bit hazy on this, but apparently when replicating, the server id should not be 0. Not sure, + // so MaxScale allows this. +#if defined(SS_DEBUG) + const int64_t SERVER_ID_MIN = std::numeric_limits::min(); + const int64_t SERVER_ID_MAX = std::numeric_limits::max(); +#endif + ss_dassert(server_id >= SERVER_ID_MIN && server_id <= SERVER_ID_MAX); + return server_id; +} diff --git a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.c b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.c index 77a3f7957..db1767754 100644 --- a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.c +++ b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.c @@ -583,7 +583,7 @@ static void do_handle_error(DCB *dcb, mxs_error_action_t action, const char *err */ if (!succp) { - session->state = SESSION_STATE_STOPPING; + poll_fake_hangup_event(session->client_dcb); } } @@ -599,13 +599,8 @@ static void gw_reply_on_error(DCB *dcb, mxs_auth_state_t state) MXS_SESSION *session = dcb->session; CHK_SESSION(session); - if (!dcb->dcb_errhandle_called) - { - do_handle_error(dcb, ERRACT_REPLY_CLIENT, - "Authentication with backend failed. Session will be closed."); - session->state = SESSION_STATE_STOPPING; - dcb->dcb_errhandle_called = true; - } + do_handle_error(dcb, ERRACT_REPLY_CLIENT, + "Authentication with backend failed. Session will be closed."); } /** @@ -1351,7 +1346,7 @@ static int gw_backend_close(DCB *dcb) session->state == SESSION_STATE_STOPPING && session->client_dcb->state == DCB_STATE_POLLING) { - dcb_close(session->client_dcb); + poll_fake_hangup_event(session->client_dcb); } return 1; diff --git a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc index af540bb32..5e8d19ee1 100644 --- a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc +++ b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc @@ -1090,8 +1090,7 @@ gw_read_finish_processing(DCB *dcb, GWBUF *read_buffer, uint64_t capabilities) dcb_close(dcb); MXS_ERROR("Routing the query failed. Session will be closed."); } - - if (proto->current_command == MXS_COM_QUIT) + else if (proto->current_command == MXS_COM_QUIT) { /** Close router session which causes closing of backends */ dcb_close(dcb); @@ -1404,29 +1403,20 @@ static int gw_client_close(DCB *dcb) */ static int gw_client_hangup_event(DCB *dcb) { - MXS_SESSION* session; - CHK_DCB(dcb); - session = dcb->session; + MXS_SESSION* session = dcb->session; - if (session != NULL && session->state == SESSION_STATE_ROUTER_READY) + if (session) { CHK_SESSION(session); + if (session->state != SESSION_STATE_DUMMY && !session_valid_for_pool(session)) + { + // The client did not send a COM_QUIT packet + modutil_send_mysql_err_packet(dcb, 0, 0, 1927, "08S01", "Connection killed by MaxScale"); + } + dcb_close(dcb); } - if (session != NULL && session->state == SESSION_STATE_STOPPING) - { - goto retblock; - } - - if (!session_valid_for_pool(session)) - { - // The client did not send a COM_QUIT packet - modutil_send_mysql_err_packet(dcb, 0, 0, 1927, "08S01", "Connection killed by MaxScale"); - } - dcb_close(dcb); - -retblock: return 1; } diff --git a/server/modules/routing/avrorouter/CMakeLists.txt b/server/modules/routing/avrorouter/CMakeLists.txt index d852862bb..769df5fb9 100644 --- a/server/modules/routing/avrorouter/CMakeLists.txt +++ b/server/modules/routing/avrorouter/CMakeLists.txt @@ -4,8 +4,12 @@ if(AVRO_FOUND AND JANSSON_FOUND) add_library(avrorouter SHARED avro.c ../binlogrouter/binlog_common.c avro_client.c avro_schema.c avro_rbr.c avro_file.c avro_index.c) set_target_properties(avrorouter PROPERTIES VERSION "1.0.0") set_target_properties(avrorouter PROPERTIES LINK_FLAGS -Wl,-z,defs) - target_link_libraries(avrorouter maxscale-common ${JANSSON_LIBRARIES} ${AVRO_LIBRARIES} maxavro sqlite3 lzma) + target_link_libraries(avrorouter maxscale-common ${JANSSON_LIBRARIES} ${AVRO_LIBRARIES} maxavro lzma) install_module(avrorouter core) + + if (BUILD_TESTS) + add_subdirectory(test) + endif() else() message(STATUS "No Avro C or Jansson libraries found, not building avrorouter.") endif() diff --git a/server/modules/routing/avrorouter/avro.c b/server/modules/routing/avrorouter/avro.c index a637ef6eb..a2c4ed5aa 100644 --- a/server/modules/routing/avrorouter/avro.c +++ b/server/modules/routing/avrorouter/avro.c @@ -60,7 +60,7 @@ static const char* avro_index_name = "avro.index"; static const char* create_table_regex = "(?i)create[a-z0-9[:space:]_]+table"; static const char* alter_table_regex = - "(?i)alter[[:space:]]+table.*column"; + "(?i)alter[[:space:]]+table"; /* The router entry points */ static MXS_ROUTER *createInstance(SERVICE *service, char **options); diff --git a/server/modules/routing/avrorouter/avro_file.c b/server/modules/routing/avrorouter/avro_file.c index f856cdd05..d37da88ce 100644 --- a/server/modules/routing/avrorouter/avro_file.c +++ b/server/modules/routing/avrorouter/avro_file.c @@ -1013,6 +1013,40 @@ void unify_whitespace(char *sql, int len) } } +/** + * A very simple function for stripping auto-generated executable comments + * + * Note that the string will not strip the trailing part of the comment, making + * the SQL invalid. + * + * @param sql String to modify + * @param len Pointer to current length of string, updated to new length if + * @c sql is modified + */ +static void strip_executable_comments(char *sql, int* len) +{ + if (strncmp(sql, "/*!", 3) == 0 || strncmp(sql, "/*M!", 4) == 0) + { + // Executable comment, remove it + char* p = sql + 3; + if (*p == '!') + { + p++; + } + + // Skip the versioning part + while (*p && isdigit(*p)) + { + p++; + } + + int n_extra = p - sql; + int new_len = *len - n_extra; + memmove(sql, sql + n_extra, new_len); + *len = new_len; + } +} + /** * @brief Handling of query events * @@ -1032,12 +1066,14 @@ void handle_query_event(AVRO_INSTANCE *router, REP_HEADER *hdr, int *pending_tra db[dblen] = 0; size_t sqlsz = len, tmpsz = len; - char *tmp = MXS_MALLOC(len); + char *tmp = MXS_MALLOC(len + 1); MXS_ABORT_IF_NULL(tmp); remove_mysql_comments((const char**)&sql, &sqlsz, &tmp, &tmpsz); sql = tmp; len = tmpsz; unify_whitespace(sql, len); + strip_executable_comments(sql, &len); + sql[len] = '\0'; static bool warn_not_row_format = true; @@ -1058,6 +1094,9 @@ void handle_query_event(AVRO_INSTANCE *router, REP_HEADER *hdr, int *pending_tra } } + char ident[MYSQL_TABLE_MAXLEN + MYSQL_DATABASE_MAXLEN + 2]; + read_table_identifier(db, sql, sql + len, ident, sizeof(ident)); + if (is_create_table_statement(router, sql, len)) { TABLE_CREATE *created = NULL; @@ -1077,7 +1116,7 @@ void handle_query_event(AVRO_INSTANCE *router, REP_HEADER *hdr, int *pending_tra } else { - created = table_create_alloc(sql, len, db); + created = table_create_alloc(ident, sql, len); } if (created && !save_and_replace_table_create(router, created)) @@ -1087,30 +1126,7 @@ void handle_query_event(AVRO_INSTANCE *router, REP_HEADER *hdr, int *pending_tra } else if (is_alter_table_statement(router, sql, len)) { - char ident[MYSQL_TABLE_MAXLEN + MYSQL_DATABASE_MAXLEN + 2]; - read_alter_identifier(sql, sql + len, ident, sizeof(ident)); - - bool combine = (strnlen(db, 1) && strchr(ident, '.') == NULL); - - size_t ident_len = strlen(ident) + 1; // + 1 for the NULL - - if (combine) - { - ident_len += (strlen(db) + 1); // + 1 for the "." - } - - char full_ident[ident_len]; - full_ident[0] = 0; // Set full_ident to "". - - if (combine) - { - strcat(full_ident, db); - strcat(full_ident, "."); - } - - strcat(full_ident, ident); - - TABLE_CREATE *created = hashtable_fetch(router->created_tables, full_ident); + TABLE_CREATE *created = hashtable_fetch(router->created_tables, ident); if (created) { @@ -1118,7 +1134,7 @@ void handle_query_event(AVRO_INSTANCE *router, REP_HEADER *hdr, int *pending_tra } else { - MXS_ERROR("Alter statement to a table with no create statement."); + MXS_ERROR("Alter statement to table '%s' has no preceding create statement.", ident); } } /* A transaction starts with this event */ diff --git a/server/modules/routing/avrorouter/avro_rbr.c b/server/modules/routing/avrorouter/avro_rbr.c index 7bd41b87a..38d57ed07 100644 --- a/server/modules/routing/avrorouter/avro_rbr.c +++ b/server/modules/routing/avrorouter/avro_rbr.c @@ -229,6 +229,7 @@ bool handle_row_event(AVRO_INSTANCE *router, REP_HEADER *hdr, uint8_t *ptr) { bool rval = false; uint8_t *start = ptr; + uint8_t *end = ptr + hdr->event_size - BINLOG_EVENT_HDR_LEN; uint8_t table_id_size = router->event_type_hdr_lens[hdr->event_type] == 6 ? 4 : 6; uint64_t table_id = 0; @@ -293,8 +294,9 @@ bool handle_row_event(AVRO_INSTANCE *router, REP_HEADER *hdr, uint8_t *ptr) snprintf(table_ident, sizeof(table_ident), "%s.%s", map->database, map->table); AVRO_TABLE* table = hashtable_fetch(router->open_tables, table_ident); TABLE_CREATE* create = map->table_create; + ss_dassert(hashtable_fetch(router->created_tables, table_ident) == create); - if (table && create && ncolumns == map->columns) + if (table && create && ncolumns == map->columns && create->columns == map->columns) { avro_value_t record; avro_generic_value_new(table->avro_writer_iface, &record); @@ -305,13 +307,12 @@ bool handle_row_event(AVRO_INSTANCE *router, REP_HEADER *hdr, uint8_t *ptr) int rows = 0; MXS_INFO("Row Event for '%s' at %lu", table_ident, router->current_pos); - while (ptr - start < hdr->event_size - BINLOG_EVENT_HDR_LEN) + while (ptr < end) { static uint64_t total_row_count = 1; MXS_INFO("Row %lu", total_row_count++); /** Add the current GTID and timestamp */ - uint8_t *end = ptr + hdr->event_size - BINLOG_EVENT_HDR_LEN; int event_type = get_event_type(hdr->event_type); prepare_record(router, hdr, event_type, &record); ptr = process_row_event_data(map, create, &record, ptr, col_present, end); @@ -353,6 +354,12 @@ bool handle_row_event(AVRO_INSTANCE *router, REP_HEADER *hdr, uint8_t *ptr) "binary logs or the stored schema was not correct.", map->database, map->table); } + else if (ncolumns == map->columns && create->columns != map->columns) + { + MXS_ERROR("Table map event has a different column count for table " + "%s.%s than the CREATE TABLE statement. Possible " + "unsupported DDL detected.", map->database, map->table); + } else { MXS_ERROR("Row event and table map event have different column " @@ -510,6 +517,23 @@ int get_metadata_len(uint8_t type) } \ }while(false) +// Debug function for checking whether a row event consists of only NULL values +static bool all_fields_null(uint8_t* null_bitmap, int ncolumns) +{ + bool rval = true; + + for (long i = 0; i < ncolumns; i++) + { + if (!bit_is_set(null_bitmap, ncolumns, i)) + { + rval = false; + break; + } + } + + return rval; +} + /** * @brief Extract the values from a single row in a row event * @@ -526,7 +550,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value { int npresent = 0; avro_value_t field; - long ncolumns = MXS_MIN(map->columns, create->columns); + long ncolumns = map->columns; uint8_t *metadata = map->column_metadata; size_t metadata_offset = 0; @@ -537,7 +561,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value /** Store the null value bitmap */ uint8_t *null_bitmap = ptr; ptr += (ncolumns + 7) / 8; - ss_dassert(ptr < end); + ss_dassert(ptr < end || (bit_is_set(null_bitmap, ncolumns, 0))); char trace[ncolumns][768]; memset(trace, 0, sizeof(trace)); @@ -576,7 +600,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value avro_value_set_string(&field, strval); sprintf(trace[i], "[%ld] ENUM: %lu bytes", i, bytes); ptr += bytes; - check_overflow(ptr < end); + check_overflow(ptr <= end); } else { @@ -612,7 +636,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value str[bytes] = '\0'; avro_value_set_string(&field, str); ptr += bytes; - check_overflow(ptr < end); + check_overflow(ptr <= end); } } else if (column_is_bit(map->column_types[i])) @@ -631,7 +655,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value avro_value_set_int(&field, value); sprintf(trace[i], "[%ld] BIT", i); ptr += bytes; - check_overflow(ptr < end); + check_overflow(ptr <= end); } else if (column_is_decimal(map->column_types[i])) { @@ -639,7 +663,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value ptr += unpack_decimal_field(ptr, metadata + metadata_offset, &f_value); avro_value_set_double(&field, f_value); sprintf(trace[i], "[%ld] DECIMAL", i); - check_overflow(ptr < end); + check_overflow(ptr <= end); } else if (column_is_variable_string(map->column_types[i])) { @@ -662,7 +686,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value buf[sz] = '\0'; ptr += sz; avro_value_set_string(&field, buf); - check_overflow(ptr < end); + check_overflow(ptr <= end); } else if (column_is_blob(map->column_types[i])) { @@ -681,7 +705,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); } - check_overflow(ptr < end); + check_overflow(ptr <= end); } else if (column_is_temporal(map->column_types[i])) { @@ -693,7 +717,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value format_temporal_value(buf, sizeof(buf), map->column_types[i], &tm); avro_value_set_string(&field, buf); sprintf(trace[i], "[%ld] %s: %s", i, column_type_to_string(map->column_types[i]), buf); - check_overflow(ptr < end); + check_overflow(ptr <= end); } /** All numeric types (INT, LONG, FLOAT etc.) */ else @@ -704,7 +728,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value &metadata[metadata_offset], lval); set_numeric_field_value(&field, map->column_types[i], &metadata[metadata_offset], lval); sprintf(trace[i], "[%ld] %s", i, column_type_to_string(map->column_types[i])); - check_overflow(ptr < end); + check_overflow(ptr <= end); } ss_dassert(metadata_offset <= map->column_metadata_size); metadata_offset += get_metadata_len(map->column_types[i]); diff --git a/server/modules/routing/avrorouter/avro_schema.c b/server/modules/routing/avrorouter/avro_schema.c index e76301838..c6a29279b 100644 --- a/server/modules/routing/avrorouter/avro_schema.c +++ b/server/modules/routing/avrorouter/avro_schema.c @@ -720,46 +720,28 @@ int resolve_table_version(const char* db, const char* table) /** * @brief Handle a query event which contains a CREATE TABLE statement - * @param sql Query SQL - * @param db Database where this query was executed + * + * @param ident Table identifier in database.table format + * @param sql The CREATE TABLE statement + * @param len Length of @c sql + * * @return New CREATE_TABLE object or NULL if an error occurred */ -TABLE_CREATE* table_create_alloc(const char* sql, int len, const char* db) +TABLE_CREATE* table_create_alloc(const char* ident, const char* sql, int len) { /** Extract the table definition so we can get the column names from it */ int stmt_len = 0; const char* statement_sql = get_table_definition(sql, len, &stmt_len); ss_dassert(statement_sql); + + char* tbl_start = strchr(ident, '.'); + ss_dassert(tbl_start); + *tbl_start++ = '\0'; + char table[MYSQL_TABLE_MAXLEN + 1]; char database[MYSQL_DATABASE_MAXLEN + 1]; - const char* err = NULL; - MXS_INFO("Create table: %.*s", len, sql); - - if (!statement_sql) - { - err = "table definition"; - } - else if (!get_table_name(sql, table)) - { - err = "table name"; - } - - if (get_database_name(sql, database)) - { - // The CREATE statement contains the database name - db = database; - } - else if (*db == '\0') - { - // No explicit or current database - err = "database name"; - } - - if (err) - { - MXS_ERROR("Malformed CREATE TABLE statement, could not extract %s: %.*s", err, len, sql); - return NULL; - } + strcpy(database, ident); + strcpy(table, tbl_start); int* lengths = NULL; char **names = NULL; @@ -773,13 +755,13 @@ TABLE_CREATE* table_create_alloc(const char* sql, int len, const char* db) { if ((rval = MXS_MALLOC(sizeof(TABLE_CREATE)))) { - rval->version = resolve_table_version(db, table); + rval->version = resolve_table_version(database, table); rval->was_used = false; rval->column_names = names; rval->column_lengths = lengths; rval->column_types = types; rval->columns = n_columns; - rval->database = MXS_STRDUP(db); + rval->database = MXS_STRDUP(database); rval->table = MXS_STRDUP(table); } @@ -930,7 +912,6 @@ static void remove_extras(char* str) ss_dassert(strlen(str) == len); } - static void remove_backticks(char* src) { char* dest = src; @@ -1134,6 +1115,110 @@ static const char* get_tok(const char* sql, int* toklen, const char* end) return NULL; } +static void rskip_whitespace(const char* sql, const char** end) +{ + const char* ptr = *end; + + while (ptr > sql && isspace(*ptr)) + { + ptr--; + } + + *end = ptr; +} + +static void rskip_token(const char* sql, const char** end) +{ + const char* ptr = *end; + + while (ptr > sql && !isspace(*ptr)) + { + ptr--; + } + + *end = ptr; +} + +static bool get_placement_specifier(const char* sql, const char* end, const char** tgt, int* tgt_len) +{ + bool rval = false; + ss_dassert(end > sql); + end--; + + *tgt = NULL; + *tgt_len = 0; + + // Skip any trailing whitespace + rskip_whitespace(sql, &end); + + if (*end == '`') + { + // Identifier, possibly AFTER `column` + const char* id_end = end; + end--; + + while (end > sql && *end != '`') + { + end--; + } + + const char* id_start = end + 1; + ss_dassert(*end == '`' && *id_end == '`'); + + end--; + + rskip_whitespace(sql, &end); + rskip_token(sql, &end); + + // end points to the character _before_ the token + end++; + + if (strncasecmp(end, "AFTER", 5) == 0) + { + // This column comes after the specified column + rval = true; + *tgt = id_start; + *tgt_len = id_end - id_start; + } + } + else + { + // Something else, possibly FIRST or un-backtick'd AFTER + const char* id_end = end + 1; // Points to either a trailing space or one-after-the-end + rskip_token(sql, &end); + + // end points to the character _before_ the token + end++; + + if (strncasecmp(end, "FIRST", 5) == 0) + { + // Put this column first + rval = true; + } + else + { + const char* id_start = end + 1; + + // Skip the whitespace and until the start of the current token + rskip_whitespace(sql, &end); + rskip_token(sql, &end); + + // end points to the character _before_ the token + end++; + + if (strncasecmp(end, "AFTER", 5) == 0) + { + // This column comes after the specified column + rval = true; + *tgt = id_start; + *tgt_len = id_end - id_start; + } + } + } + + return rval; +} + static bool tok_eq(const char *a, const char *b, size_t len) { size_t i = 0; @@ -1150,15 +1235,130 @@ static bool tok_eq(const char *a, const char *b, size_t len) return true; } -void read_alter_identifier(const char *sql, const char *end, char *dest, int size) +static void skip_whitespace(const char** saved) { - 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 + const char* ptr = *saved; + + while (*ptr && isspace(*ptr)) { - snprintf(dest, size, "%.*s", len, tok); - remove_backticks(dest); + ptr++; + } + + *saved = ptr; +} + +static void skip_token(const char** saved) +{ + const char* ptr = *saved; + + while (*ptr && !isspace(*ptr) && *ptr != '(' && *ptr != '.') + { + ptr++; + } + + *saved = ptr; +} + +static void skip_non_backtick(const char** saved) +{ + const char* ptr = *saved; + + while (*ptr && *ptr != '`') + { + ptr++; + } + + *saved = ptr; +} + +const char* keywords[] = +{ + "CREATE", + "DROP", + "ALTER", + "IF", + "EXISTS", + "REPLACE", + "OR", + "TABLE", + "NOT", + NULL +}; + +static bool token_is_keyword(const char* tok, int len) +{ + for (int i = 0; keywords[i]; i++) + { + if (strncasecmp(keywords[i], tok, len) == 0) + { + return true; + } + } + + return false; +} + +void read_table_identifier(const char* db, const char *sql, const char *end, char *dest, int size) +{ + const char* start; + int len = 0; + bool is_keyword = true; + + while (is_keyword) + { + skip_whitespace(&sql); // Leading whitespace + + if (*sql == '`') + { + // Quoted identifier, not a keyword + is_keyword = false; + sql++; + start = sql; + skip_non_backtick(&sql); + len = sql - start; + sql++; + } + else + { + start = sql; + skip_token(&sql); + len = sql - start; + is_keyword = token_is_keyword(start, len); + } + } + + skip_whitespace(&sql); // Space after first identifier + + if (*sql != '.') + { + // No explicit database + snprintf(dest, size, "%s.%.*s", db, len, start); + } + else + { + // Explicit database, skip the period + sql++; + skip_whitespace(&sql); // Space after first identifier + + const char* id_start; + int id_len = 0; + + if (*sql == '`') + { + sql++; + id_start = sql; + skip_non_backtick(&sql); + id_len = sql - id_start; + sql++; + } + else + { + id_start = sql; + skip_token(&sql); + id_len = sql - id_start; + } + + snprintf(dest, size, "%.*s.%.*s", len, start, id_len, id_start); } } @@ -1192,6 +1392,14 @@ int get_column_index(TABLE_CREATE *create, const char *tok, int len) char safe_tok[len + 2]; memcpy(safe_tok, tok, len); safe_tok[len] = '\0'; + + if (*safe_tok == '`') + { + int toklen = strlen(safe_tok) - 2; // Token length without backticks + memmove(safe_tok, safe_tok + 1, toklen); // Overwrite first backtick + safe_tok[toklen] = '\0'; // Null-terminate the string before the second backtick + } + fix_reserved_word(safe_tok); for (int x = 0; x < create->columns; x++) @@ -1206,6 +1414,35 @@ int get_column_index(TABLE_CREATE *create, const char *tok, int len) return idx; } +static bool not_column_operation(const char* tok, int len) +{ + const char* keywords[] = + { + "PRIMARY", + "UNIQUE", + "FULLTEXT", + "SPATIAL", + "PERIOD", + "PRIMARY", + "KEY", + "KEYS", + "INDEX", + "FOREIGN", + "CONSTRAINT", + NULL + }; + + for (int i = 0; keywords[i]; i++) + { + if (tok_eq(tok, keywords[i], strlen(keywords[i]))) + { + return true; + } + } + + return false; +} + bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end) { const char *tbl = strcasestr(sql, "table"), *def; @@ -1231,9 +1468,19 @@ bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end) if (tok) { - if (tok_eq(ptok, "add", plen) && tok_eq(tok, "column", len)) + if (not_column_operation(tok, len)) { + MXS_INFO("Statement doesn't affect columns, not processing: %s", sql); + return true; + } + else if (tok_eq(tok, "column", len)) + { + // Skip the optional COLUMN keyword tok = get_tok(tok + len, &len, end); + } + + if (tok_eq(ptok, "add", plen)) + { char avro_token[len + 1]; make_avro_token(avro_token, tok, len); bool is_new = true; @@ -1264,10 +1511,8 @@ bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end) tok = get_next_def(tok, end); len = 0; } - else if (tok_eq(ptok, "drop", plen) && tok_eq(tok, "column", len)) + else if (tok_eq(ptok, "drop", plen)) { - tok = get_tok(tok + len, &len, end); - int idx = get_column_index(create, tok, len); if (idx != -1) @@ -1291,10 +1536,8 @@ bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end) tok = get_next_def(tok, end); len = 0; } - else if (tok_eq(ptok, "change", plen) && tok_eq(tok, "column", len)) + else if (tok_eq(ptok, "change", plen)) { - tok = get_tok(tok + len, &len, end); - int idx = get_column_index(create, tok, len); if (idx != -1 && (tok = get_tok(tok + len, &len, end))) diff --git a/server/modules/routing/avrorouter/avrorouter.h b/server/modules/routing/avrorouter/avrorouter.h index 27818068c..67029e1c5 100644 --- a/server/modules/routing/avrorouter/avrorouter.h +++ b/server/modules/routing/avrorouter/avrorouter.h @@ -310,12 +310,12 @@ extern void read_table_info(uint8_t *ptr, uint8_t post_header_len, uint64_t *tab char* dest, size_t len); extern TABLE_MAP *table_map_alloc(uint8_t *ptr, uint8_t hdr_len, TABLE_CREATE* create); extern void table_map_free(TABLE_MAP *map); -extern TABLE_CREATE* table_create_alloc(const char* sql, int len, const char* db); +extern TABLE_CREATE* table_create_alloc(const char* ident, const char* sql, int len); extern TABLE_CREATE* table_create_copy(AVRO_INSTANCE *router, const char* sql, size_t len, const char* db); extern void table_create_free(TABLE_CREATE* value); extern bool table_create_save(TABLE_CREATE *create, const char *filename); extern bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end); -extern void read_alter_identifier(const char *sql, const char *end, char *dest, int size); +extern void read_table_identifier(const char* db, const char *sql, const char *end, char *dest, int size); extern int avro_client_handle_request(AVRO_INSTANCE *, AVRO_CLIENT *, GWBUF *); extern void avro_client_rotate(AVRO_INSTANCE *router, AVRO_CLIENT *client, uint8_t *ptr); extern bool avro_open_binlog(const char *binlogdir, const char *file, int *fd); diff --git a/server/modules/routing/avrorouter/test/CMakeLists.txt b/server/modules/routing/avrorouter/test/CMakeLists.txt new file mode 100644 index 000000000..016461f38 --- /dev/null +++ b/server/modules/routing/avrorouter/test/CMakeLists.txt @@ -0,0 +1,3 @@ +add_executable(test_alter_parsing test_alter_parsing.c) +target_link_libraries(test_alter_parsing maxscale-common ${JANSSON_LIBRARIES} ${AVRO_LIBRARIES} maxavro sqlite3 lzma) +add_test(test_alter_parsing test_alter_parsing) \ No newline at end of file diff --git a/server/modules/routing/avrorouter/test/test_alter_parsing.c b/server/modules/routing/avrorouter/test/test_alter_parsing.c new file mode 100644 index 000000000..c622d6e75 --- /dev/null +++ b/server/modules/routing/avrorouter/test/test_alter_parsing.c @@ -0,0 +1,99 @@ +#include "../avro_schema.c" + +static struct +{ + const char* statement; + const char* target; + bool rval; +} data[] = +{ + {"/*!40000 ALTER TABLE `t1` DISABLE KEYS */", NULL, false}, + {"/*!40000 ALTER TABLE `t1` ENABLE KEYS */", NULL, false}, + {"ADD COLUMN `a` INT", NULL, false}, + {"ADD COLUMN `a`", NULL, false}, + {"ALTER TABLE `t1` ADD `account_id` INT", NULL, false}, + {"ALTER TABLE `t1` ADD `amount` INT", NULL, false}, + {"ALTER TABLE `t1` ADD `app_id` VARCHAR(64)", NULL, false}, + {"ALTER TABLE `t1` ADD `create_time` DATETIME", NULL, false}, + {"alter TABLE t1 add `end_time` varchar(10) DEFAULT NULL COMMENT 'this is a comment'", NULL, false}, + {"ALTER TABLE `t1` ADD `expire_time` DATETIME", NULL, false}, + {"ALTER TABLE `t1` ADD `id_a` VARCHAR(128)", NULL, false}, + {"ALTER TABLE `t1` ADD `id` BIGINT(20)", NULL, false}, + {"ALTER TABLE `t1` ADD `id` VARCHAR(64)", NULL, false}, + {"ALTER TABLE `t1` ADD `node_state` INT(4)", NULL, false}, + {"ALTER TABLE `t1` ADD `no` INT", NULL, false}, + {"ALTER TABLE `t1` ADD `order_id` INT", NULL, false}, + {"alter TABLE t1 add `start_time` varchar(10) DEFAULT NULL COMMENT 'this is a comment'", NULL, false}, + {"ALTER TABLE `t1` ADD `status` INT", NULL, false}, + {"ALTER TABLE `t1` ADD `task_id` BIGINT(20)", NULL, false}, + {"alter TABLE t1 add `undo` int(1) DEFAULT '0' COMMENT 'this is a comment'", NULL, false}, + {"alter table `t1` add unique (`a`,`id`)", NULL, false}, + {"alter table `t1` add unique (`a`)", NULL, false}, + {"alter table `t1` add UNIQUE(`a`)", NULL, false}, + {"ALTER TABLE `t1` ADD UNIQUE `idx_id` USING BTREE (`id`, `result`)", NULL, false}, + {"ALTER TABLE `t1` ADD `update_time` INT", NULL, false}, + {"ALTER TABLE `t1` ADD `username` VARCHAR(16)", NULL, false}, + {"ALTER TABLE `t1` AUTO_INCREMENT = 1", NULL, false}, + {"ALTER TABLE `t1` CHANGE `account_id` `account_id` BIGINT(20)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `amount` `amount` DECIMAL(32,2)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `app_id` `app_id` VARCHAR(64)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `business_id` `business_id` VARCHAR(128)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `business_id` `business_id` VARCHAR(64)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `business_unique_no` `business_unique_no` VARCHAR(64)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `expire_time` `expire_time` DATETIME", NULL, false}, + {"ALTER TABLE `t1` CHANGE `id_a` `id_a` VARCHAR(128)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `id` `id` BIGINT(20)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `node_state` `node_state` INT(4)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `order_id` `order_id` BIGINT(20)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `status` `status` INT(1)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `update_time` `update_time` TIMESTAMP", NULL, false}, + {"ALTER TABLE `t1` CHANGE `username` `username` VARCHAR(16)", NULL, false}, + {"ALTER TABLE `t1` COMMENT = 'a comment'", NULL, false}, + {"alter table `t1` drop index a", NULL, false}, + {"alter table t1 drop index t1_idx", NULL, false}, + {"alter table t1 index(account_id, business_id)", NULL, false}, + {"ALTER TABLE `t1` MODIFY COLUMN `expire_time` DATETIME DEFAULT NULL COMMENT 'this is a comment' AFTER `update_time`", "update_time", true}, + {"ALTER TABLE `t1` MODIFY COLUMN `id_a` VARCHAR(128) CHARACTER SET utf8 COLLATE utf8_general_ci COMMENT 'this is a comment' AFTER `username`", "username", true}, + {"ALTER TABLE `t1` MODIFY COLUMN `number` VARCHAR(64) CHARACTER SET utf8 COLLATE utf8_general_ci DEFAULT NULL COMMENT 'this is a comment' AFTER `business_id`", "business_id", true}, + {"ALTER TABLE `t1` MODIFY COLUMN `task_id` BIGINT(20) DEFAULT NULL COMMENT 'this is a comment' AFTER `business_id`", "business_id", true}, + {"ALTER TABLE `t1` MODIFY COLUMN `username` VARCHAR(16) CHARACTER SET utf8 COLLATE utf8_general_ci NOT NULL COMMENT 'this is a comment' AFTER `business_id`", "business_id", true}, + {"ALTER TABLE `t1` RENAME `t2`", NULL, false}, + {"ALTER TABLE `db1`.`t1` ADD COLUMN `num` varchar(32) COMMENT 'this is a comment' AFTER `bank_name`", "bank_name", true}, + {"ALTER TABLE `db1`.`t1` ADD INDEX `idx_node_state` USING BTREE (`node_state`) comment ''", NULL, false}, + {"ALTER TABLE `db1`.`t1` CHANGE COLUMN `num` `code` varchar(32) DEFAULT NULL COMMENT 'this is a comment'", NULL, false}, + {"ALTER TABLE `db1`.`t1` DROP INDEX `a`, ADD INDEX `a` USING BTREE (`a`) comment ''", NULL, false}, + {"ALTER TABLE `db1`.`t1` DROP INDEX `a`, ADD INDEX `idx_a` USING BTREE (`a`) comment ''", NULL, false}, + {"ALTER TABLE `t1` CHANGE COLUMN `a` `c` INT AFTER `b`", "b", true}, + {"ALTER TABLE `t1` CHANGE COLUMN `a` `c` INT first", NULL, true}, + {"ALTER TABLE `t1` CHANGE COLUMN `a` `c` INT", NULL, false}, + {"ALTER TABLE `t1` MODIFY COLUMN `a` INT PRIMARY KEY", NULL, false}, + {NULL} +}; + +int main(int argc, char** argv) +{ + int rval = 0; + + for (int i = 0; data[i].statement; i++) + { + const char* target = NULL; + int len = 0; + const char* stmt = data[i].statement; + const char* end = data[i].statement + strlen(data[i].statement); + + if (get_placement_specifier(stmt, end, &target, &len) != data[i].rval) + { + const char* a = data[i].rval ? "true" : "false"; + const char* b = data[i].rval ? "false" : "true"; + printf("Expected '%s', got '%s' for '%s'\n", a, b, data[i].statement); + rval++; + } + else if (((bool)data[i].target != (bool)target) || (strncmp(target, data[i].target, len) != 0)) + { + printf("Expected '%s', got '%.*s' for '%s'\n", data[i].target, len, target, data[i].statement); + rval++; + } + } + + return rval; +} diff --git a/server/modules/routing/binlogrouter/CMakeLists.txt b/server/modules/routing/binlogrouter/CMakeLists.txt index d28ebae1a..a847e9792 100644 --- a/server/modules/routing/binlogrouter/CMakeLists.txt +++ b/server/modules/routing/binlogrouter/CMakeLists.txt @@ -1,11 +1,11 @@ add_library(binlogrouter SHARED blr.c blr_master.c blr_cache.c blr_slave.c blr_file.c) set_target_properties(binlogrouter PROPERTIES INSTALL_RPATH ${CMAKE_INSTALL_RPATH}:${MAXSCALE_LIBDIR} VERSION "2.0.0") set_target_properties(binlogrouter PROPERTIES LINK_FLAGS -Wl,-z,defs) -target_link_libraries(binlogrouter maxscale-common ${PCRE_LINK_FLAGS} uuid sqlite3) +target_link_libraries(binlogrouter maxscale-common ${PCRE_LINK_FLAGS} uuid) install_module(binlogrouter core) add_executable(maxbinlogcheck maxbinlogcheck.c blr_file.c blr_cache.c blr_master.c blr_slave.c blr.c) -target_link_libraries(maxbinlogcheck maxscale-common ${PCRE_LINK_FLAGS} uuid sqlite3) +target_link_libraries(maxbinlogcheck maxscale-common ${PCRE_LINK_FLAGS} uuid) install_executable(maxbinlogcheck core) diff --git a/server/modules/routing/binlogrouter/blr_file.c b/server/modules/routing/binlogrouter/blr_file.c index d40b99a85..41b707aa7 100644 --- a/server/modules/routing/binlogrouter/blr_file.c +++ b/server/modules/routing/binlogrouter/blr_file.c @@ -521,9 +521,12 @@ blr_file_create(ROUTER_INSTANCE *router, char *orig_file) { close(router->binlog_fd); spinlock_acquire(&router->binlog_lock); - int len = strlen(file); - memmove(router->binlog_name, file, len); - router->binlog_name[len] = '\0'; + + /// Use an intermediate buffer in case the source and destination overlap + char new_binlog[strlen(file) + 1]; + strcpy(new_binlog, file); + strcpy(router->binlog_name, new_binlog); + router->binlog_fd = fd; /* Initial position after the magic number */ router->current_pos = BINLOG_MAGIC_SIZE; diff --git a/server/modules/routing/binlogrouter/test/CMakeLists.txt b/server/modules/routing/binlogrouter/test/CMakeLists.txt index e93f9306a..d600418eb 100644 --- a/server/modules/routing/binlogrouter/test/CMakeLists.txt +++ b/server/modules/routing/binlogrouter/test/CMakeLists.txt @@ -1,5 +1,5 @@ if(BUILD_TESTS) add_executable(testbinlogrouter testbinlog.c ../blr.c ../blr_slave.c ../blr_master.c ../blr_file.c ../blr_cache.c) - target_link_libraries(testbinlogrouter maxscale-common ${PCRE_LINK_FLAGS} uuid sqlite3) + target_link_libraries(testbinlogrouter maxscale-common ${PCRE_LINK_FLAGS} uuid) add_test(NAME TestBinlogRouter COMMAND ./testbinlogrouter WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) endif() diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc index 142d2abf5..b9e25a200 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc @@ -97,13 +97,26 @@ route_target_t get_target_type(RWSplitSession *rses, GWBUF *buffer, uint8_t* command, uint32_t* type, uint32_t* stmt_id) { route_target_t route_target = TARGET_MASTER; + bool in_read_only_trx = rses->target_node && session_trx_is_read_only(rses->client_dcb->session); if (gwbuf_length(buffer) > MYSQL_HEADER_LEN) { *command = mxs_mysql_get_command(buffer); - *type = determine_query_type(buffer, *command); - handle_multi_temp_and_load(rses, buffer, *command, type); + /** + * If the session is inside a read-only transaction, we trust that the + * server acts properly even when non-read-only queries are executed. + * For this reason, we can skip the parsing of the statement completely. + */ + if (in_read_only_trx) + { + *type = QUERY_TYPE_READ; + } + else + { + *type = determine_query_type(buffer, *command); + handle_multi_temp_and_load(rses, buffer, *command, type); + } if (MXS_LOG_PRIORITY_IS_ENABLED(LOG_INFO)) { @@ -140,7 +153,8 @@ route_target_t get_target_type(RWSplitSession *rses, GWBUF *buffer, } else { - if (*command == MXS_COM_QUERY && + if (!in_read_only_trx && + *command == MXS_COM_QUERY && qc_get_operation(buffer) == QUERY_OP_EXECUTE) { std::string id = get_text_ps_id(buffer);