diff --git a/Documentation/Filters/Cache.md b/Documentation/Filters/Cache.md index 53372dcfc..8679f58c3 100644 --- a/Documentation/Filters/Cache.md +++ b/Documentation/Filters/Cache.md @@ -173,8 +173,11 @@ where, * the _op_ can be `=`, `!=`, `like` or `unlike`, and * the _value_ a string. -If _op_ is `=` or `!=` then _value_ is used verbatim; if it is `like` +If _op_ is `=` or `!=` then _value_ is used as a string; if it is `like` or `unlike`, then _value_ is interpreted as a _pcre2_ regular expression. +Note though that if _attribute_ is `database`, `table` or `column`, then +the string is interpreted as a name, where a dot `.` denotes qualification +or scoping. The objects in the `store` array are processed in order. If the result of a comparison is _true_, no further processing will be made and the @@ -206,6 +209,39 @@ select * from tbl where b = 3 and a = 2; as well. Although they conceptually are identical, there will be two cache entries. +### Qualified Names + +When using `=` or `!=` in the rule object in conjunction with `database`, +`table` and `column`, the provided string is interpreted as a name, that is, +dot (`.`) denotes qualification or scope. + +In practice that means that if _attribute_ is `database` then _value_ may +not contain a dot, if _attribute_ is `table` then _value_ may contain one +dot, used for separating the database and table names respectively, and +if _attribute_ is `column` then _value_ may contain one or two dots, used +for separating table and column names, or database, table and column names. + +Note that if a qualified name is used as a _value_, then all parts of the +name must be available for a match. Currently Maria DB MaxScale may not +always be capable of deducing in what table a particular column is. If +that is the case, then a value like `tbl.field` may not necessarily +be a match even if the field is `field` and the table actually is `tbl`. + +### Implication of the _default_ database. + +If the rules concerns the `database`, then only if the statement refers +to *no* specific database, will the default database be considered. + +### Regexp Matching + +The string used for matching the regular expression contains as much +information as there is available. For instance, in a situation like +``` +use somedb; +select fld from tbl; +``` +the string matched against the regular expression will be `somedb.tbl.fld`. + ### Examples Cache all queries targeting a particular database. diff --git a/include/maxscale/monitor.h b/include/maxscale/monitor.h index 1db55f8c3..46a9df814 100644 --- a/include/maxscale/monitor.h +++ b/include/maxscale/monitor.h @@ -233,6 +233,15 @@ connect_result_t mon_connect_to_db(MONITOR* mon, MONITOR_SERVERS *database); void mon_log_connect_error(MONITOR_SERVERS* database, connect_result_t rval); void mon_log_state_change(MONITOR_SERVERS *ptr); +/** + * @brief Hangup connections to failed servers + * + * Injects hangup events for DCB that are connected to servers that are down. + * + * @param monitor Monitor object + */ +void mon_hangup_failed_servers(MONITOR *monitor); + /** * @brief Serialize a monitor to a file * diff --git a/include/maxscale/server.h b/include/maxscale/server.h index 51fe48bea..cfde36a10 100644 --- a/include/maxscale/server.h +++ b/include/maxscale/server.h @@ -115,6 +115,7 @@ typedef struct server long persistpoolmax; /**< Maximum size of persistent connections pool */ long persistmaxtime; /**< Maximum number of seconds connection can live */ int persistmax; /**< Maximum pool size actually achieved since startup */ + uint8_t charset; /**< Default server character set */ bool is_active; /**< Server is active and has not been "destroyed" */ #if defined(SS_DEBUG) skygw_chk_t server_chk_tail; diff --git a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc index e13a469ea..f774c20eb 100644 --- a/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc +++ b/query_classifier/qc_mysqlembedded/qc_mysqlembedded.cc @@ -97,7 +97,7 @@ static parsing_info_t* parsing_info_init(void (*donefun)(void *)); static void parsing_info_set_plain_str(void* ptr, char* str); /** Free THD context and close MYSQL */ static void parsing_info_done(void* ptr); -static void* skygw_get_affected_tables(void* lexptr); +static TABLE_LIST* skygw_get_affected_tables(void* lexptr); static bool ensure_query_is_parsed(GWBUF* query); static bool parse_query(GWBUF* querybuf); static bool query_is_parsed(GWBUF* buf); @@ -1090,7 +1090,7 @@ LEX* get_lex(GWBUF* querybuf) * @param thd Pointer to a valid THD * @return Pointer to the head of the TABLE_LIST chain or NULL in case of an error */ -static void* skygw_get_affected_tables(void* lexptr) +static TABLE_LIST* skygw_get_affected_tables(void* lexptr) { LEX* lex = (LEX*) lexptr; @@ -1100,7 +1100,23 @@ static void* skygw_get_affected_tables(void* lexptr) return NULL; } - return (void*) lex->current_select->table_list.first; + TABLE_LIST *tbl = lex->current_select->table_list.first; + + if (tbl && tbl->schema_select_lex && tbl->schema_select_lex->table_list.elements && + lex->sql_command != SQLCOM_SHOW_KEYS) + { + /** + * Some statements e.g. EXPLAIN or SHOW COLUMNS give `information_schema` + * as the underlying table and the table in the query is stored in + * @c schema_select_lex. + * + * SHOW [KEYS | INDEX] does the reverse so we need to skip the + * @c schema_select_lex when processing a SHOW [KEYS | INDEX] statement. + */ + tbl = tbl->schema_select_lex->table_list.first; + } + + return tbl; } char** qc_get_table_names(GWBUF* querybuf, int* tblsize, bool fullnames) @@ -1129,7 +1145,7 @@ char** qc_get_table_names(GWBUF* querybuf, int* tblsize, bool fullnames) while (lex->current_select) { - tbl = (TABLE_LIST*) skygw_get_affected_tables(lex); + tbl = skygw_get_affected_tables(lex); while (tbl) { diff --git a/query_classifier/qc_sqlite/qc_sqlite.c b/query_classifier/qc_sqlite/qc_sqlite.c index 9ff54994e..6fcb79074 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.c +++ b/query_classifier/qc_sqlite/qc_sqlite.c @@ -2417,7 +2417,7 @@ extern void maxscaleShow(Parse* pParse, MxsShow* pShow) case MXS_SHOW_COLUMNS: { info->types = QUERY_TYPE_READ; - update_names(info, "information_schema", "COLUMNS"); + update_names(info, zDatabase, zName); if (pShow->data == MXS_SHOW_COLUMNS_FULL) { update_field_info(info, "information_schema", "COLUMNS", "COLLATION_NAME", u, NULL); diff --git a/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y b/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y index ab6b50ea7..5f5769e4a 100644 --- a/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y +++ b/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y @@ -3022,13 +3022,17 @@ like_or_where_opt ::= WHERE expr. %type show {MxsShow} -show(A) ::= SHOW full_opt(X) COLUMNS from_or_in nm(Y) dbnm(Z) from_or_in_db_opt like_or_where_opt . { +show(A) ::= SHOW full_opt(X) COLUMNS from_or_in nm(Y) dbnm(Z) from_or_in_db_opt(W) like_or_where_opt . { A.what = MXS_SHOW_COLUMNS; A.data = X; if (Z.z) { A.pName = &Z; A.pDatabase = &Y; } + else if (W.z) { + A.pName = &Y; + A.pDatabase = &W; + } else { A.pName = &Y; A.pDatabase = NULL; diff --git a/server/core/config.c b/server/core/config.c index ea812487f..f2aec6ab3 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -1731,7 +1731,7 @@ process_config_update(CONFIG_CONTEXT *context) if (connection_timeout) { - serviceSetTimeout(service, config_truth_value(connection_timeout)); + serviceSetTimeout(service, atoi(connection_timeout)); } if (strlen(max_connections)) diff --git a/server/core/dcb.c b/server/core/dcb.c index c07bd979b..288fad614 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1678,6 +1678,38 @@ dcb_grab_writeq(DCB *dcb, bool first_time) return local_writeq; } +static void log_illegal_dcb(DCB *dcb) +{ + const char *connected_to; + + switch (dcb->dcb_role) + { + case DCB_ROLE_BACKEND_HANDLER: + connected_to = dcb->server->unique_name; + break; + + case DCB_ROLE_CLIENT_HANDLER: + connected_to = dcb->remote; + break; + + case DCB_ROLE_INTERNAL: + connected_to = "Internal DCB"; + break; + + case DCB_ROLE_SERVICE_LISTENER: + connected_to = dcb->service->name; + break; + + default: + connected_to = "Illegal DCB role"; + break; + } + + MXS_ERROR("[dcb_close] Error : Removing DCB %p but it is in state %s " + "which is not legal for a call to dcb_close. The DCB is connected to: %s", + dcb, STRDCBSTATE(dcb->state), connected_to); +} + /** * Removes dcb from poll set, and adds it to zombies list. As a consequence, * dcb first moves to DCB_STATE_NOPOLLING, and then to DCB_STATE_ZOMBIE state. @@ -1697,11 +1729,7 @@ dcb_close(DCB *dcb) if (DCB_STATE_UNDEFINED == dcb->state || DCB_STATE_DISCONNECTED == dcb->state) { - MXS_ERROR("%lu [dcb_close] Error : Removing DCB %p but was in state %s " - "which is not legal for a call to dcb_close. ", - pthread_self(), - dcb, - STRDCBSTATE(dcb->state)); + log_illegal_dcb(dcb); raise(SIGABRT); } diff --git a/server/core/monitor.c b/server/core/monitor.c index 39c2c14f1..4708a4c9d 100644 --- a/server/core/monitor.c +++ b/server/core/monitor.c @@ -349,7 +349,7 @@ void monitorRemoveServer(MONITOR *mon, SERVER *server) MONITOR_SERVERS *ptr = mon->databases; - if (ptr->server == server) + if (ptr && ptr->server == server) { mon->databases = mon->databases->next; } @@ -800,8 +800,14 @@ mon_get_event_type(MONITOR_SERVERS* node) } else { + /** These are used to detect whether we actually lost something or + * just transitioned from one state to another */ + unsigned int prev_bits = prev & (SERVER_MASTER | SERVER_SLAVE); + unsigned int present_bits = present & (SERVER_MASTER | SERVER_SLAVE); + /* Was running and still is */ - if (prev & (SERVER_MASTER | SERVER_SLAVE | SERVER_JOINED | SERVER_NDB)) + if ((!prev_bits || !present_bits || prev_bits == present_bits) && + prev & (SERVER_MASTER | SERVER_SLAVE | SERVER_JOINED | SERVER_NDB)) { /* We used to know what kind of server it was */ event_type = LOSS_EVENT; @@ -1252,3 +1258,16 @@ bool monitor_serialize_servers(const MONITOR *monitor) return rval; } + +void mon_hangup_failed_servers(MONITOR *monitor) +{ + for (MONITOR_SERVERS *ptr = monitor->databases; ptr; ptr = ptr->next) + { + if (mon_status_changed(ptr) && + (!(SERVER_IS_RUNNING(ptr->server)) || + !(SERVER_IS_IN_CLUSTER(ptr->server)))) + { + dcb_hangup_foreach(ptr->server); + } + } +} diff --git a/server/core/mysql_utils.c b/server/core/mysql_utils.c index 1ba15bf67..93d99b3c3 100644 --- a/server/core/mysql_utils.c +++ b/server/core/mysql_utils.c @@ -163,8 +163,6 @@ MYSQL *mxs_mysql_real_connect(MYSQL *con, SERVER *server, const char *user, cons if (listener) { - GATEWAY_CONF* config = config_get_global_options(); - // mysql_ssl_set always returns true. mysql_ssl_set(con, listener->ssl_key, listener->ssl_cert, listener->ssl_ca_cert, NULL, NULL); } diff --git a/server/core/server.c b/server/core/server.c index 729f8916a..ef45fcb23 100644 --- a/server/core/server.c +++ b/server/core/server.c @@ -51,6 +51,9 @@ #include #include +/** The latin1 charset */ +#define SERVER_DEFAULT_CHARSET 0x08 + static SPINLOCK server_spin = SPINLOCK_INIT; static SERVER *allServers = NULL; @@ -129,6 +132,7 @@ SERVER* server_alloc(const char *name, const char *address, unsigned short port, server->monuser[0] = '\0'; server->monpw[0] = '\0'; server->is_active = true; + server->charset = SERVER_DEFAULT_CHARSET; spinlock_init(&server->persistlock); spinlock_acquire(&server_spin); diff --git a/server/modules/authenticator/MySQLAuth/dbusers.c b/server/modules/authenticator/MySQLAuth/dbusers.c index d08a0d40a..42ecf3cb3 100644 --- a/server/modules/authenticator/MySQLAuth/dbusers.c +++ b/server/modules/authenticator/MySQLAuth/dbusers.c @@ -2532,6 +2532,11 @@ static bool check_server_permissions(SERVICE *service, SERVER* server, return my_errno != ER_ACCESS_DENIED_ERROR; } + /** Copy the server charset */ + MY_CHARSET_INFO cs_info; + mysql_get_character_set_info(mysql, &cs_info); + server->charset = cs_info.number; + if (server->server_string == NULL) { const char *server_string = mysql_get_server_info(mysql); diff --git a/server/modules/filter/cache/rules.c b/server/modules/filter/cache/rules.c index 7dd5bbdf0..c4917aaa5 100644 --- a/server/modules/filter/cache/rules.c +++ b/server/modules/filter/cache/rules.c @@ -76,10 +76,28 @@ static CACHE_RULE *cache_rule_create_simple(cache_rule_attribute_t attribute, cache_rule_op_t op, const char *value, uint32_t debug); +static CACHE_RULE *cache_rule_create_simple_ctd(cache_rule_attribute_t attribute, + cache_rule_op_t op, + const char *cvalue, + uint32_t debug); +static CACHE_RULE *cache_rule_create_simple_user(cache_rule_attribute_t attribute, + cache_rule_op_t op, + const char *cvalue, + uint32_t debug); +static CACHE_RULE *cache_rule_create_simple_query(cache_rule_attribute_t attribute, + cache_rule_op_t op, + const char *cvalue, + uint32_t debug); static CACHE_RULE *cache_rule_create(cache_rule_attribute_t attribute, cache_rule_op_t op, const char *value, uint32_t debug); +static bool cache_rule_matches_column_regexp(CACHE_RULE *rule, + const char *default_db, + const GWBUF *query); +static bool cache_rule_matches_column_simple(CACHE_RULE *rule, + const char *default_db, + const GWBUF *query); static bool cache_rule_matches_column(CACHE_RULE *rule, const char *default_db, const GWBUF *query); @@ -92,6 +110,12 @@ static bool cache_rule_matches_query(CACHE_RULE *rule, static bool cache_rule_matches_table(CACHE_RULE *rule, const char *default_db, const GWBUF *query); +static bool cache_rule_matches_table_regexp(CACHE_RULE *rule, + const char *default_db, + const GWBUF *query); +static bool cache_rule_matches_table_simple(CACHE_RULE *rule, + const char *default_db, + const GWBUF *query); static bool cache_rule_matches_user(CACHE_RULE *rule, const char *user); static bool cache_rule_matches(CACHE_RULE *rule, const char *default_db, @@ -366,7 +390,6 @@ bool cache_rules_should_use(CACHE_RULES *self, const SESSION *session) while (rule && !should_use) { should_use = cache_rule_matches_user(rule, account); - rule = rule->next; } } @@ -515,11 +538,22 @@ static CACHE_RULE *cache_rule_create_regexp(cache_rule_attribute_t attribute, return rule; } -static CACHE_RULE *cache_rule_create_user(cache_rule_attribute_t attribute, - cache_rule_op_t op, - const char *cvalue, - uint32_t debug) +/** + * Creates a CACHE_RULE object matching users. + * + * @param attribute CACHE_ATTRIBUTE_USER + * @param op An operator, CACHE_OP_EQ or CACHE_OP_NEQ. + * @param cvalue A string in the MySQL user format. + * @param debug The debug level. + * + * @return A new rule object or NULL in case of failure. + */ +static CACHE_RULE *cache_rule_create_simple_user(cache_rule_attribute_t attribute, + cache_rule_op_t op, + const char *cvalue, + uint32_t debug) { + ss_dassert(attribute == CACHE_ATTRIBUTE_USER); ss_dassert((op == CACHE_OP_EQ) || (op == CACHE_OP_NEQ)); CACHE_RULE *rule = NULL; @@ -608,6 +642,211 @@ static CACHE_RULE *cache_rule_create_user(cache_rule_attribute_t attribute, return rule; } +/** + * Creates a CACHE_RULE object matching column/table/database. + * + * @param attribute CACHE_ATTRIBUTE_[COLUMN|TABLE|DATABASE] + * @param op An operator, CACHE_OP_EQ or CACHE_OP_NEQ. + * @param cvalue A name, with 0, 1 or 2 dots. + * @param debug The debug level. + * + * @return A new rule object or NULL in case of failure. + */ +static CACHE_RULE *cache_rule_create_simple_ctd(cache_rule_attribute_t attribute, + cache_rule_op_t op, + const char *cvalue, + uint32_t debug) +{ + ss_dassert((attribute == CACHE_ATTRIBUTE_COLUMN) || + (attribute == CACHE_ATTRIBUTE_TABLE) || + (attribute == CACHE_ATTRIBUTE_DATABASE)); + ss_dassert((op == CACHE_OP_EQ) || (op == CACHE_OP_NEQ)); + + CACHE_RULE *rule = (CACHE_RULE*)MXS_CALLOC(1, sizeof(CACHE_RULE)); + char *value = MXS_STRDUP(cvalue); + + if (rule && value) + { + rule->attribute = attribute; + rule->op = op; + rule->value = value; + rule->debug = debug; + + bool allocation_failed = false; + + char buffer[strlen(value) + 1]; + strcpy(buffer, value); + + const char* first = NULL; + const char* second = NULL; + const char* third = NULL; + char* dot1 = strchr(buffer, '.'); + char* dot2 = dot1 ? strchr(dot1 + 1, '.') : NULL; + + if (dot1 && dot2) + { + first = buffer; + *dot1 = 0; + second = dot1 + 1; + *dot2 = 0; + third = dot2 + 1; + } + else if (dot1) + { + first = buffer; + *dot1 = 0; + second = dot1 + 1; + } + else + { + first = buffer; + } + + switch (attribute) + { + case CACHE_ATTRIBUTE_COLUMN: + { + if (third) // implies also 'first' and 'second' + { + rule->simple.column = MXS_STRDUP(third); + rule->simple.table = MXS_STRDUP(second); + rule->simple.database = MXS_STRDUP(first); + + if (!rule->simple.column || !rule->simple.table || !rule->simple.database) + { + allocation_failed = true; + } + } + else if (second) // implies also 'first' + { + rule->simple.column = MXS_STRDUP(second); + rule->simple.table = MXS_STRDUP(first); + + if (!rule->simple.column || !rule->simple.table) + { + allocation_failed = true; + } + } + else // only 'first' + { + rule->simple.column = MXS_STRDUP(first); + + if (!rule->simple.column) + { + allocation_failed = true; + } + } + } + break; + + case CACHE_ATTRIBUTE_TABLE: + if (third) + { + MXS_ERROR("A cache rule value for matching a table name, cannot contain two dots: '%s'", + cvalue); + allocation_failed = true; + } + else + { + if (second) // implies also 'first' + { + rule->simple.database = MXS_STRDUP(first); + rule->simple.table = MXS_STRDUP(second); + if (!rule->simple.database || !rule->simple.table) + { + allocation_failed = true; + } + } + else // only 'first' + { + rule->simple.table = MXS_STRDUP(first); + if (!rule->simple.table) + { + allocation_failed = true; + } + } + } + break; + + case CACHE_ATTRIBUTE_DATABASE: + if (second) + { + MXS_ERROR("A cache rule value for matching a database, cannot contain a dot: '%s'", + cvalue); + allocation_failed = true; + } + else + { + rule->simple.database = MXS_STRDUP(first); + if (!rule->simple.database) + { + allocation_failed = true; + } + } + break; + + default: + ss_dassert(!true); + } + + if (allocation_failed) + { + MXS_FREE(rule->simple.column); + MXS_FREE(rule->simple.table); + MXS_FREE(rule->simple.database); + MXS_FREE(value); + MXS_FREE(rule); + rule = NULL; + } + } + else + { + MXS_FREE(value); + MXS_FREE(rule); + rule = NULL; + } + + return rule; +} + +/** + * Creates a CACHE_RULE object matching an entire query. + * + * @param attribute CACHE_ATTRIBUTE_QUERY. + * @param op An operator, CACHE_OP_EQ or CACHE_OP_NEQ. + * @param cvalue A string. + * @param debug The debug level. + * + * @return A new rule object or NULL in case of failure. + */ +static CACHE_RULE *cache_rule_create_simple_query(cache_rule_attribute_t attribute, + cache_rule_op_t op, + const char *cvalue, + uint32_t debug) +{ + ss_dassert(attribute == CACHE_ATTRIBUTE_QUERY); + ss_dassert((op == CACHE_OP_EQ) || (op == CACHE_OP_NEQ)); + + CACHE_RULE *rule = MXS_CALLOC(1, sizeof(CACHE_RULE)); + char *value = MXS_STRDUP(cvalue); + + if (rule && value) + { + rule->attribute = attribute; + rule->op = op; + rule->debug = debug; + rule->value = value; + } + else + { + MXS_FREE(value); + MXS_FREE(rule); + rule = NULL; + } + + return rule; +} + /** * Creates a CACHE_RULE object doing simple matching. * @@ -627,28 +866,25 @@ static CACHE_RULE *cache_rule_create_simple(cache_rule_attribute_t attribute, CACHE_RULE *rule; - if (attribute == CACHE_ATTRIBUTE_USER) + switch (attribute) { - rule = cache_rule_create_user(attribute, op, cvalue, debug); - } - else - { - rule = (CACHE_RULE*)MXS_CALLOC(1, sizeof(CACHE_RULE)); - char *value = MXS_STRDUP(cvalue); + case CACHE_ATTRIBUTE_COLUMN: + case CACHE_ATTRIBUTE_TABLE: + case CACHE_ATTRIBUTE_DATABASE: + rule = cache_rule_create_simple_ctd(attribute, op, cvalue, debug); + break; - if (rule && value) - { - rule->attribute = attribute; - rule->op = op; - rule->debug = debug; - rule->value = value; - } - else - { - MXS_FREE(rule); - MXS_FREE(value); - rule = NULL; - } + case CACHE_ATTRIBUTE_USER: + rule = cache_rule_create_simple_user(attribute, op, cvalue, debug); + break; + + case CACHE_ATTRIBUTE_QUERY: + rule = cache_rule_create_simple_query(attribute, op, cvalue, debug); + break; + + default: + MXS_ERROR("Unknown attribute type: %d", (int)attribute); + ss_dassert(!true); } return rule; @@ -708,7 +944,13 @@ static void cache_rule_free(CACHE_RULE* rule) MXS_FREE(rule->value); - if ((rule->op == CACHE_OP_LIKE) || (rule->op == CACHE_OP_UNLIKE)) + if ((rule->op == CACHE_OP_EQ) || (rule->op == CACHE_OP_NEQ)) + { + MXS_FREE(rule->simple.column); + MXS_FREE(rule->simple.table); + MXS_FREE(rule->simple.database); + } + else if ((rule->op == CACHE_OP_LIKE) || (rule->op == CACHE_OP_UNLIKE)) { pcre2_match_data_free(rule->regexp.data); pcre2_code_free(rule->regexp.code); @@ -728,7 +970,25 @@ static void cache_rule_free(CACHE_RULE* rule) */ static bool cache_rule_compare(CACHE_RULE *self, const char *value) { - return cache_rule_compare_n(self, value, strlen(value)); + bool rv; + + if (value) + { + rv = cache_rule_compare_n(self, value, strlen(value)); + } + else + { + if ((self->op == CACHE_OP_EQ) || (self->op == CACHE_OP_LIKE)) + { + rv = false; + } + else + { + rv = true; + } + } + + return rv; } /** @@ -779,44 +1039,35 @@ static bool cache_rule_compare_n(CACHE_RULE *self, const char *value, size_t len * * @return True, if the rule matches, false otherwise. */ -static bool cache_rule_matches_column(CACHE_RULE *self, const char *default_db, const GWBUF *query) +static bool cache_rule_matches_column_regexp(CACHE_RULE *self, const char *default_db, const GWBUF *query) { ss_dassert(self->attribute == CACHE_ATTRIBUTE_COLUMN); + ss_dassert((self->op == CACHE_OP_LIKE) || (self->op == CACHE_OP_UNLIKE)); - // TODO: Do this "parsing" when the rule item is created. - char buffer[strlen(self->value) + 1]; - strcpy(buffer, self->value); + const char* default_database = NULL; - const char* rule_column = NULL; - const char* rule_table = NULL; - const char* rule_database = NULL; - char* dot1 = strchr(buffer, '.'); - char* dot2 = dot1 ? strchr(buffer, '.') : NULL; + int n_databases; + char **databases = qc_get_database_names((GWBUF*)query, &n_databases); - if (dot1 && dot2) + if (n_databases == 0) { - rule_database = buffer; - *dot1 = 0; - rule_table = dot1 + 1; - *dot2 = 0; - rule_column = dot2 + 1; + // If no databases have been mentioned, then we can assume that all + // tables and columns that are not explcitly qualified refer to the + // default database. + default_database = default_db; } - else if (dot1) + else if ((default_db == NULL) && (n_databases == 1)) { - rule_table = buffer; - *dot1 = 0; - rule_column = dot1 + 1; - } - else - { - rule_column = buffer; + // If there is no default database and exactly one database has been + // explicitly mentioned, then we can assume all tables and columns that + // are not explicitly qualified refer to that database. + default_database = databases[0]; } - const QC_FIELD_INFO *infos; - size_t n_infos; + size_t default_database_len = default_database ? strlen(default_database) : 0; int n_tables; - char** tables = qc_get_table_names((GWBUF*)query, &n_tables, false); + char **tables = qc_get_table_names((GWBUF*)query, &n_tables, false); const char* default_table = NULL; @@ -827,6 +1078,11 @@ static bool cache_rule_matches_column(CACHE_RULE *self, const char *default_db, default_table = tables[0]; } + size_t default_table_len = default_table ? strlen(default_table) : 0; + + const QC_FIELD_INFO *infos; + size_t n_infos; + qc_get_field_info((GWBUF*)query, &infos, &n_infos); bool matches = false; @@ -836,49 +1092,214 @@ static bool cache_rule_matches_column(CACHE_RULE *self, const char *default_db, { const QC_FIELD_INFO *info = (infos + i); - if ((strcmp(info->column, rule_column) == 0) || (strcmp(info->column, "*") == 0)) + if (info->usage & QC_USED_IN_SELECT) { - if (rule_table) + size_t database_len; + const char *database; + + if (info->database) { - const char* check_table = info->table ? info->table : default_table; + database = info->database; + database_len = strlen(info->database); + } + else + { + database = default_database; + database_len = default_database_len; + } - if (check_table && (strcmp(check_table, rule_table) == 0)) + size_t table_len; + const char *table; + + if (info->table) + { + table = info->table; + table_len = strlen(info->table); + } + else + { + table = default_table; + table_len = default_table_len; + } + + char buffer[database_len + 1 + table_len + strlen(info->column) + 1]; + buffer[0] = 0; + + if (database) + { + strcat(buffer, database); + strcat(buffer, "."); + } + + if (table) + { + strcat(buffer, table); + strcat(buffer, "."); + } + + strcat(buffer, info->column); + + matches = cache_rule_compare(self, buffer); + } + + ++i; + } + + if (tables) + { + for (i = 0; i < (size_t)n_tables; ++i) + { + MXS_FREE(tables[i]); + } + MXS_FREE(tables); + } + + if (databases) + { + for (i = 0; i < (size_t)n_databases; ++i) + { + MXS_FREE(databases[i]); + } + MXS_FREE(databases); + } + + return matches; +} + +/** + * Returns boolean indicating whether the column rule matches the query or not. + * + * @param self The CACHE_RULE object. + * @param default_db The current default db. + * @param query The query. + * + * @return True, if the rule matches, false otherwise. + */ +static bool cache_rule_matches_column_simple(CACHE_RULE *self, const char *default_db, const GWBUF *query) +{ + ss_dassert(self->attribute == CACHE_ATTRIBUTE_COLUMN); + ss_dassert((self->op == CACHE_OP_EQ) || (self->op == CACHE_OP_NEQ)); + + const char* rule_column = self->simple.column; + const char* rule_table = self->simple.table; + const char* rule_database = self->simple.database; + + const char* default_database = NULL; + + int n_databases; + char **databases = qc_get_database_names((GWBUF*)query, &n_databases); + + if (n_databases == 0) + { + // If no databases have been mentioned, then we can assume that all + // tables and columns that are not explcitly qualified refer to the + // default database. + default_database = default_db; + } + else if ((default_db == NULL) && (n_databases == 1)) + { + // If there is no default database and exactly one database has been + // explicitly mentioned, then we can assume all tables and columns that + // are not explicitly qualified refer to that database. + default_database = databases[0]; + } + + int n_tables; + char **tables = qc_get_table_names((GWBUF*)query, &n_tables, false); + + const char* default_table = NULL; + + if (n_tables == 1) + { + // Only if we have exactly one table can we assume anything + // about a table that has not been mentioned explicitly. + default_table = tables[0]; + } + + const QC_FIELD_INFO *infos; + size_t n_infos; + + qc_get_field_info((GWBUF*)query, &infos, &n_infos); + + bool matches = false; + + size_t i = 0; + while (!matches && (i < n_infos)) + { + const QC_FIELD_INFO *info = (infos + i); + + if (info->usage & QC_USED_IN_SELECT) + { + if ((strcasecmp(info->column, rule_column) == 0) || strcmp(rule_column, "*") == 0) + { + if (rule_table) { - if (rule_database) - { - const char *check_database = info->database ? info->database : default_db; + const char* check_table = info->table ? info->table : default_table; - if (check_database && (strcmp(check_database, rule_database) == 0)) + if (check_table) + { + if (strcasecmp(check_table, rule_table) == 0) { - matches = true; + if (rule_database) + { + const char *check_database = + info->database ? info->database : default_database; + + if (check_database) + { + if (strcasecmp(check_database, rule_database) == 0) + { + // The column, table and database matched. + matches = true; + } + else + { + // The column, table matched but the database did not. + matches = false; + } + } + else + { + // If the rules specify a database but we do not know the database, + // we consider the databases not to match. + matches = false; + } + } + else + { + // If the rule specifies no database, then if the column and the table + // matches, the rule matches. + matches = true; + } } else { - // If the rules specifies a database and either the database - // does not match or we do not know the database, the rule - // does *not* match. + // The column matched, but the table did not. matches = false; } } else { - // If the rule specifies no table, then if the table and column matches, - // the rule matches. - matches = true; + // If the rules specify a table but we do not know the table, we + // consider the tables not to match. + matches = false; } } else { - // The rules specifies a table and either the table does not match - // or we do not know the table, the rule does *not* match. - matches = false; + // The column matched and there is no table rule. + matches = true; } } else { - // If the rule specifies no table, then if the column matches, the - // rule matches. - matches = true; + // The column did not match. + matches = false; + } + + if (self->op == CACHE_OP_NEQ) + { + matches = !matches; } } @@ -894,6 +1315,49 @@ static bool cache_rule_matches_column(CACHE_RULE *self, const char *default_db, MXS_FREE(tables); } + if (databases) + { + for (i = 0; i < (size_t)n_databases; ++i) + { + MXS_FREE(databases[i]); + } + MXS_FREE(databases); + } + + return matches; +} + +/** + * Returns boolean indicating whether the column rule matches the query or not. + * + * @param self The CACHE_RULE object. + * @param default_db The current default db. + * @param query The query. + * + * @return True, if the rule matches, false otherwise. + */ +static bool cache_rule_matches_column(CACHE_RULE *self, const char *default_db, const GWBUF *query) +{ + ss_dassert(self->attribute == CACHE_ATTRIBUTE_COLUMN); + + bool matches = false; + + switch (self->op) + { + case CACHE_OP_EQ: + case CACHE_OP_NEQ: + matches = cache_rule_matches_column_simple(self, default_db, query); + break; + + case CACHE_OP_LIKE: + case CACHE_OP_UNLIKE: + matches = cache_rule_matches_column_regexp(self, default_db, query); + break; + + default: + ss_dassert(!true); + } + return matches; } @@ -912,8 +1376,9 @@ static bool cache_rule_matches_database(CACHE_RULE *self, const char *default_db bool matches = false; + bool fullnames = true; int n; - char **names = qc_get_database_names((GWBUF*)query, &n); // TODO: Make qc const-correct. + char **names = qc_get_table_names((GWBUF*)query, &n, fullnames); // TODO: Make qc const-correct. if (names) { @@ -921,20 +1386,32 @@ static bool cache_rule_matches_database(CACHE_RULE *self, const char *default_db while (!matches && (i < n)) { - matches = cache_rule_compare(self, names[i]); + char *name = names[i]; + char *dot = strchr(name, '.'); + const char *database = NULL; + + if (dot) + { + *dot = 0; + database = name; + } + else + { + database = default_db; + } + + matches = cache_rule_compare(self, database); + + MXS_FREE(name); ++i; } - for (int i = 0; i < n; ++i) + while (i < n) { - MXS_FREE(names[i]); + MXS_FREE(names[i++]); } - MXS_FREE(names); - } - if (!matches && default_db) - { - matches = cache_rule_compare(self, default_db); + MXS_FREE(names); } return matches; @@ -962,6 +1439,174 @@ static bool cache_rule_matches_query(CACHE_RULE *self, const char *default_db, c return cache_rule_compare_n(self, sql, len); } +/** + * Returns boolean indicating whether the table regexp rule matches the query or not. + * + * @param self The CACHE_RULE object. + * @param default_db The current default db. + * @param query The query. + * + * @return True, if the rule matches, false otherwise. + */ +static bool cache_rule_matches_table_regexp(CACHE_RULE *self, const char *default_db, const GWBUF *query) +{ + ss_dassert(self->attribute == CACHE_ATTRIBUTE_TABLE); + ss_dassert((self->op == CACHE_OP_LIKE) || (self->op == CACHE_OP_UNLIKE)); + + bool matches = false; + + int n; + char **names; + bool fullnames; + + fullnames = true; + names = qc_get_table_names((GWBUF*)query, &n, fullnames); + + if (names) + { + size_t default_db_len = default_db ? strlen(default_db) : 0; + + int i = 0; + while (!matches && (i < n)) + { + char *name = names[i]; + char *dot = strchr(name, '.'); + + if (!dot) + { + // Only "tbl" + + if (default_db) + { + char buffer[default_db_len + 1 + strlen(name) + 1]; + + strcpy(name, default_db); + strcpy(name + default_db_len, "."); + strcpy(name + default_db_len + 1, name); + + matches = cache_rule_compare(self, name); + } + else + { + matches = cache_rule_compare(self, name); + } + + MXS_FREE(names[i]); + } + else + { + // A qualified name "db.tbl". + matches = cache_rule_compare(self, name); + } + + ++i; + } + + if (i < n) + { + MXS_FREE(names[i]); + ++i; + } + + MXS_FREE(names); + } + else if (self->op == CACHE_OP_UNLIKE) + { + matches = true; + } + + return matches; +} + +/** + * Returns boolean indicating whether the table simple rule matches the query or not. + * + * @param self The CACHE_RULE object. + * @param default_db The current default db. + * @param query The query. + * + * @return True, if the rule matches, false otherwise. + */ +static bool cache_rule_matches_table_simple(CACHE_RULE *self, const char *default_db, const GWBUF *query) +{ + ss_dassert(self->attribute == CACHE_ATTRIBUTE_TABLE); + ss_dassert((self->op == CACHE_OP_EQ) || (self->op == CACHE_OP_NEQ)); + + bool matches = false; + + bool fullnames = false; + + if (self->simple.database) + { + fullnames = true; + } + + int n; + char **names; + + names = qc_get_table_names((GWBUF*)query, &n, fullnames); + + if (names) + { + int i = 0; + while (!matches && (i < n)) + { + char *name = names[i]; + const char *database = NULL; + const char *table = NULL; + + if (fullnames) + { + char *dot = strchr(name, '.'); + + if (dot) + { + *dot = 0; + + database = name; + table = dot + 1; + } + else + { + database = default_db; + table = name; + } + + if (database) + { + matches = + (strcasecmp(self->simple.database, database) == 0) && + (strcasecmp(self->simple.table, table) == 0); + } + } + else + { + table = name; + + matches = (strcasecmp(self->simple.table, table) == 0); + } + + if (self->op == CACHE_OP_NEQ) + { + matches = !matches; + } + + MXS_FREE(name); + ++i; + } + + if (i < n) + { + MXS_FREE(names[i]); + ++i; + } + + MXS_FREE(names); + } + + return matches; +} + /** * Returns boolean indicating whether the table rule matches the query or not. * @@ -977,73 +1622,20 @@ static bool cache_rule_matches_table(CACHE_RULE *self, const char *default_db, c bool matches = false; - int n; - char **names; - bool fullnames; - - fullnames = false; - names = qc_get_table_names((GWBUF*)query, &n, fullnames); - - if (names) + switch (self->op) { - int i = 0; - while (!matches && (i < n)) - { - char *name = names[i]; - matches = cache_rule_compare(self, name); - MXS_FREE(name); - ++i; - } + case CACHE_OP_EQ: + case CACHE_OP_NEQ: + matches = cache_rule_matches_table_simple(self, default_db, query); + break; - if (i < n) - { - MXS_FREE(names[i]); - ++i; - } + case CACHE_OP_LIKE: + case CACHE_OP_UNLIKE: + matches = cache_rule_matches_table_regexp(self, default_db, query); + break; - MXS_FREE(names); - - if (!matches) - { - fullnames = true; - names = qc_get_table_names((GWBUF*)query, &n, fullnames); - - size_t default_db_len = default_db ? strlen(default_db) : 0; - i = 0; - - while (!matches && (i < n)) - { - char *name = names[i]; - char *dot = strchr(name, '.'); - - if (!dot) - { - if (default_db) - { - name = (char*)MXS_MALLOC(default_db_len + 1 + strlen(name) + 1); - - strcpy(name, default_db); - strcpy(name + default_db_len, "."); - strcpy(name + default_db_len + 1, names[i]); - - MXS_FREE(names[i]); - names[i] = name; - } - } - - matches = cache_rule_compare(self, name); - MXS_FREE(name); - ++i; - } - - if (i < n) - { - MXS_FREE(names[i]); - ++i; - } - - MXS_FREE(names); - } + default: + ss_dassert(!true); } return matches; @@ -1063,7 +1655,6 @@ static bool cache_rule_matches_user(CACHE_RULE *self, const char *account) bool matches = cache_rule_compare(self, account); - if ((matches && (self->debug & CACHE_DEBUG_MATCHING)) || (!matches && (self->debug & CACHE_DEBUG_NON_MATCHING))) { diff --git a/server/modules/filter/cache/rules.h b/server/modules/filter/cache/rules.h index 667ad344b..703c24d91 100644 --- a/server/modules/filter/cache/rules.h +++ b/server/modules/filter/cache/rules.h @@ -48,8 +48,14 @@ typedef struct cache_rule char *value; // The value from the rule file. struct { - pcre2_code* code; - pcre2_match_data* data; + char *database; + char *table; + char *column; + } simple; // Details, only for CACHE_OP_[EQ|NEQ] + struct + { + pcre2_code *code; + pcre2_match_data *data; } regexp; // Regexp data, only for CACHE_OP_[LIKE|UNLIKE]. uint32_t debug; // The debug level. struct cache_rule *next; diff --git a/server/modules/filter/cache/test/testrules.c b/server/modules/filter/cache/test/testrules.c index 153f342a5..601d0e2bc 100644 --- a/server/modules/filter/cache/test/testrules.c +++ b/server/modules/filter/cache/test/testrules.c @@ -132,8 +132,46 @@ struct store_test_case // false: The query should NOT match the rule. const struct store_test_case store_test_cases[] = { - STORE_TEST_CASE("column", "=", "a", true, NULL, "SELECT a FROM tbl"), - STORE_TEST_CASE("column", "=", "b", false, NULL, "SELECT a FROM tbl") + STORE_TEST_CASE("column", "=", "a", true, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("column", "!=", "a", false, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("column", "=", "b", false, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("column", "!=", "b", true, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("column", "=", "tbl.a", true, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("column", "=", "tbl.a", true, NULL, "SELECT tbl.a FROM tbl"), + + STORE_TEST_CASE("column", "like", ".*a", true, NULL, "SELECT a from tbl"), + STORE_TEST_CASE("column", "like", ".*a", true, NULL, "SELECT tbl.a from tbl"), + STORE_TEST_CASE("column", "like", ".*a", true, NULL, "SELECT db.tbl.a from tbl"), + STORE_TEST_CASE("column", "like", ".*aa", false, NULL, "SELECT a from tbl"), + STORE_TEST_CASE("column", "like", ".*aa", false, NULL, "SELECT tbl.a from tbl"), + STORE_TEST_CASE("column", "like", ".*aa", false, NULL, "SELECT db.tbl.a from tbl"), + STORE_TEST_CASE("column", "unlike", ".*aa", true, NULL, "SELECT a from tbl"), + STORE_TEST_CASE("column", "unlike", ".*aa", true, NULL, "SELECT tbl.a from tbl"), + STORE_TEST_CASE("column", "unlike", ".*aa", true, NULL, "SELECT db.tbl.a from tbl"), + + STORE_TEST_CASE("table", "=", "tbl", true, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("table", "!=", "tbl", false, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("table", "=", "tbl2", false, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("table", "!=", "tbl2", true, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("table", "=", "db.tbl", true, NULL, "SELECT a from db.tbl"), + STORE_TEST_CASE("table", "=", "db.tbl", true, "db", "SELECT a from tbl"), + STORE_TEST_CASE("table", "!=", "db.tbl", false, NULL, "SELECT a from db.tbl"), + STORE_TEST_CASE("table", "!=", "db.tbl", false, "db", "SELECT a from tbl"), + + STORE_TEST_CASE("database", "=", "db", false, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("database", "!=", "db", true, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("database", "=", "db1", true, NULL, "SELECT a FROM db1.tbl"), + STORE_TEST_CASE("database", "!=", "db1", false, NULL, "SELECT a FROM db1.tbl"), + STORE_TEST_CASE("database", "=", "db1", true, "db1", "SELECT a FROM tbl"), + STORE_TEST_CASE("database", "!=", "db1", false, "db1", "SELECT a FROM tbl"), + + STORE_TEST_CASE("query", "=", "SELECT a FROM tbl", true, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("query", "!=", "SELECT a FROM tbl", false, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("query", "=", "SELECT b FROM tbl", false, NULL, "SELECT a FROM tbl"), + STORE_TEST_CASE("query", "!=", "SELECT b FROM tbl", true, NULL, "SELECT a FROM tbl"), + + STORE_TEST_CASE("column", "=", "a", false, NULL, "SELECT b FROM tbl WHERE a = 5"), + STORE_TEST_CASE("column", "=", "a", true, NULL, "SELECT a, b FROM tbl WHERE a = 5"), }; const size_t n_store_test_cases = sizeof(store_test_cases) / sizeof(store_test_cases[0]); @@ -144,6 +182,7 @@ int test_store() for (int i = 0; i < n_store_test_cases; ++i) { + printf("TC : %d\n", i + 1); const struct store_test_case *test_case = &store_test_cases[i]; CACHE_RULES *rules = cache_rules_parse(test_case->rule, 0); @@ -160,10 +199,12 @@ int test_store() { printf("Query : %s\n" "Rule : %s\n" + "Def-db : %s\n" "Expected: %s\n" "Result : %s\n\n", test_case->query, test_case->rule, + test_case->default_db, test_case->matches ? "A match" : "Not a match", matches ? "A match" : "Not a match"); } diff --git a/server/modules/monitor/galeramon/galeramon.c b/server/modules/monitor/galeramon/galeramon.c index db28a6123..00e33c545 100644 --- a/server/modules/monitor/galeramon/galeramon.c +++ b/server/modules/monitor/galeramon/galeramon.c @@ -535,16 +535,9 @@ monitorMain(void *arg) STRSRVSTATUS(ptr->server)); } - if (!(SERVER_IS_RUNNING(ptr->server)) || - !(SERVER_IS_IN_CLUSTER(ptr->server))) - { - dcb_hangup_foreach(ptr->server); - } - if (SERVER_IS_DOWN(ptr->server)) { /** Increase this server'e error count */ - dcb_hangup_foreach(ptr->server); ptr->mon_err_count += 1; } @@ -650,6 +643,8 @@ monitorMain(void *arg) } ptr = ptr->next; } + + mon_hangup_failed_servers(mon); } } diff --git a/server/modules/monitor/mmmon/mmmon.c b/server/modules/monitor/mmmon/mmmon.c index 08197cc10..ef5ea103f 100644 --- a/server/modules/monitor/mmmon/mmmon.c +++ b/server/modules/monitor/mmmon/mmmon.c @@ -596,15 +596,6 @@ monitorMain(void *arg) /* monitor current node */ monitorDatabase(mon, ptr); - if (mon_status_changed(ptr)) - { - if (!(SERVER_IS_RUNNING(ptr->server)) || - !(SERVER_IS_IN_CLUSTER(ptr->server))) - { - dcb_hangup_foreach(ptr->server); - } - } - if (mon_status_changed(ptr) || mon_print_fail_status(ptr)) { @@ -676,6 +667,8 @@ monitorMain(void *arg) } ptr = ptr->next; } + + mon_hangup_failed_servers(mon); } } diff --git a/server/modules/monitor/mysqlmon/mysql_mon.c b/server/modules/monitor/mysqlmon/mysql_mon.c index eafd1c251..75593b583 100644 --- a/server/modules/monitor/mysqlmon/mysql_mon.c +++ b/server/modules/monitor/mysqlmon/mysql_mon.c @@ -1210,18 +1210,6 @@ monitorMain(void *arg) ptr->server->name, ptr->server->port); } - /** - * Here we say: If the server's state changed - * so that it isn't running or some other way - * lost cluster membership, call call-back function - * of every DCB for which such callback was - * registered for this kind of issue (DCB_REASON_...) - */ - if (!(SERVER_IS_RUNNING(ptr->server)) || - !(SERVER_IS_IN_CLUSTER(ptr->server))) - { - dcb_hangup_foreach(ptr->server); - } } if (mon_status_changed(ptr)) @@ -1483,6 +1471,8 @@ monitorMain(void *arg) ptr = ptr->next; } } + + mon_hangup_failed_servers(mon); } /*< while (1) */ } diff --git a/server/modules/monitor/ndbclustermon/ndbclustermon.c b/server/modules/monitor/ndbclustermon/ndbclustermon.c index 45ec1ec54..54adb6bff 100644 --- a/server/modules/monitor/ndbclustermon/ndbclustermon.c +++ b/server/modules/monitor/ndbclustermon/ndbclustermon.c @@ -444,6 +444,8 @@ monitorMain(void *arg) } ptr = ptr->next; } + + mon_hangup_failed_servers(mon); } } diff --git a/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c b/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c index df89c4b01..cc943a460 100644 --- a/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c +++ b/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c @@ -1112,6 +1112,12 @@ static int gw_error_backend_event(DCB *dcb) CHK_SESSION(session); if (SESSION_STATE_DUMMY == session->state) { + if (dcb->persistentstart == 0) + { + /** Not a persistent connection, something is wrong. */ + MXS_ERROR("EPOLLERR event on a non-persistent DCB with no session. " + "Closing connection."); + } dcb_close(dcb); return 1; } @@ -1629,13 +1635,19 @@ static int gw_change_user(DCB *backend, } else { - rv = gw_send_change_user_to_backend(database, username, client_sha1, backend_protocol); - /* - * Now copy new data into user session - */ + /** This assumes that authentication will succeed. If authentication fails, + * the internal session will represent the wrong user. This is wrong and + * a check whether the COM_CHANGE_USER succeeded should be done in the + * backend protocol reply handling. + * + * For the time being, it is simpler to assume a COM_CHANGE_USER will always + * succeed if the authentication in MaxScale is successful. In practice this + * might not be true but these cases are handled by the router modules + * and the servers that fail to execute the COM_CHANGE_USER are discarded. */ strcpy(current_session->user, username); strcpy(current_session->db, database); memcpy(current_session->client_sha1, client_sha1, sizeof(current_session->client_sha1)); + rv = gw_send_change_user_to_backend(database, username, client_sha1, backend_protocol); } retblock: diff --git a/server/modules/protocol/MySQL/MySQLClient/mysql_client.c b/server/modules/protocol/MySQL/MySQLClient/mysql_client.c index 9346eb32e..49695b136 100644 --- a/server/modules/protocol/MySQL/MySQLClient/mysql_client.c +++ b/server/modules/protocol/MySQL/MySQLClient/mysql_client.c @@ -191,6 +191,11 @@ int MySQLSendHandshake(DCB* dcb) int len_version_string = 0; int id_num; + if (dcb->service->dbref) + { + mysql_server_language = dcb->service->dbref->server->charset; + } + MySQLProtocol *protocol = DCB_PROTOCOL(dcb, MySQLProtocol); GWBUF *buf; diff --git a/server/modules/routing/binlog/blr_file.c b/server/modules/routing/binlog/blr_file.c index 8bada0923..df71d3afb 100644 --- a/server/modules/routing/binlog/blr_file.c +++ b/server/modules/routing/binlog/blr_file.c @@ -424,7 +424,8 @@ blr_write_binlog_record(ROUTER_INSTANCE *router, REP_HEADER *hdr, uint32_t size, * Fill the gap with a self generated ignorable event * Binlog file position is incremented by blr_write_special_event() */ - if (hdr->next_pos && (hdr->next_pos > (file_offset + size))) + if (router->master_event_state == BLR_EVENT_DONE && + hdr->next_pos && (hdr->next_pos > (file_offset + size))) { uint64_t hole_size = hdr->next_pos - file_offset - size; if (!blr_write_special_event(router, file_offset, hole_size, hdr, BLRM_IGNORABLE)) diff --git a/server/modules/routing/binlog/blr_master.c b/server/modules/routing/binlog/blr_master.c index 2786f074f..533dad4f1 100644 --- a/server/modules/routing/binlog/blr_master.c +++ b/server/modules/routing/binlog/blr_master.c @@ -1229,7 +1229,14 @@ blr_handle_binlog_record(ROUTER_INSTANCE *router, GWBUF *pkt) } else { + /* Terminate replication and exit from main loop */ blr_terminate_master_replication(router, ptr, len); + + gwbuf_free(pkt); + pkt = NULL; + pkt_length = 0; + + break; } if (hdr.ok == 0) diff --git a/server/modules/routing/binlog/blr_slave.c b/server/modules/routing/binlog/blr_slave.c index 5d78a9e5d..7235251fe 100644 --- a/server/modules/routing/binlog/blr_slave.c +++ b/server/modules/routing/binlog/blr_slave.c @@ -88,6 +88,7 @@ #include #include +static char* get_next_token(char *str, const char* delim, char **saveptr); extern int load_mysql_users(SERV_LISTENER *listener); extern void blr_master_close(ROUTER_INSTANCE* router); extern int blr_file_new_binlog(ROUTER_INSTANCE *router, char *file); @@ -888,10 +889,16 @@ blr_slave_query(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue) spinlock_acquire(&router->lock); + /* Set the BLRM_UNCONFIGURED state */ router->master_state = BLRM_UNCONFIGURED; blr_master_set_empty_config(router); blr_master_free_config(current_master); + /* Remove any error message and errno */ + free(router->m_errmsg); + router->m_errmsg = NULL; + router->m_errno = 0; + spinlock_release(&router->lock); if (removed_cfg == -1) @@ -1035,11 +1042,19 @@ blr_slave_query(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue) router->master_state = BLRM_SLAVE_STOPPED; spinlock_release(&router->lock); - } - if (!router->trx_safe) - { + /* + * The binlog server has just been configured + * master.ini file written in router->binlogdir. + * Now create the binlogfile specified in MASTER_LOG_FILE + */ + + if (blr_file_new_binlog(router, router->binlog_name)) + { + MXS_INFO("%s: 'master.ini' created, binlog file '%s' created", router->service->name, router->binlog_name); + } blr_master_free_config(current_master); + return blr_slave_send_ok(router, slave); } if (router->trx_safe && router->pending_transaction) @@ -1055,17 +1070,24 @@ blr_slave_query(ROUTER_INSTANCE *router, ROUTER_SLAVE *slave, GWBUF *queue) return blr_slave_send_warning_message(router, slave, message); } - else - { - blr_master_free_config(current_master); - return blr_slave_send_ok(router, slave); - } + } - } - else + blr_master_free_config(current_master); + + /* + * The CHAMGE MASTER command might specify a new binlog file. + * Let's create the binlogfile specified in MASTER_LOG_FILE + */ + + if (strlen(router->prevbinlog) && strcmp(router->prevbinlog, router->binlog_name)) { - return blr_slave_send_ok(router, slave); - } + if (blr_file_new_binlog(router, router->binlog_name)) + { + MXS_INFO("%s: created new binlog file '%s' by 'CHANGE MASTER TO' command", + router->service->name, router->binlog_name); + } + } + return blr_slave_send_ok(router, slave); } } } @@ -3549,22 +3571,20 @@ blr_start_slave(ROUTER_INSTANCE* router, ROUTER_SLAVE* slave) /* Send warning message to mysql command */ blr_slave_send_warning_message(router, slave, msg); } + } - /* create new one */ + /* No file has beem opened, create a new binlog file */ + if (router->binlog_fd == -1) + { blr_file_new_binlog(router, router->binlog_name); } else { - if (router->binlog_fd == -1) - { - /* create new one */ - blr_file_new_binlog(router, router->binlog_name); - } - else - { - /* use existing one */ - blr_file_append(router, router->binlog_name); - } + /* A new binlog file has been created by CHANGE MASTER TO + * if no pending transaction is detected. + * use the existing one. + */ + blr_file_append(router, router->binlog_name); } /** Initialise SSL: exit on error */ @@ -4455,6 +4475,148 @@ blr_set_master_password(ROUTER_INSTANCE *router, char *password) return 0; } +/** + * Get next token + * + * Works exactly like strtok_t except that a delim character which appears + * anywhere within quotes is ignored. For instance, if delim is "," then + * a string like "MASTER_USER='maxscale_repl_user',MASTER_PASSWORD='a,a'" + * will be tokenized into the following two tokens: + * + * MASTER_USER='maxscale_repl_user' + * MASTER_PASSWORD='a,a' + * + * @see strtok_r + */ +static char* get_next_token(char *str, const char* delim, char **saveptr) +{ + if (str) + { + *saveptr = str; + } + + if (!*saveptr) + { + return NULL; + } + + bool delim_found = true; + + // Skip any delims in the beginning. + while (**saveptr && delim_found) + { + const char* d = delim; + + while (*d) + { + if (*d == **saveptr) + { + break; + } + + ++d; + } + + if (*d == 0) + { + delim_found = false; + } + else + { + ++*saveptr; + } + } + + if (!**saveptr) + { + return NULL; + } + + delim_found = false; + + char *token = *saveptr; + char *p = *saveptr; + + char quote = 0; + + while (*p && !delim_found) + { + switch (*p) + { + case '\'': + case '"': + case '`': + if (!quote) + { + quote = *p; + } + else if (quote == *p) + { + quote = 0; + } + break; + + default: + if (!quote) + { + const char *d = delim; + while (*d && !delim_found) + { + if (*p == *d) + { + delim_found = true; + *p = 0; + } + else + { + ++d; + } + } + } + } + + ++p; + } + + if (*p == 0) + { + *saveptr = NULL; + } + else if (delim_found) + { + *saveptr = p; + + delim_found = true; + + while (**saveptr && delim_found) + { + const char *d = delim; + while (*d) + { + if (**saveptr == *d) + { + break; + } + else + { + ++d; + } + } + + if (*d == 0) + { + delim_found = false; + } + else + { + ++*saveptr; + } + } + } + + return token; +} + /** * Parse a CHANGE MASTER TO SQL command * @@ -4469,7 +4631,7 @@ blr_parse_change_master_command(char *input, char *error_string, CHANGE_MASTER_O char *sep = ","; char *word, *brkb; - if ((word = strtok_r(input, sep, &brkb)) == NULL) + if ((word = get_next_token(input, sep, &brkb)) == NULL) { snprintf(error_string, BINLOG_ERROR_MSG_LEN, "Unable to parse query [%s]", input); return 1; @@ -4483,7 +4645,7 @@ blr_parse_change_master_command(char *input, char *error_string, CHANGE_MASTER_O } } - while ((word = strtok_r(NULL, sep, &brkb)) != NULL) + while ((word = get_next_token(NULL, sep, &brkb)) != NULL) { /* parse options key=val */ if (blr_handle_change_master_token(word, error_string, config)) @@ -4507,12 +4669,12 @@ static int blr_handle_change_master_token(char *input, char *error, CHANGE_MASTER_OPTIONS *config) { /* space+TAB+= */ - char *sep = " ="; + char *sep = " \t="; char *word, *brkb; char *value = NULL; char **option_field = NULL; - if ((word = strtok_r(input, sep, &brkb)) == NULL) + if ((word = get_next_token(input, sep, &brkb)) == NULL) { snprintf(error, BINLOG_ERROR_MSG_LEN, "error parsing %s", brkb); return 1; @@ -4551,7 +4713,7 @@ static char * blr_get_parsed_command_value(char *input) { /* space+TAB+= */ - char *sep = " ="; + char *sep = " \t="; char *ret = NULL; char *word; char *value = NULL; @@ -4565,7 +4727,7 @@ blr_get_parsed_command_value(char *input) return ret; } - if ((word = strtok_r(NULL, sep, &input)) != NULL) + if ((word = get_next_token(NULL, sep, &input)) != NULL) { char *ptr; diff --git a/server/modules/routing/maxinfo/maxinfo_exec.c b/server/modules/routing/maxinfo/maxinfo_exec.c index c5b031b94..85cdc1f78 100644 --- a/server/modules/routing/maxinfo/maxinfo_exec.c +++ b/server/modules/routing/maxinfo/maxinfo_exec.c @@ -527,8 +527,6 @@ exec_clear(DCB *dcb, MAXINFO_TREE *tree) MXS_ERROR("%s", errmsg); } -extern void shutdown_server(); - /** * MaxScale shutdown * @param dcb Client DCB @@ -536,7 +534,7 @@ extern void shutdown_server(); */ void exec_shutdown_maxscale(DCB *dcb, MAXINFO_TREE *tree) { - shutdown_server(); + maxscale_shutdown(); maxinfo_send_ok(dcb); } diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index aab1775f5..3b779e83f 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -327,7 +327,7 @@ static void *newSession(ROUTER *router_inst, SESSION *session) if (!select_connect_backend_servers(&master_ref, backend_ref, router_nservers, max_nslaves, max_slave_rlag, client_rses->rses_config.rw_slave_select_criteria, - session, router)) + session, router, false)) { /** * Master and at least slaves must be found if the router is @@ -458,6 +458,40 @@ static void freeSession(ROUTER *router_instance, void *router_client_session) return; } +/** + * @brief Mark a backend reference as failed + * + * @param bref Backend reference to close + * @param fatal Whether the failure was fatal + */ +void close_failed_bref(backend_ref_t *bref, bool fatal) +{ + if (BREF_IS_WAITING_RESULT(bref)) + { + bref_clear_state(bref, BREF_WAITING_RESULT); + } + + bref_clear_state(bref, BREF_QUERY_ACTIVE); + bref_clear_state(bref, BREF_IN_USE); + bref_set_state(bref, BREF_CLOSED); + + if (fatal) + { + bref_set_state(bref, BREF_FATAL_FAILURE); + } + + if (sescmd_cursor_is_active(&bref->bref_sescmd_cur)) + { + sescmd_cursor_set_active(&bref->bref_sescmd_cur, false); + } + + if (bref->bref_pending_cmd) + { + gwbuf_free(bref->bref_pending_cmd); + bref->bref_pending_cmd = NULL; + } +} + /** * @brief The main routing entry point for a query (API) * @@ -652,7 +686,8 @@ static void clientReply(ROUTER *instance, void *router_session, GWBUF *writebuf, router_cli_ses->rses_config.rw_max_slave_replication_lag, router_cli_ses->rses_config.rw_slave_select_criteria, router_cli_ses->rses_master_ref->bref_dcb->session, - router_cli_ses->router); + router_cli_ses->router, + true); } } /** @@ -1296,29 +1331,11 @@ static void handleError(ROUTER *instance, void *router_session, * If master has lost its Master status error can't be * handled so that session could continue. */ - if (rses->rses_master_ref && rses->rses_master_ref->bref_dcb == problem_dcb && - !SERVER_IS_MASTER(rses->rses_master_ref->ref->server)) + if (rses->rses_master_ref && rses->rses_master_ref->bref_dcb == problem_dcb) { SERVER *srv = rses->rses_master_ref->ref->server; - backend_ref_t *bref; - bref = get_bref_from_dcb(rses, problem_dcb); - if (bref != NULL) - { - CHK_BACKEND_REF(bref); - if (BREF_IS_WAITING_RESULT(bref)) - { - bref_clear_state(bref, BREF_WAITING_RESULT); - } - bref_clear_state(bref, BREF_IN_USE); - bref_set_state(bref, BREF_CLOSED); - } - else - { - MXS_ERROR("server %s:%d lost the " - "master status but could not locate the " - "corresponding backend ref.", - srv->name, srv->port); - } + backend_ref_t *bref = get_bref_from_dcb(rses, problem_dcb); + bool can_continue = false; if (rses->rses_config.rw_master_failure_mode != RW_FAIL_INSTANTLY && (bref == NULL || !BREF_IS_WAITING_RESULT(bref))) @@ -1332,38 +1349,41 @@ static void handleError(ROUTER *instance, void *router_session, * can't be sure whether it was executed or not. In this * case the safest thing to do is to close the client * connection. */ - *succp = true; + can_continue = true; + } + else if (!SERVER_IS_MASTER(srv) && !srv->master_err_is_logged) + { + MXS_ERROR("Server %s:%d lost the master status. Readwritesplit " + "service can't locate the master. Client sessions " + "will be closed.", srv->name, srv->port); + srv->master_err_is_logged = true; + } + + *succp = can_continue; + + if (bref != NULL) + { + CHK_BACKEND_REF(bref); + close_failed_bref(bref, true); } else { - if (!srv->master_err_is_logged) - { - MXS_ERROR("server %s:%d lost the " - "master status. Readwritesplit " - "service can't locate the master. " - "Client sessions will be closed.", - srv->name, srv->port); - srv->master_err_is_logged = true; - } - *succp = false; - } + MXS_ERROR("Server %s:%d lost the master status but could not locate the " + "corresponding backend ref.", srv->name, srv->port); + } } else { /** * This is called in hope of getting replacement for - * failed slave(s). This call may free rses. + * failed slave(s). */ *succp = handle_error_new_connection(inst, &rses, problem_dcb, errmsgbuf); } dcb_close(problem_dcb); close_dcb = false; - /* Free the lock if rses still exists */ - if (rses) - { - rses_end_locked_router_action(rses); - } + rses_end_locked_router_action(rses); break; } @@ -1419,13 +1439,7 @@ static void handle_error_reply_client(SESSION *ses, ROUTER_CLIENT_SES *rses, if (BREF_IS_IN_USE(bref)) { - bref_clear_state(bref, BREF_IN_USE); - bref_set_state(bref, BREF_CLOSED); - if (BREF_IS_WAITING_RESULT(bref)) - { - bref_clear_state(bref, BREF_WAITING_RESULT); - } - + close_failed_bref(bref, false); dcb_close(backend_dcb); } } @@ -1499,13 +1513,11 @@ static bool handle_error_new_connection(ROUTER_INSTANCE *inst, */ if (BREF_IS_WAITING_RESULT(bref)) { - DCB *client_dcb; - client_dcb = ses->client_dcb; + DCB *client_dcb = ses->client_dcb; client_dcb->func.write(client_dcb, gwbuf_clone(errmsg)); - bref_clear_state(bref, BREF_WAITING_RESULT); } - bref_clear_state(bref, BREF_IN_USE); - bref_set_state(bref, BREF_CLOSED); + + close_failed_bref(bref, false); /** * Error handler is already called for this DCB because @@ -1542,7 +1554,7 @@ static bool handle_error_new_connection(ROUTER_INSTANCE *inst, myrses->rses_nbackends, max_nslaves, max_slave_rlag, myrses->rses_config.rw_slave_select_criteria, - ses, inst); + ses, inst, true); } return_succp: diff --git a/server/modules/routing/readwritesplit/readwritesplit.h b/server/modules/routing/readwritesplit/readwritesplit.h index 5e24a4162..216c96205 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.h +++ b/server/modules/routing/readwritesplit/readwritesplit.h @@ -38,7 +38,7 @@ typedef enum bref_state BREF_WAITING_RESULT = 0x02, /*< for session commands only */ BREF_QUERY_ACTIVE = 0x04, /*< for other queries */ BREF_CLOSED = 0x08, - BREF_SESCMD_FAILED = 0x10 /*< Backend references that should be dropped */ + BREF_FATAL_FAILURE = 0x10 /*< Backend references that should be dropped */ } bref_state_t; #define BREF_IS_NOT_USED(s) ((s)->bref_state & ~BREF_IN_USE) @@ -46,7 +46,7 @@ typedef enum bref_state #define BREF_IS_WAITING_RESULT(s) ((s)->bref_num_result_wait > 0) #define BREF_IS_QUERY_ACTIVE(s) ((s)->bref_state & BREF_QUERY_ACTIVE) #define BREF_IS_CLOSED(s) ((s)->bref_state & BREF_CLOSED) -#define BREF_HAS_FAILED(s) ((s)->bref_state & BREF_SESCMD_FAILED) +#define BREF_HAS_FAILED(s) ((s)->bref_state & BREF_FATAL_FAILURE) typedef enum backend_type_t { diff --git a/server/modules/routing/readwritesplit/rwsplit_internal.h b/server/modules/routing/readwritesplit/rwsplit_internal.h index cbaaa979b..e5f38b377 100644 --- a/server/modules/routing/readwritesplit/rwsplit_internal.h +++ b/server/modules/routing/readwritesplit/rwsplit_internal.h @@ -121,10 +121,11 @@ GWBUF *sescmd_cursor_process_replies(GWBUF *replybuf, bool select_connect_backend_servers(backend_ref_t **p_master_ref, backend_ref_t *backend_ref, int router_nservers, int max_nslaves, - int max_rlag, + int max_slave_rlag, select_criteria_t select_criteria, SESSION *session, - ROUTER_INSTANCE *router); + ROUTER_INSTANCE *router, + bool active_session); /* * The following are implemented in rwsplit_tmp_table_multi.c @@ -132,13 +133,14 @@ bool select_connect_backend_servers(backend_ref_t **p_master_ref, void check_drop_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, GWBUF *querybuf, mysql_server_cmd_t packet_type); -qc_query_type_t is_read_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, +bool is_read_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, GWBUF *querybuf, qc_query_type_t type); void check_create_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, GWBUF *querybuf, qc_query_type_t type); bool check_for_multi_stmt(GWBUF *buf, void *protocol, mysql_server_cmd_t packet_type); qc_query_type_t determine_query_type(GWBUF *querybuf, int packet_type, bool non_empty_packet); +void close_failed_bref(backend_ref_t *bref, bool fatal); #ifdef __cplusplus } diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.c b/server/modules/routing/readwritesplit/rwsplit_route_stmt.c index 298358b4f..2bfa45593 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.c +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.c @@ -974,9 +974,9 @@ handle_multi_temp_and_load(ROUTER_CLIENT_SES *rses, GWBUF *querybuf, if (rses->have_tmp_tables) { check_drop_tmp_table(rses, querybuf, packet_type); - if (is_packet_a_query(packet_type)) + if (is_packet_a_query(packet_type) && is_read_tmp_table(rses, querybuf, *qtype)) { - *qtype = is_read_tmp_table(rses, querybuf, *qtype); + *qtype |= QUERY_TYPE_MASTER_READ; } } check_create_tmp_table(rses, querybuf, *qtype); @@ -1116,6 +1116,64 @@ bool handle_slave_is_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses, } } +/** + * @brief Log master write failure + * + * @param rses Router session + */ +static void log_master_routing_failure(ROUTER_CLIENT_SES *rses, bool found, + DCB *master_dcb, DCB *curr_master_dcb) +{ + char errmsg[MAX_SERVER_NAME_LEN * 2 + 100]; // Extra space for error message + + if (!found) + { + sprintf(errmsg, "Could not find a valid master connection"); + } + else if (master_dcb && curr_master_dcb) + { + /** We found a master but it's not the same connection */ + ss_dassert(master_dcb != curr_master_dcb); + if (master_dcb->server != curr_master_dcb->server) + { + sprintf(errmsg, "Master server changed from '%s' to '%s'", + master_dcb->server->unique_name, + curr_master_dcb->server->unique_name); + } + else + { + ss_dassert(false); // Currently we don't reconnect to the master + sprintf(errmsg, "Connection to master '%s' was recreated", + curr_master_dcb->server->unique_name); + } + } + else if (master_dcb) + { + /** We have an original master connection but we couldn't find it */ + sprintf(errmsg, "The connection to master server '%s' is not available", + master_dcb->server->unique_name); + } + else + { + /** We never had a master connection, the session must be in read-only mode */ + if (rses->rses_config.rw_master_failure_mode != RW_FAIL_INSTANTLY) + { + sprintf(errmsg, "Session is in read-only mode because it was created " + "when no master was available"); + } + else + { + ss_dassert(false); // A session should always have a master reference + sprintf(errmsg, "Was supposed to route to master but couldn't " + "find master in a suitable state"); + } + } + + MXS_WARNING("[%s] Write query received from %s@%s. %s. Closing client connection.", + rses->router->service->name, rses->client_dcb->user, + rses->client_dcb->remote, errmsg); +} + /** * @brief Handle master is the target * @@ -1128,7 +1186,7 @@ bool handle_slave_is_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses, * @return bool - true if succeeded, false otherwise */ bool handle_master_is_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses, - DCB **target_dcb) + DCB **target_dcb) { DCB *master_dcb = rses->rses_master_ref ? rses->rses_master_ref->bref_dcb : NULL; DCB *curr_master_dcb = NULL; @@ -1141,29 +1199,26 @@ bool handle_master_is_target(ROUTER_INSTANCE *inst, ROUTER_CLIENT_SES *rses, } else { - if (succp && master_dcb != curr_master_dcb) + if (succp && master_dcb == curr_master_dcb) { - MXS_INFO("Was supposed to route to master but master has changed."); + atomic_add(&inst->stats.n_master, 1); + *target_dcb = master_dcb; } else { - MXS_INFO("Was supposed to route to master but couldn't find master" - " in a suitable state."); - } - - if (rses->rses_config.rw_master_failure_mode == RW_ERROR_ON_WRITE) - { - /** Old master is no longer available */ - succp = send_readonly_error(rses->client_dcb); - } - else - { - MXS_WARNING("[%s] Write query received from %s@%s when no master is " - "available, closing client connection.", inst->service->name, - rses->client_dcb->user, rses->client_dcb->remote); - succp = false; + /** The original master is not available, we can't route the write */ + if (rses->rses_config.rw_master_failure_mode == RW_ERROR_ON_WRITE) + { + succp = send_readonly_error(rses->client_dcb); + } + else + { + log_master_routing_failure(rses, succp, master_dcb, curr_master_dcb); + succp = false; + } } } + return succp; } @@ -1369,6 +1424,7 @@ static backend_ref_t *get_root_master_bref(ROUTER_CLIENT_SES *rses) bref = &rses->rses_backend_ref[i]; if (bref && BREF_IS_IN_USE(bref)) { + ss_dassert(!BREF_IS_CLOSED(bref) && !BREF_HAS_FAILED(bref)); if (bref == rses->rses_master_ref) { /** Store master state for better error reporting */ @@ -1386,13 +1442,11 @@ static backend_ref_t *get_root_master_bref(ROUTER_CLIENT_SES *rses) } } - if (candidate_bref == NULL && rses->rses_config.rw_master_failure_mode == RW_FAIL_INSTANTLY) + if (candidate_bref == NULL && rses->rses_config.rw_master_failure_mode == RW_FAIL_INSTANTLY && + rses->rses_master_ref && BREF_IS_IN_USE(rses->rses_master_ref)) { - MXS_ERROR("Could not find master among the backend " - "servers. Previous master's state : %s", - rses->rses_master_ref == NULL ? "No master found" : - (!BREF_IS_IN_USE(rses->rses_master_ref) ? "Master is not in use" : - STRSRVSTATUS(&master))); + MXS_ERROR("Could not find master among the backend servers. " + "Previous master's state : %s", STRSRVSTATUS(&master)); } return candidate_bref; diff --git a/server/modules/routing/readwritesplit/rwsplit_select_backends.c b/server/modules/routing/readwritesplit/rwsplit_select_backends.c index 35875be65..59b599276 100644 --- a/server/modules/routing/readwritesplit/rwsplit_select_backends.c +++ b/server/modules/routing/readwritesplit/rwsplit_select_backends.c @@ -61,12 +61,71 @@ int (*criteria_cmpfun[LAST_CRITERIA])(const void *, const void *) = bref_cmp_current_load }; -/* - * The following function is the only one that is called from elsewhere in - * the read write split router. It is not intended for use from outside this - * router. Other functions in this module are internal and are called - * directly or indirectly by this function. +/** + * @brief Check whether it's possible to connect to this server + * + * @param bref Backend reference + * @return True if a connection to this server can be attempted */ +static bool bref_valid_for_connect(const backend_ref_t *bref) +{ + return !BREF_HAS_FAILED(bref) && SERVER_IS_RUNNING(bref->ref->server); +} + +/** + * Check whether it's possible to use this server as a slave + * + * @param bref Backend reference + * @param master_host The master server + * @return True if this server is a valid slave candidate + */ +static bool bref_valid_for_slave(const backend_ref_t *bref, const SERVER *master_host) +{ + SERVER *server = bref->ref->server; + + return (SERVER_IS_SLAVE(server) || SERVER_IS_RELAY_SERVER(server)) && + (master_host == NULL || (server != master_host)); +} + +/** + * @brief Find the best slave candidate + * + * This function iterates through @c bref and tries to find the best backend + * reference that is not in use. @c cmpfun will be called to compare the backends. + * + * @param bref Backend reference + * @param n Size of @c bref + * @param master The master server + * @param cmpfun qsort() compatible comparison function + * @return The best slave backend reference or NULL if no candidates could be found + */ +backend_ref_t* get_slave_candidate(backend_ref_t *bref, int n, const SERVER *master, + int (*cmpfun)(const void *, const void *)) +{ + backend_ref_t *candidate = NULL; + + for (int i = 0; i < n; i++) + { + if (!BREF_IS_IN_USE(&bref[i]) && + bref_valid_for_connect(&bref[i]) && + bref_valid_for_slave(&bref[i], master)) + { + if (candidate) + { + if (cmpfun(candidate, &bref[i]) > 0) + { + candidate = &bref[i]; + } + } + else + { + candidate = &bref[i]; + } + } + } + + return candidate; +} /** * @brief Search suitable backend servers from those of router instance @@ -92,7 +151,8 @@ bool select_connect_backend_servers(backend_ref_t **p_master_ref, int max_slave_rlag, select_criteria_t select_criteria, SESSION *session, - ROUTER_INSTANCE *router) + ROUTER_INSTANCE *router, + bool active_session) { if (p_master_ref == NULL || backend_ref == NULL) { @@ -103,31 +163,33 @@ bool select_connect_backend_servers(backend_ref_t **p_master_ref, } /* get the root Master */ - SERVER_REF *master_host = get_root_master(backend_ref, router_nservers); + SERVER_REF *master_backend = get_root_master(backend_ref, router_nservers); + SERVER *master_host = master_backend ? master_backend->server : NULL; if (router->rwsplit_config.rw_master_failure_mode == RW_FAIL_INSTANTLY && - (master_host == NULL || !SERVER_REF_IS_ACTIVE(master_host) || - SERVER_IS_DOWN(master_host->server))) + (master_host == NULL || SERVER_IS_DOWN(master_host))) { MXS_ERROR("Couldn't find suitable Master from %d candidates.", router_nservers); return false; } /** - * Existing session : master is already chosen and connected. - * The function was called because new slave must be selected to replace - * failed one. + * New session: + * + * Connect to both master and slaves + * + * Existing session: + * + * Master is already connected or we don't have a master. The function was + * called because new slaves must be selected to replace failed ones. */ - bool master_connected = *p_master_ref != NULL; + bool master_connected = active_session || *p_master_ref != NULL; /** Check slave selection criteria and set compare function */ int (*p)(const void *, const void *) = criteria_cmpfun[select_criteria]; ss_dassert(p); - /** Sort the pointer list to servers according to slave selection criteria. - * The servers that match the criteria the best are at the beginning of - * the list. */ - qsort(backend_ref, (size_t) router_nservers, sizeof(backend_ref_t), p); + SERVER *old_master = *p_master_ref ? (*p_master_ref)->ref->server : NULL; if (MXS_LOG_PRIORITY_IS_ENABLED(LOG_INFO)) { @@ -139,53 +201,59 @@ bool select_connect_backend_servers(backend_ref_t **p_master_ref, const int min_nslaves = 0; /*< not configurable at the time */ bool succp = false; - /** - * Choose at least 1+min_nslaves (master and slave) and at most 1+max_nslaves - * servers from the sorted list. First master found is selected. - */ - for (int i = 0; i < router_nservers && - (slaves_connected < max_nslaves || !master_connected); i++) + if (!master_connected) { - SERVER *serv = backend_ref[i].ref->server; - - if (!BREF_HAS_FAILED(&backend_ref[i]) && - SERVER_REF_IS_ACTIVE(backend_ref[i].ref) && - SERVER_IS_RUNNING(serv)) + /** Find a master server */ + for (int i = 0; i < router_nservers; i++) { - /* check also for relay servers and don't take the master_host */ - if (slaves_found < max_nslaves && - (max_slave_rlag == MAX_RLAG_UNDEFINED || - (serv->rlag != MAX_RLAG_NOT_AVAILABLE && - serv->rlag <= max_slave_rlag)) && - (SERVER_IS_SLAVE(serv) || SERVER_IS_RELAY_SERVER(serv)) && - (master_host == NULL || (serv != master_host->server))) - { - slaves_found += 1; + SERVER *serv = backend_ref[i].ref->server; - if (BREF_IS_IN_USE((&backend_ref[i])) || - connect_server(&backend_ref[i], session, true)) - { - slaves_connected += 1; - } - } - /* take the master_host for master */ - else if (master_host && (serv == master_host->server)) + if (bref_valid_for_connect(&backend_ref[i]) && + master_host && serv == master_host) { - /** p_master_ref must be assigned with this backend_ref pointer - * because its original value may have been lost when backend - * references were sorted with qsort. */ - *p_master_ref = &backend_ref[i]; - - if (!master_connected) + if (connect_server(&backend_ref[i], session, false)) { - if (connect_server(&backend_ref[i], session, false)) - { - master_connected = true; - } + *p_master_ref = &backend_ref[i]; + break; } } } - } /*< for */ + } + + /** Calculate how many connections we already have */ + for (int i = 0; i < router_nservers; i++) + { + if (bref_valid_for_connect(&backend_ref[i]) && + bref_valid_for_slave(&backend_ref[i], master_host)) + { + slaves_found += 1; + + if (BREF_IS_IN_USE(&backend_ref[i])) + { + slaves_connected += 1; + } + } + } + + ss_dassert(slaves_connected < max_nslaves); + + backend_ref_t *bref = get_slave_candidate(backend_ref, router_nservers, master_host, p); + + /** Connect to all possible slaves */ + while (bref && slaves_connected < max_nslaves) + { + if (connect_server(bref, session, true)) + { + slaves_connected += 1; + } + else + { + /** Failed to connect, mark server as failed */ + bref_set_state(bref, BREF_FATAL_FAILURE); + } + + bref = get_slave_candidate(backend_ref, router_nservers, master_host, p); + } /** * Successful cases @@ -218,11 +286,9 @@ bool select_connect_backend_servers(backend_ref_t **p_master_ref, /** Failure cases */ else { - if (slaves_connected < min_nslaves) - { - MXS_ERROR("Couldn't establish required amount of " - "slave connections for router session."); - } + MXS_ERROR("Couldn't establish required amount of slave connections for " + "router session. Would need between %d and %d slaves but only have %d.", + min_nslaves, max_nslaves, slaves_connected); /** Clean up connections */ for (int i = 0; i < router_nservers; i++) @@ -231,8 +297,8 @@ bool select_connect_backend_servers(backend_ref_t **p_master_ref, { ss_dassert(backend_ref[i].ref->connections > 0); - /** disconnect opened connections */ - bref_clear_state(&backend_ref[i], BREF_IN_USE); + close_failed_bref(&backend_ref[i], true); + /** Decrease backend's connection counter. */ atomic_add(&backend_ref[i].ref->connections, -1); dcb_close(backend_ref[i].bref_dcb); diff --git a/server/modules/routing/readwritesplit/rwsplit_session_cmd.c b/server/modules/routing/readwritesplit/rwsplit_session_cmd.c index f06bf96e0..f215eaa7e 100644 --- a/server/modules/routing/readwritesplit/rwsplit_session_cmd.c +++ b/server/modules/routing/readwritesplit/rwsplit_session_cmd.c @@ -169,16 +169,13 @@ GWBUF *sescmd_cursor_process_replies(GWBUF *replybuf, /** Set response status received */ bref_clear_state(bref, BREF_WAITING_RESULT); - if (bref->reply_cmd != scmd->reply_cmd) + if (bref->reply_cmd != scmd->reply_cmd && BREF_IS_IN_USE(bref)) { MXS_ERROR("Slave server '%s': response differs from master's response. " "Closing connection due to inconsistent session state.", bref->ref->server->unique_name); - sescmd_cursor_set_active(scur, false); - bref_clear_state(bref, BREF_QUERY_ACTIVE); - bref_clear_state(bref, BREF_IN_USE); - bref_set_state(bref, BREF_CLOSED); - bref_set_state(bref, BREF_SESCMD_FAILED); + close_failed_bref(bref, true); + if (bref->bref_dcb) { dcb_close(bref->bref_dcb); @@ -213,12 +210,11 @@ GWBUF *sescmd_cursor_process_replies(GWBUF *replybuf, { /** This backend has already received a response */ if (ses->rses_backend_ref[i].reply_cmd != scmd->reply_cmd && - !BREF_IS_CLOSED(&ses->rses_backend_ref[i])) + !BREF_IS_CLOSED(&ses->rses_backend_ref[i]) && + BREF_IS_IN_USE(&ses->rses_backend_ref[i])) { - bref_clear_state(&ses->rses_backend_ref[i], BREF_QUERY_ACTIVE); - bref_clear_state(&ses->rses_backend_ref[i], BREF_IN_USE); - bref_set_state(&ses->rses_backend_ref[i], BREF_CLOSED); - bref_set_state(bref, BREF_SESCMD_FAILED); + close_failed_bref(&ses->rses_backend_ref[i], true); + if (ses->rses_backend_ref[i].bref_dcb) { dcb_close(ses->rses_backend_ref[i].bref_dcb); diff --git a/server/modules/routing/readwritesplit/rwsplit_tmp_table_multi.c b/server/modules/routing/readwritesplit/rwsplit_tmp_table_multi.c index e008ed031..b6b43cb23 100644 --- a/server/modules/routing/readwritesplit/rwsplit_tmp_table_multi.c +++ b/server/modules/routing/readwritesplit/rwsplit_tmp_table_multi.c @@ -109,9 +109,9 @@ void check_drop_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, GWBUF *querybuf, * @param type The type of the query resolved so far * @return The type of the query */ -qc_query_type_t is_read_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, +bool is_read_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, GWBUF *querybuf, - qc_query_type_t type) + qc_query_type_t qtype) { bool target_tmp_table = false; @@ -120,20 +120,20 @@ qc_query_type_t is_read_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, char *dbname; char hkey[MYSQL_DATABASE_MAXLEN + MYSQL_TABLE_MAXLEN + 2]; MYSQL_session *data; - qc_query_type_t qtype = type; + bool rval = false; rses_property_t *rses_prop_tmp; if (router_cli_ses == NULL || querybuf == NULL) { MXS_ERROR("[%s] Error: NULL parameters passed: %p %p", __FUNCTION__, router_cli_ses, querybuf); - return type; + return false; } if (router_cli_ses->client_dcb == NULL) { MXS_ERROR("[%s] Error: Client DCB is NULL.", __FUNCTION__); - return type; + return false; } rses_prop_tmp = router_cli_ses->rses_properties[RSES_PROP_TYPE_TMPTABLES]; @@ -142,7 +142,7 @@ qc_query_type_t is_read_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, if (data == NULL) { MXS_ERROR("[%s] Error: User data in client DBC is NULL.", __FUNCTION__); - return qtype; + return false; } dbname = (char *)data->db; @@ -166,7 +166,7 @@ qc_query_type_t is_read_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, if (hashtable_fetch(rses_prop_tmp->rses_prop_data.temp_tables, hkey)) { /**Query target is a temporary table*/ - qtype = QUERY_TYPE_READ_TMP_TABLE; + rval = true; MXS_INFO("Query targets a temporary table: %s", hkey); break; } @@ -184,7 +184,7 @@ qc_query_type_t is_read_tmp_table(ROUTER_CLIENT_SES *router_cli_ses, MXS_FREE(tbl); } - return qtype; + return rval; } /**