From a4c6f3542adfe8577b802476b68e8bf1c0684f4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 16 Apr 2019 18:17:02 +0300 Subject: [PATCH 01/15] MXS-2315: Tokenized CS version extraction The STL regex implementations have proven to be unreliable on older systems and replacing the regex with hand-written code for version extraction is less prone to break. --- server/modules/monitor/csmon/csmon.cc | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/server/modules/monitor/csmon/csmon.cc b/server/modules/monitor/csmon/csmon.cc index 607320ced..d57896644 100644 --- a/server/modules/monitor/csmon/csmon.cc +++ b/server/modules/monitor/csmon/csmon.cc @@ -56,17 +56,22 @@ static std::string do_query(MXS_MONITORED_SERVER* srv, const char* query) // Returns a numeric version similar to mysql_get_server_version int get_cs_version(MXS_MONITORED_SERVER* srv) { - // GCC 4.8 appears to have a broken std::regex_constants::ECMAScript that doesn't support brackets - std::regex re("Columnstore \\([0-9]*\\)[.]\\([0-9]*\\)[.]\\([0-9]*\\)-[0-9]*", - std::regex_constants::basic); - std::string result = do_query(srv, "SELECT @@version_comment"); - std::smatch match; int rval = 0; + std::string prefix = "Columnstore "; + std::string result = do_query(srv, "SELECT @@version_comment"); + auto pos = result.find(prefix); - if (std::regex_match(result, match, re) && match.size() == 4) + if (pos != std::string::npos) { - rval = atoi(match[1].str().c_str()) * 10000 + atoi(match[2].str().c_str()) * 100 - + atoi(match[3].str().c_str()); + std::istringstream os(result.substr(pos + prefix.length())); + int major = 0, minor = 0, patch = 0; + char dot; + os >> major; + os >> dot; + os >> minor; + os >> dot; + os >> patch; + rval = major * 10000 + minor * 100 + patch; } return rval; From cba23781e09dc6c2eec8802a943124126740d7d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 16 Apr 2019 22:55:39 +0300 Subject: [PATCH 02/15] Remove deprecated Galera options The Galera installation warns that the two removed options are deprecated. --- maxscale-system-test/mdbci/cnf/galera_server1.cnf | 6 ------ maxscale-system-test/mdbci/cnf/galera_server2.cnf | 6 ------ maxscale-system-test/mdbci/cnf/galera_server3.cnf | 6 ------ maxscale-system-test/mdbci/cnf/galera_server4.cnf | 6 ------ 4 files changed, 24 deletions(-) diff --git a/maxscale-system-test/mdbci/cnf/galera_server1.cnf b/maxscale-system-test/mdbci/cnf/galera_server1.cnf index a378434b0..b2b736319 100644 --- a/maxscale-system-test/mdbci/cnf/galera_server1.cnf +++ b/maxscale-system-test/mdbci/cnf/galera_server1.cnf @@ -16,9 +16,6 @@ innodb_file_per_table # To avoid issues with 'bulk mode inserts' using autoincrement fields innodb_autoinc_lock_mode=2 -# Required to prevent deadlocks on parallel transaction execution -innodb_locks_unsafe_for_binlog=1 - # Query Cache is not supported by Galera wsrep replication query_cache_size=0 query_cache_type=0 @@ -87,9 +84,6 @@ wsrep_auto_increment_control=1 # Retry autoinc insert, when the insert failed for "duplicate key error" wsrep_drupal_282555_workaround=0 -# Enable "strictly synchronous" semantics for read operations -wsrep_causal_reads=1 - # Command to call when node status or cluster membership changes. # Will be passed all or some of the following options: # --status - new status of this node diff --git a/maxscale-system-test/mdbci/cnf/galera_server2.cnf b/maxscale-system-test/mdbci/cnf/galera_server2.cnf index acfc2a358..da7130a92 100644 --- a/maxscale-system-test/mdbci/cnf/galera_server2.cnf +++ b/maxscale-system-test/mdbci/cnf/galera_server2.cnf @@ -16,9 +16,6 @@ innodb_file_per_table # To avoid issues with 'bulk mode inserts' using autoincrement fields innodb_autoinc_lock_mode=2 -# Required to prevent deadlocks on parallel transaction execution -innodb_locks_unsafe_for_binlog=1 - # Query Cache is not supported by Galera wsrep replication query_cache_size=0 query_cache_type=0 @@ -87,9 +84,6 @@ wsrep_auto_increment_control=1 # Retry autoinc insert, when the insert failed for "duplicate key error" wsrep_drupal_282555_workaround=0 -# Enable "strictly synchronous" semantics for read operations -wsrep_causal_reads=1 - # Command to call when node status or cluster membership changes. # Will be passed all or some of the following options: # --status - new status of this node diff --git a/maxscale-system-test/mdbci/cnf/galera_server3.cnf b/maxscale-system-test/mdbci/cnf/galera_server3.cnf index a393b7613..623760a02 100644 --- a/maxscale-system-test/mdbci/cnf/galera_server3.cnf +++ b/maxscale-system-test/mdbci/cnf/galera_server3.cnf @@ -16,9 +16,6 @@ innodb_file_per_table # To avoid issues with 'bulk mode inserts' using autoincrement fields innodb_autoinc_lock_mode=2 -# Required to prevent deadlocks on parallel transaction execution -innodb_locks_unsafe_for_binlog=1 - # Query Cache is not supported by Galera wsrep replication query_cache_size=0 query_cache_type=0 @@ -87,9 +84,6 @@ wsrep_auto_increment_control=1 # Retry autoinc insert, when the insert failed for "duplicate key error" wsrep_drupal_282555_workaround=0 -# Enable "strictly synchronous" semantics for read operations -wsrep_causal_reads=1 - # Command to call when node status or cluster membership changes. # Will be passed all or some of the following options: # --status - new status of this node diff --git a/maxscale-system-test/mdbci/cnf/galera_server4.cnf b/maxscale-system-test/mdbci/cnf/galera_server4.cnf index 2854acf02..41fcf9da5 100644 --- a/maxscale-system-test/mdbci/cnf/galera_server4.cnf +++ b/maxscale-system-test/mdbci/cnf/galera_server4.cnf @@ -16,9 +16,6 @@ innodb_file_per_table # To avoid issues with 'bulk mode inserts' using autoincrement fields innodb_autoinc_lock_mode=2 -# Required to prevent deadlocks on parallel transaction execution -innodb_locks_unsafe_for_binlog=1 - # Query Cache is not supported by Galera wsrep replication query_cache_size=0 query_cache_type=0 @@ -87,9 +84,6 @@ wsrep_auto_increment_control=1 # Retry autoinc insert, when the insert failed for "duplicate key error" wsrep_drupal_282555_workaround=0 -# Enable "strictly synchronous" semantics for read operations -wsrep_causal_reads=1 - # Command to call when node status or cluster membership changes. # Will be passed all or some of the following options: # --status - new status of this node From 163b3a2da53cc02d5645f2cf95307f6cc2230040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 17 Apr 2019 00:00:24 +0300 Subject: [PATCH 03/15] Fix unhandled promise rejection in create monitor If a monitor was created with an argument that was not a key-value type, a promise would be rejected outside of the main maxctrl function. Added a test case that covers this and fixed a few other test coverage problems that were present. --- maxctrl/lib/create.js | 11 ++++++----- maxctrl/test/createdestroy.js | 17 +++++++++++++++++ maxctrl/test/drain.js | 5 +++++ maxctrl/test/unknown.js | 3 ++- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/maxctrl/lib/create.js b/maxctrl/lib/create.js index 45d869579..954c4cd92 100644 --- a/maxctrl/lib/create.js +++ b/maxctrl/lib/create.js @@ -159,12 +159,10 @@ exports.builder = function(yargs) { } } - if (argv.params) { - var err = validateParams(argv, argv.params) - if (err) { - return Promise.reject(err) - } + var err = false; + if (argv.params) { + err = validateParams(argv, argv.params) monitor.data.attributes.parameters = argv.params.reduce(to_obj, {}) } @@ -182,6 +180,9 @@ exports.builder = function(yargs) { } maxctrl(argv, function(host) { + if (err) { + return Promise.reject(err) + } return doRequest(host, 'monitors', null, {method: 'POST', body: monitor}) }) }) diff --git a/maxctrl/test/createdestroy.js b/maxctrl/test/createdestroy.js index bde20b181..59a76ff3d 100644 --- a/maxctrl/test/createdestroy.js +++ b/maxctrl/test/createdestroy.js @@ -18,6 +18,18 @@ describe("Create/Destroy Commands", function() { .should.be.rejected }) + it('monitor without parameters fails due to missing user parameter', function() { + return verifyCommand('create monitor my-monitor mysqlmon', 'monitors/my-monitor') + .should.be.rejected + }) + + it('destroy monitor created without parameters', function() { + return doCommand('destroy monitor my-monitor') + .should.be.fulfilled + .then(() => doCommand('show monitor my-monitor')) + .should.be.rejected + }) + it('will not destroy the same monitor again', function() { return doCommand('destroy monitor my-monitor') .should.be.rejected @@ -38,6 +50,11 @@ describe("Create/Destroy Commands", function() { .should.be.rejected }) + it('will not create monitor with malformed parameters', function() { + return doCommand('create monitor my-monitor mariadbmon not-a-param') + .should.be.rejected + }) + it('create monitor with options', function() { return doCommand('unlink monitor MariaDB-Monitor server4') .then(() => verifyCommand('create monitor my-monitor mysqlmon --servers server4 --monitor-user maxuser --monitor-password maxpwd', diff --git a/maxctrl/test/drain.js b/maxctrl/test/drain.js index b4d5f6673..ec489b8d1 100644 --- a/maxctrl/test/drain.js +++ b/maxctrl/test/drain.js @@ -15,5 +15,10 @@ describe("Draining servers", function() { .should.eventually.have.string("Maintenance") }) + it('does not drain non-existent server', function() { + return doCommand('drain server not-a-server') + .should.be.rejected + }) + after(stopMaxScale) }); diff --git a/maxctrl/test/unknown.js b/maxctrl/test/unknown.js index 7f818095a..1d0a30470 100644 --- a/maxctrl/test/unknown.js +++ b/maxctrl/test/unknown.js @@ -19,7 +19,8 @@ describe("Unknown Commands", function() { 'alter', 'rotate', 'call', - 'cluster' + 'cluster', + 'drain' ] endpoints.forEach(function (i) { From 64e282f74b56491fe05ee4808ef5caac9f369c88 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Wed, 17 Apr 2019 11:19:07 +0300 Subject: [PATCH 04/15] Fix config of mysqlmon_multimaster_serverid --- .../cnf/maxscale.cnf.template.mysqlmon_multimaster_serverid | 2 ++ 1 file changed, 2 insertions(+) diff --git a/maxscale-system-test/cnf/maxscale.cnf.template.mysqlmon_multimaster_serverid b/maxscale-system-test/cnf/maxscale.cnf.template.mysqlmon_multimaster_serverid index 749147a54..a85c925f8 100644 --- a/maxscale-system-test/cnf/maxscale.cnf.template.mysqlmon_multimaster_serverid +++ b/maxscale-system-test/cnf/maxscale.cnf.template.mysqlmon_multimaster_serverid @@ -10,6 +10,7 @@ user=maxskysql password= skysql detect_stale_master=0 monitor_interval=1000 +assume_unique_hostnames=false [RW Split Router] type=service @@ -18,6 +19,7 @@ servers=server1, server2, server3, server4 user=maxskysql password=skysql slave_selection_criteria=LEAST_ROUTER_CONNECTIONS +max_slave_replication_lag=1 [Read Connection Router Slave] type=service From d77530a2a4f3b334ed449b033f309ca92b92708a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 17 Apr 2019 13:31:36 +0300 Subject: [PATCH 05/15] Use a known binlog name for Galera The binlog files for Galera shouldn't rely on implicit naming simply due to the fact that it logs a warning. --- maxscale-system-test/mdbci/cnf/galera_server1.cnf | 2 +- maxscale-system-test/mdbci/cnf/galera_server2.cnf | 2 +- maxscale-system-test/mdbci/cnf/galera_server3.cnf | 2 +- maxscale-system-test/mdbci/cnf/galera_server4.cnf | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/maxscale-system-test/mdbci/cnf/galera_server1.cnf b/maxscale-system-test/mdbci/cnf/galera_server1.cnf index b2b736319..ef833fd05 100644 --- a/maxscale-system-test/mdbci/cnf/galera_server1.cnf +++ b/maxscale-system-test/mdbci/cnf/galera_server1.cnf @@ -7,7 +7,7 @@ wsrep_on=ON # Row binary log format is required by Galera binlog_format=ROW -log-bin +log-bin=mar-bin # InnoDB is currently the only storage engine supported by Galera default-storage-engine=innodb diff --git a/maxscale-system-test/mdbci/cnf/galera_server2.cnf b/maxscale-system-test/mdbci/cnf/galera_server2.cnf index da7130a92..0df6aa09c 100644 --- a/maxscale-system-test/mdbci/cnf/galera_server2.cnf +++ b/maxscale-system-test/mdbci/cnf/galera_server2.cnf @@ -7,7 +7,7 @@ wsrep_on=ON # Row binary log format is required by Galera binlog_format=ROW -log-bin +log-bin=mar-bin # InnoDB is currently the only storage engine supported by Galera default-storage-engine=innodb diff --git a/maxscale-system-test/mdbci/cnf/galera_server3.cnf b/maxscale-system-test/mdbci/cnf/galera_server3.cnf index 623760a02..efc663255 100644 --- a/maxscale-system-test/mdbci/cnf/galera_server3.cnf +++ b/maxscale-system-test/mdbci/cnf/galera_server3.cnf @@ -7,7 +7,7 @@ wsrep_on=ON # Row binary log format is required by Galera binlog_format=ROW -log-bin +log-bin=mar-bin # InnoDB is currently the only storage engine supported by Galera default-storage-engine=innodb diff --git a/maxscale-system-test/mdbci/cnf/galera_server4.cnf b/maxscale-system-test/mdbci/cnf/galera_server4.cnf index 41fcf9da5..4a13b1776 100644 --- a/maxscale-system-test/mdbci/cnf/galera_server4.cnf +++ b/maxscale-system-test/mdbci/cnf/galera_server4.cnf @@ -7,7 +7,7 @@ wsrep_on=ON # Row binary log format is required by Galera binlog_format=ROW -log-bin +log-bin=mar-bin # InnoDB is currently the only storage engine supported by Galera default-storage-engine=innodb From b008c5a1e63a9cab4f292d19fee438de0ecee46a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 17 Apr 2019 14:10:48 +0300 Subject: [PATCH 06/15] Add links to the sizes section Links to the size type description in the documentation help explain what sort of values are expected. --- Documentation/Getting-Started/Configuration-Guide.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/Getting-Started/Configuration-Guide.md b/Documentation/Getting-Started/Configuration-Guide.md index 090093a97..d586efdac 100644 --- a/Documentation/Getting-Started/Configuration-Guide.md +++ b/Documentation/Getting-Started/Configuration-Guide.md @@ -855,7 +855,7 @@ than `0`, this configuration setting will not have an effect. #### `writeq_high_water` High water mark for network write buffer. Controls when network traffic -throtting is started. The parameter accepts size type values. +throtting is started. The parameter accepts [size type values](#sizes). More specifically, if the client side write queue is above this value, it will block traffic coming from backend servers. If the backend side write queue is @@ -869,7 +869,7 @@ throtting is enabled. By default, traffic throttling is disabled. Low water mark for network write buffer. Once the traffic throttling is enabled, it will only be disabled when the write queue is below `writeq_low_water`. The -parameter accepts size type values. The minimum allowed size is 512 +parameter accepts [size type values](#sizes). The minimum allowed size is 512 bytes. `writeq_high_water` must always be greater than `writeq_low_water`. #### `load_persisted_configs` From aca3d65bbba6187a0730e6a102158f999985f85d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 17 Apr 2019 09:34:01 +0300 Subject: [PATCH 07/15] MXS-2381: Add `alter user` The `alter user` command makes password changes easier and keeps the usage consistent across types in MaxScale. --- maxctrl/lib/alter.js | 22 ++++++++++++++++++++++ maxctrl/test/alter.js | 12 ++++++++++++ 2 files changed, 34 insertions(+) diff --git a/maxctrl/lib/alter.js b/maxctrl/lib/alter.js index 2c6294e26..88bd3eee8 100644 --- a/maxctrl/lib/alter.js +++ b/maxctrl/lib/alter.js @@ -135,6 +135,28 @@ exports.builder = function(yargs) { return updateValue(host, 'maxscale', 'data.attributes.parameters.' + argv.key, argv.value) }) }) + .command('user ', 'Alter admin user passwords', function(yargs) { + return yargs.epilog('Changes the password for a user. To change the user type, destroy the user and then create it again.') + .usage('Usage: alter user ') + }, function(argv) { + maxctrl(argv, function(host) { + + var user = { + 'data': { + 'id': argv.name, + 'type': 'inet', + 'attributes': { + 'password': argv.password + } + } + } + + return getJson(host, 'users/inet/' + argv.name) + .then((res) => user.data.attributes.account = res.data.attributes.account) + .then(() => doRequest(host, 'users/inet/' + argv.name, null, {method: 'DELETE'})) + .then(() => doRequest(host, 'users/inet', null, {method: 'POST', body: user})) + }) + }) .usage('Usage: alter ') .help() .command('*', 'the default command', {}, function(argv) { diff --git a/maxctrl/test/alter.js b/maxctrl/test/alter.js index a0d45b3f5..14fcf62df 100644 --- a/maxctrl/test/alter.js +++ b/maxctrl/test/alter.js @@ -108,5 +108,17 @@ describe("Alter Commands", function() { .should.be.rejected }) + it('creates user', function() { + return verifyCommand('create user testuser test', 'users/inet/testuser') + }) + + it('alters the password of a user', function() { + return verifyCommand('alter user testuser test2', 'users/inet/testuser') + }) + + it('destroys the altered user', function() { + return doCommand('destroy user testuser') + }) + after(stopMaxScale) }); From d13e6e56ee6d2c1ad108545d05945bdfc301a1a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 17 Apr 2019 10:39:10 +0300 Subject: [PATCH 08/15] MXS-2433: Never cache multi-packet queries If a query spans more than a single packet, it will never be successfully classified due to the fact that the complete SQL is never available to the query classifier. For this reason, it is pointless to cache them. --- server/core/query_classifier.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server/core/query_classifier.cc b/server/core/query_classifier.cc index aeeb25e1b..9208d31a9 100644 --- a/server/core/query_classifier.cc +++ b/server/core/query_classifier.cc @@ -176,10 +176,15 @@ public: mxb_assert(peek(canonical_stmt) == nullptr); mxb_assert(this_unit.classifier); + // 0xffffff is the maximum packet size, 4 is for packet header and 1 is for command byte. These are + // MariaDB/MySQL protocol specific values that are also defined in but + // should not be exposed to the core. + constexpr int64_t max_entry_size = 0xffffff - 5; + int64_t cache_max_size = this_unit.cache_max_size() / config_get_global_options()->n_threads; int64_t size = canonical_stmt.size(); - if (size <= cache_max_size) + if (size < max_entry_size && size <= cache_max_size) { int64_t required_space = (m_stats.size + size) - cache_max_size; From 61f728c05adf395ac71346752c0196c6bf144064 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 17 Apr 2019 10:45:35 +0300 Subject: [PATCH 09/15] MXS-2433: Reduce default query classifier cache size Reduced the default cache size from 40% to 15%. Most cases don't benefit from that much memory and the defaults have caused problems in live environments. --- Documentation/Getting-Started/Configuration-Guide.md | 4 +++- server/core/config.cc | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/Getting-Started/Configuration-Guide.md b/Documentation/Getting-Started/Configuration-Guide.md index d586efdac..4e77370f7 100644 --- a/Documentation/Getting-Started/Configuration-Guide.md +++ b/Documentation/Getting-Started/Configuration-Guide.md @@ -679,7 +679,9 @@ _qc_sqlite_. #### `query_classifier_cache_size` Specifies the maximum size of the query classifier cache. The default limit is -40% of total system memory. +15% of total system memory starting with MaxScale 2.3.7. In older versions the +default limit was 40% of total system memory. This feature was added in MaxScale +2.3.0. When the query classifier cache has been enabled, MaxScale will, after a statement has been parsed, store the classification result using the diff --git a/server/core/config.cc b/server/core/config.cc index 0fc27870f..df3dffced 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -2850,7 +2850,7 @@ void config_set_global_defaults() gateway.peer_password[0] = '\0'; gateway.log_target = MXB_LOG_TARGET_DEFAULT; - gateway.qc_cache_properties.max_size = get_total_memory() * 0.4; + gateway.qc_cache_properties.max_size = get_total_memory() * 0.15; if (gateway.qc_cache_properties.max_size == 0) { From 3e0492256519606afb41ca66dbb660ef07c963e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 17 Apr 2019 12:31:13 +0300 Subject: [PATCH 10/15] MXS-2415: Fix client callbacks in arvorouter The DCB callbacks shouldn't be used to send more events as they cause the callback to be called recursively. The recursive calls caused rows to be sent before the schemas for the rows were sent. Queuing the events via the worker mechanism prevents this. --- .../modules/routing/avrorouter/avro_client.cc | 29 ++++++------------- .../modules/routing/avrorouter/avrorouter.hh | 7 ++--- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/server/modules/routing/avrorouter/avro_client.cc b/server/modules/routing/avrorouter/avro_client.cc index e80d171d0..566b7b571 100644 --- a/server/modules/routing/avrorouter/avro_client.cc +++ b/server/modules/routing/avrorouter/avro_client.cc @@ -34,6 +34,7 @@ #include #include #include +#include std::pair get_avrofile_and_gtid(std::string file); @@ -239,22 +240,14 @@ bool file_in_dir(const char* dir, const char* file) } /** - * @brief The client callback for sending data - * - * @param dcb Client DCB - * @param reason Why the callback was called - * @param userdata Data provided when the callback was added - * @return Always 0 + * Queue the client callback for execution */ -int avro_client_callback(DCB* dcb, DCB_REASON reason, void* userdata) +void AvroSession::queue_client_callback() { - if (reason == DCB_REASON_DRAINED) - { - AvroSession* client = static_cast(userdata); - client->client_callback(); - } - - return 0; + auto worker = mxs::RoutingWorker::get(mxs::RoutingWorker::MAIN); + worker->execute([this]() { + client_callback(); + }, mxs::RoutingWorker::EXECUTE_QUEUED); } /** @@ -338,11 +331,7 @@ void AvroSession::process_command(GWBUF* queue) if (file_in_dir(router->avrodir.c_str(), avro_binfile.c_str())) { - /* set callback routine for data sending */ - dcb_add_callback(dcb, DCB_REASON_DRAINED, avro_client_callback, this); - - /* Add fake event that will call the avro_client_callback() routine */ - poll_fake_write_event(dcb); + queue_client_callback(); } else { @@ -734,7 +723,7 @@ void AvroSession::client_callback() if (next_file || read_more) { - poll_fake_write_event(dcb); + queue_client_callback(); } } diff --git a/server/modules/routing/avrorouter/avrorouter.hh b/server/modules/routing/avrorouter/avrorouter.hh index 7f22af073..d51f78e0d 100644 --- a/server/modules/routing/avrorouter/avrorouter.hh +++ b/server/modules/routing/avrorouter/avrorouter.hh @@ -170,11 +170,6 @@ public: */ int routeQuery(GWBUF* buffer); - /** - * Handler for the EPOLLOUT event - */ - void client_callback(); - private: AvroSession(Avro* instance, MXS_SESSION* session); @@ -187,6 +182,8 @@ private: bool seek_to_gtid(); bool stream_data(); void rotate_avro_file(std::string fullname); + void client_callback(); + void queue_client_callback(); }; void read_table_info(uint8_t* ptr, From 03dc969cf203decb4d0bc65280fe4f901da87830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 17 Apr 2019 16:22:58 +0300 Subject: [PATCH 11/15] Fix use-after-free in LocalClient If the DCB was closed before the handshake for the LocalCliet connection was received, the gw_decode_mysql_server_handshake would use the closed DCB to log the connection ID. Clearing out the pointer prevents it. --- server/modules/protocol/MySQL/mariadb_client.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/modules/protocol/MySQL/mariadb_client.cc b/server/modules/protocol/MySQL/mariadb_client.cc index 5a52ecbde..9f9f1a1e6 100644 --- a/server/modules/protocol/MySQL/mariadb_client.cc +++ b/server/modules/protocol/MySQL/mariadb_client.cc @@ -32,6 +32,8 @@ LocalClient::LocalClient(MYSQL_session* session, MySQLProtocol* proto, int fd) , m_self_destruct(false) { MXB_POLL_DATA::handler = LocalClient::poll_handler; + m_protocol.owner_dcb = nullptr; + m_protocol.stored_query = nullptr; } LocalClient::~LocalClient() From 7fc3527e1d5b3451f1983a38ebd348c3cb44728a Mon Sep 17 00:00:00 2001 From: Timofey Turenko Date: Thu, 18 Apr 2019 12:10:24 +0300 Subject: [PATCH 12/15] Check for Docker in run_npm_test.sh Lack of Docker can cause build failure To avoid it if Docker is not available test is not started, simply returns 0 It is hard to make Docker installation reliable especially for very old or very new distros --- test/run_npm_test.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/run_npm_test.sh b/test/run_npm_test.sh index 8cd614f5b..ef3c855df 100755 --- a/test/run_npm_test.sh +++ b/test/run_npm_test.sh @@ -12,6 +12,21 @@ then exit 1 fi +# Prevent failures in case if Docker is not available +command -v docker +if [ $? != 0 ] +then + echo "Docker is not available, skipping the test" + exit 0 +fi + +command -v docker-compose +if [ $? != 0 ] +then + echo "docker-compose is not available, skipping the test" + exit 0 +fi + srcdir=$1 testsrc=$2 testdir=$3 From 2d8a93e88d63317281ab37f9ec4e707bd6e26707 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Tue, 23 Apr 2019 11:17:01 +0300 Subject: [PATCH 13/15] Update 2.3.6 release date --- Documentation/Release-Notes/MaxScale-2.3.6-Release-Notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Release-Notes/MaxScale-2.3.6-Release-Notes.md b/Documentation/Release-Notes/MaxScale-2.3.6-Release-Notes.md index c2e248278..29ae50b32 100644 --- a/Documentation/Release-Notes/MaxScale-2.3.6-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-2.3.6-Release-Notes.md @@ -1,4 +1,4 @@ -# MariaDB MaxScale 2.3.6 Release Notes +# MariaDB MaxScale 2.3.6 Release Notes -- 2019-04-23 Release 2.3.6 is a GA release. From f41ce6db16631caa816e2c0742b428140b4b296b Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Tue, 23 Apr 2019 11:20:38 +0300 Subject: [PATCH 14/15] Update maintenance version for 2.3 --- VERSION23.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION23.cmake b/VERSION23.cmake index 643875ab7..3bceeb242 100644 --- a/VERSION23.cmake +++ b/VERSION23.cmake @@ -5,7 +5,7 @@ set(MAXSCALE_VERSION_MAJOR "2" CACHE STRING "Major version") set(MAXSCALE_VERSION_MINOR "3" CACHE STRING "Minor version") -set(MAXSCALE_VERSION_PATCH "6" CACHE STRING "Patch version") +set(MAXSCALE_VERSION_PATCH "7" CACHE STRING "Patch version") # This should only be incremented if a package is rebuilt set(MAXSCALE_BUILD_NUMBER 1 CACHE STRING "Release number") From ba79028a46936f4b17ad9a417dd5bccb9498bb57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 18 Apr 2019 10:18:29 +0300 Subject: [PATCH 15/15] Add debug assertions into the core The assertions make sure DCB writes and reads are only done by the thread that owns them. --- server/core/dcb.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/core/dcb.cc b/server/core/dcb.cc index 3b72d4748..c18ba056b 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -565,6 +565,7 @@ int dcb_read(DCB* dcb, GWBUF** head, int maxbytes) { + mxb_assert(dcb->poll.owner == RoutingWorker::get_current()); int nsingleread = 0; int nreadtotal = 0; @@ -904,6 +905,7 @@ static int dcb_log_errors_SSL(DCB* dcb, int ret) */ int dcb_write(DCB* dcb, GWBUF* queue) { + mxb_assert(dcb->poll.owner == RoutingWorker::get_current()); dcb->writeqlen += gwbuf_length(queue); // The following guarantees that queue is not NULL if (!dcb_write_parameter_check(dcb, queue)) @@ -3301,6 +3303,7 @@ public: RoutingWorker& rworker = static_cast(worker); if (dcb_is_still_valid(m_dcb, rworker.id()) && m_dcb->m_uid == m_uid) { + mxb_assert(m_dcb->poll.owner == RoutingWorker::get_current()); m_dcb->fakeq = m_buffer; dcb_handler(m_dcb, m_ev); } @@ -3321,6 +3324,7 @@ static void poll_add_event_to_dcb(DCB* dcb, GWBUF* buf, uint32_t ev) { if (dcb == this_thread.current_dcb) { + mxb_assert(dcb->poll.owner == RoutingWorker::get_current()); // If the fake event is added to the current DCB, we arrange for // it to be handled immediately in dcb_handler() when the handling // of the current events are done...