From 0de9561b5a619259cef9d0576503b1d7547aec0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 2 Mar 2017 11:11:17 +0200 Subject: [PATCH 1/6] MXS-1146: Fix command tracking for large packets The current command was updated with invalid data when the packet size exceeded 2^24 bytes. --- server/include/gw.h | 2 +- server/modules/include/mysql_client_server_protocol.h | 2 +- server/modules/protocol/mysql_client.c | 7 ++++++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/server/include/gw.h b/server/include/gw.h index 5f6ed7f76..4ba708562 100644 --- a/server/include/gw.h +++ b/server/include/gw.h @@ -59,7 +59,7 @@ #define GW_MYSQL_SERVER_CAPABILITIES_BYTE1 0xff #define GW_MYSQL_SERVER_CAPABILITIES_BYTE2 0xf7 #define GW_MYSQL_SERVER_LANGUAGE 0x08 -#define GW_MYSQL_MAX_PACKET_LEN 0xffffffL; +#define GW_MYSQL_MAX_PACKET_LEN 0xffffffL #define GW_MYSQL_SCRAMBLE_SIZE 20 // debug for mysql_* functions diff --git a/server/modules/include/mysql_client_server_protocol.h b/server/modules/include/mysql_client_server_protocol.h index 6534a81c4..ec52c86fd 100644 --- a/server/modules/include/mysql_client_server_protocol.h +++ b/server/modules/include/mysql_client_server_protocol.h @@ -74,7 +74,7 @@ #define GW_MYSQL_SERVER_CAPABILITIES_BYTE1 0xff #define GW_MYSQL_SERVER_CAPABILITIES_BYTE2 0xf7 #define GW_MYSQL_SERVER_LANGUAGE 0x08 -#define GW_MYSQL_MAX_PACKET_LEN 0xffffffL; +#define GW_MYSQL_MAX_PACKET_LEN 0xffffffL #define GW_MYSQL_SCRAMBLE_SIZE 20 #define GW_SCRAMBLE_LENGTH_323 8 diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 0146e819e..6030d94db 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -766,7 +766,12 @@ static bool process_client_commands(DCB* dcb, int bytes_available, GWBUF** buffe } MySQLProtocol *proto = (MySQLProtocol*)dcb->protocol; - proto->current_command = cmd; + if (dcb->protocol_packet_length - MYSQL_HEADER_LEN != GW_MYSQL_MAX_PACKET_LEN) + { + /** We're processing the first packet of a command */ + proto->current_command = cmd; + } + dcb->protocol_packet_length = pktlen + MYSQL_HEADER_LEN; dcb->protocol_bytes_processed = 0; } From 97115fae6aa32f81473cf35da1e0ffb9099957e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 2 Mar 2017 12:07:51 +0200 Subject: [PATCH 2/6] Fix `threads` documentation The description of the `auto` value was wrong. The algorithm is: CPU count - 1 --- Documentation/Getting-Started/Configuration-Guide.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/Getting-Started/Configuration-Guide.md b/Documentation/Getting-Started/Configuration-Guide.md index bd593ea5a..24abe119d 100644 --- a/Documentation/Getting-Started/Configuration-Guide.md +++ b/Documentation/Getting-Started/Configuration-Guide.md @@ -77,9 +77,9 @@ processor cores does not improve the performance, rather is likely to degrade it, and can consume resources needlessly. You can enable automatic configuration of this value by setting the value to -`auto`. This way MariaDB MaxScale will detect the number of available processors and -set the amount of threads to be equal to that number. This should only be used -for systems dedicated for running MariaDB MaxScale. +`auto`. This way MariaDB MaxScale will detect the number of available processors +and set the amount of threads to be equal to that number minus one. This should +only be used for systems dedicated for running MariaDB MaxScale. ``` # Valid options are: From e4b4dad94ae585604301b8f0a4fb9262e4356fa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 2 Mar 2017 13:11:52 +0200 Subject: [PATCH 3/6] Add more debug assertions to avro row processing The debug assertions check that the event pointer isn't moving beyond allocated memory. --- server/modules/routing/avro/avro_rbr.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/server/modules/routing/avro/avro_rbr.c b/server/modules/routing/avro/avro_rbr.c index 148c6932a..b8149a20b 100644 --- a/server/modules/routing/avro/avro_rbr.c +++ b/server/modules/routing/avro/avro_rbr.c @@ -29,7 +29,7 @@ static bool warn_large_enumset = false; /**< Remove when support for ENUM/SET va uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value_t *record, uint8_t *ptr, - uint8_t *columns_present); + uint8_t *columns_present, uint8_t *end); void notify_all_clients(AVRO_INSTANCE *router); void add_used_table(AVRO_INSTANCE* router, const char* table); @@ -292,9 +292,10 @@ bool handle_row_event(AVRO_INSTANCE *router, REP_HEADER *hdr, uint8_t *ptr) while (ptr - start < hdr->event_size - BINLOG_EVENT_HDR_LEN) { /** Add the current GTID and timestamp */ + uint8_t *end = ptr + hdr->event_size; 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); + ptr = process_row_event_data(map, create, &record, ptr, col_present, end); avro_file_writer_append_value(table->avro_file, &record); /** Update rows events have the before and after images of the @@ -303,7 +304,7 @@ bool handle_row_event(AVRO_INSTANCE *router, REP_HEADER *hdr, uint8_t *ptr) if (event_type == UPDATE_EVENT) { prepare_record(router, hdr, UPDATE_EVENT_AFTER, &record); - ptr = process_row_event_data(map, create, &record, ptr, col_present); + ptr = process_row_event_data(map, create, &record, ptr, col_present, end); avro_file_writer_append_value(table->avro_file, &record); } @@ -480,7 +481,7 @@ int get_metadata_len(uint8_t type) * @return Pointer to the first byte after the current row event */ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value_t *record, - uint8_t *ptr, uint8_t *columns_present) + uint8_t *ptr, uint8_t *columns_present, uint8_t *end) { int npresent = 0; avro_value_t field; @@ -490,10 +491,12 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value /** BIT type values use the extra bits in the row event header */ int extra_bits = (((ncolumns + 7) / 8) * 8) - ncolumns; + ss_dassert(ptr < end); /** Store the null value bitmap */ uint8_t *null_bitmap = ptr; ptr += (ncolumns + 7) / 8; + ss_dassert(ptr < end); for (long i = 0; i < map->columns && npresent < ncolumns; i++) { @@ -527,6 +530,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value } avro_value_set_string(&field, strval); ptr += bytes; + ss_dassert(ptr < end); } else { @@ -536,6 +540,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 + 1; + ss_dassert(ptr < end); } } else if (column_is_bit(map->column_types[i])) @@ -555,12 +560,14 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value } avro_value_set_int(&field, value); ptr += bytes; + ss_dassert(ptr < end); } else if (column_is_decimal(map->column_types[i])) { double f_value = 0.0; ptr += unpack_decimal_field(ptr, metadata + metadata_offset, &f_value); avro_value_set_double(&field, f_value); + ss_dassert(ptr < end); } else if (column_is_variable_string(map->column_types[i])) { @@ -570,6 +577,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value memcpy(buf, str, sz); buf[sz] = '\0'; avro_value_set_string(&field, buf); + ss_dassert(ptr < end); } else if (column_is_blob(map->column_types[i])) { @@ -579,6 +587,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value ptr += bytes; avro_value_set_bytes(&field, ptr, len); ptr += len; + ss_dassert(ptr < end); } else if (column_is_temporal(map->column_types[i])) { @@ -587,6 +596,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value ptr += unpack_temporal_value(map->column_types[i], ptr, &metadata[metadata_offset], &tm); format_temporal_value(buf, sizeof(buf), map->column_types[i], &tm); avro_value_set_string(&field, buf); + ss_dassert(ptr < end); } /** All numeric types (INT, LONG, FLOAT etc.) */ else @@ -596,6 +606,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value ptr += unpack_numeric_field(ptr, map->column_types[i], &metadata[metadata_offset], lval); set_numeric_field_value(&field, map->column_types[i], &metadata[metadata_offset], lval); + ss_dassert(ptr < end); } ss_dassert(metadata_offset <= map->column_metadata_size); metadata_offset += get_metadata_len(map->column_types[i]); From e01a6a0d588b97c66a64630ddc2fe33f9b052543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 2 Mar 2017 13:12:45 +0200 Subject: [PATCH 4/6] Fix field name parsing The fix to field name parsing didn't properly break the loop when the backtick character was detected. --- server/modules/routing/avro/avro_schema.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/server/modules/routing/avro/avro_schema.c b/server/modules/routing/avro/avro_schema.c index dd75e9d7d..604b18aea 100644 --- a/server/modules/routing/avro/avro_schema.c +++ b/server/modules/routing/avro/avro_schema.c @@ -451,6 +451,10 @@ static const char *extract_field_name(const char* ptr, char* dest, size_t size) while (*ptr && (isspace(*ptr) || (bt = *ptr == '`'))) { ptr++; + if (bt) + { + break; + } } if (strncasecmp(ptr, "constraint", 10) == 0 || strncasecmp(ptr, "index", 5) == 0 || @@ -483,11 +487,6 @@ static const char *extract_field_name(const char* ptr, char* dest, size_t size) /** Valid identifier */ size_t bytes = ptr - start; - if (bt) - { - bytes--; - } - memcpy(dest, start, bytes); dest[bytes] = '\0'; From e2869052bd3477e843ba59382fff0f091a806c9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 2 Mar 2017 13:14:07 +0200 Subject: [PATCH 5/6] MXS-1081: Fix VARCHAR field processing The data length is stored in the field metadata instead of the data being encoded as a length encoded string. --- server/modules/routing/avro/avro_rbr.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/server/modules/routing/avro/avro_rbr.c b/server/modules/routing/avro/avro_rbr.c index b8149a20b..3ac3cf8d5 100644 --- a/server/modules/routing/avro/avro_rbr.c +++ b/server/modules/routing/avro/avro_rbr.c @@ -572,10 +572,22 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value else if (column_is_variable_string(map->column_types[i])) { size_t sz; - char *str = lestr_consume(&ptr, &sz); + int bytes = metadata[metadata_offset] | metadata[metadata_offset + 1] << 8; + if (bytes > 255) + { + sz = gw_mysql_get_byte2(ptr); + ptr += 2; + } + else + { + sz = *ptr; + ptr++; + } + char buf[sz + 1]; - memcpy(buf, str, sz); + memcpy(buf, ptr, sz); buf[sz] = '\0'; + ptr += sz; avro_value_set_string(&field, buf); ss_dassert(ptr < end); } From 075ca42482d6f8265dab03999d49ad6b2bfdde05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 2 Mar 2017 13:33:45 +0200 Subject: [PATCH 6/6] Expand `slave_selection_criteria` documentation Added a section about how `slave_selection_criteria` and `max_slave_connections` affect each other. --- Documentation/Routers/ReadWriteSplit.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Documentation/Routers/ReadWriteSplit.md b/Documentation/Routers/ReadWriteSplit.md index f76dbf30e..c3a33d827 100644 --- a/Documentation/Routers/ReadWriteSplit.md +++ b/Documentation/Routers/ReadWriteSplit.md @@ -81,6 +81,25 @@ The `LEAST_GLOBAL_CONNECTIONS` and `LEAST_ROUTER_CONNECTIONS` use the connection `LEAST_BEHIND_MASTER` does not take server weights into account when choosing a server. +#### Interaction Between `slave_selection_criteria` and `max_slave_connections` + +Depending on the value of `max_slave_connections`, the slave selection criteria +behave in different ways. Here are a few example cases of how the different +criteria work with different amounts of slave connections. + +* With `slave_selection_criteria=LEAST_GLOBAL_CONNECTIONS` and `max_slave_connections=1`, each session picks + one slave and one master + +* With `slave_selection_criteria=LEAST_CURRENT_OPERATIONS` and `max_slave_connections=100%`, each session + picks one master and as many slaves as possible + +* With `slave_selection_criteria=LEAST_CURRENT_OPERATIONS` each read is load balanced based on how many + queries are active on a particular slave + +* With `slave_selection_criteria=LEAST_GLOBAL_CONNECTIONS` each read is sent to the slave with the least + amount of connections + + ### `max_sescmd_history` **`max_sescmd_history`** sets a limit on how many session commands each session can execute before the session command history is disabled. The default is an unlimited number of session commands.