Fix to bug #694, http://bugs.mariadb.com/show_bug.cgi?id=694
query_classifier.cc: set_query_type lost previous query type if the new was more restrictive. Problem was that if query is both READ and SESSION_WRITE and configuration parameter use_sql_variables_in=all was set, routing target became ambiguous. Replaced call to set_query_type with simply adding new type to type (=bit field) and checking unsupported combinations in readwritesplit.c:get_route_target. If such a case is met, a detailed error is written to error log in readwritesplit.c. mysql_client.c sees the error code and sends an error to client. Then mysql_client.c calls router's handleError which ensures that there are enough backend servers so that the session can continue.
This commit is contained in:
@ -391,29 +391,6 @@ return_here:
|
|||||||
return failp;
|
return failp;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Set new query type if new is more restrictive than old.
|
|
||||||
*
|
|
||||||
* Parameters:
|
|
||||||
* @param qtype Existing type
|
|
||||||
*
|
|
||||||
* @param new_type New query type
|
|
||||||
*
|
|
||||||
* @return Query type as an unsigned int value which must be casted to qtype.
|
|
||||||
*
|
|
||||||
*
|
|
||||||
* @details The implementation relies on that enumerated values correspond
|
|
||||||
* to the restrictiviness of the value. That is, smaller value means less
|
|
||||||
* restrictive, for example, QUERY_TYPE_READ is smaller than QUERY_TYPE_WRITE.
|
|
||||||
*
|
|
||||||
*/
|
|
||||||
static u_int32_t set_query_type(
|
|
||||||
u_int32_t* qtype,
|
|
||||||
u_int32_t new_type)
|
|
||||||
{
|
|
||||||
*qtype = MAX(*qtype, new_type);
|
|
||||||
return *qtype;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Detect query type by examining parsed representation of it.
|
* Detect query type by examining parsed representation of it.
|
||||||
@ -817,7 +794,7 @@ static skygw_query_type_t resolve_query_type(
|
|||||||
break;
|
break;
|
||||||
} /**< switch */
|
} /**< switch */
|
||||||
/**< Set new query type */
|
/**< Set new query type */
|
||||||
type |= set_query_type(&type, func_qtype);
|
type |= func_qtype;
|
||||||
}
|
}
|
||||||
#if defined(UPDATE_VAR_SUPPORT)
|
#if defined(UPDATE_VAR_SUPPORT)
|
||||||
/**
|
/**
|
||||||
|
@ -870,18 +870,52 @@ int gw_read_client_event(
|
|||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
modutil_send_mysql_err_packet(dcb,
|
bool succp;
|
||||||
1,
|
GWBUF* errbuf;
|
||||||
0,
|
|
||||||
2003,
|
|
||||||
"HY000",
|
|
||||||
"Write to backend failed. Session closed.");
|
|
||||||
LOGIF(LE, (skygw_log_write_flush(
|
|
||||||
LOGFILE_ERROR,
|
|
||||||
"Error : Routing the query failed. "
|
|
||||||
"Session will be closed.")));
|
|
||||||
|
|
||||||
dcb_close(dcb);
|
/**
|
||||||
|
* Send error message indicating that routing
|
||||||
|
* failed
|
||||||
|
*/
|
||||||
|
mysql_send_custom_error(
|
||||||
|
dcb,
|
||||||
|
1,
|
||||||
|
0,
|
||||||
|
"Routing query to backend failed. See "
|
||||||
|
"the error log for further details.");
|
||||||
|
/**
|
||||||
|
* Create error to be sent to client if session
|
||||||
|
* can't be continued.
|
||||||
|
*/
|
||||||
|
errbuf = mysql_create_custom_error(
|
||||||
|
1,
|
||||||
|
0,
|
||||||
|
"Routing failed. Session is closed.");
|
||||||
|
/**
|
||||||
|
* Ensure that there are enough backends
|
||||||
|
* available.
|
||||||
|
*/
|
||||||
|
router->handleError(
|
||||||
|
router_instance,
|
||||||
|
session->router_session,
|
||||||
|
errbuf,
|
||||||
|
dcb,
|
||||||
|
ERRACT_NEW_CONNECTION,
|
||||||
|
&succp);
|
||||||
|
free(errbuf);
|
||||||
|
/**
|
||||||
|
* If there are not enough backends close
|
||||||
|
* session
|
||||||
|
*/
|
||||||
|
if (!succp)
|
||||||
|
{
|
||||||
|
LOGIF(LE, (skygw_log_write_flush(
|
||||||
|
LOGFILE_ERROR,
|
||||||
|
"Error : Routing the query failed. "
|
||||||
|
"Session will be closed.")));
|
||||||
|
|
||||||
|
dcb_close(dcb);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
goto return_rc;
|
goto return_rc;
|
||||||
|
@ -1368,8 +1368,27 @@ static route_target_t get_route_target (
|
|||||||
QUERY_IS_TYPE(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT) ||
|
QUERY_IS_TYPE(qtype, QUERY_TYPE_ENABLE_AUTOCOMMIT) ||
|
||||||
QUERY_IS_TYPE(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT))
|
QUERY_IS_TYPE(qtype, QUERY_TYPE_DISABLE_AUTOCOMMIT))
|
||||||
{
|
{
|
||||||
/** hints don't affect on routing */
|
/**
|
||||||
target = TARGET_ALL;
|
* This is problematic query because it would be routed to all
|
||||||
|
* backends but since this is SELECT that is not possible:
|
||||||
|
* 1. response set is not handled correctly in clientReply and
|
||||||
|
* 2. multiple results can degrade performance.
|
||||||
|
*/
|
||||||
|
if (QUERY_IS_TYPE(qtype, QUERY_TYPE_READ))
|
||||||
|
{
|
||||||
|
LOGIF(LE, (skygw_log_write_flush(
|
||||||
|
LOGFILE_ERROR,
|
||||||
|
"Warning : The query can't be routed to all "
|
||||||
|
"backend servers because it includes SELECT and "
|
||||||
|
"SQL variable modifications which is not supported. "
|
||||||
|
"Set use_sql_variables_in=master or split the "
|
||||||
|
"query to two, where SQL variable modifications "
|
||||||
|
"are done in the first and the SELECT in the "
|
||||||
|
"second one.")));
|
||||||
|
|
||||||
|
target = TARGET_MASTER;
|
||||||
|
}
|
||||||
|
target |= TARGET_ALL;
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
* Hints may affect on routing of the following queries
|
* Hints may affect on routing of the following queries
|
||||||
@ -2139,6 +2158,34 @@ static bool route_single_stmt(
|
|||||||
|
|
||||||
if (TARGET_IS_ALL(route_target))
|
if (TARGET_IS_ALL(route_target))
|
||||||
{
|
{
|
||||||
|
/** Multiple, conflicting routing target. Return error */
|
||||||
|
if (TARGET_IS_MASTER(route_target) ||
|
||||||
|
TARGET_IS_SLAVE(route_target))
|
||||||
|
{
|
||||||
|
char* query_str = modutil_get_query(querybuf);
|
||||||
|
char* qtype_str = skygw_get_qtype_str(qtype);
|
||||||
|
|
||||||
|
LOGIF(LE, (skygw_log_write_flush(
|
||||||
|
LOGFILE_ERROR,
|
||||||
|
"Error: Can't route %s:%s:\"%s\". SELECT with "
|
||||||
|
"session data modification is not supported "
|
||||||
|
"if configuration parameter "
|
||||||
|
"use_sql_variables_in=all .",
|
||||||
|
STRPACKETTYPE(packet_type),
|
||||||
|
qtype_str,
|
||||||
|
(query_str == NULL ? "(empty)" : query_str))));
|
||||||
|
|
||||||
|
LOGIF(LT, (skygw_log_write(LOGFILE_TRACE,
|
||||||
|
"Unable to route the query "
|
||||||
|
"without losing session data "
|
||||||
|
"modification from other "
|
||||||
|
"servers. <")));
|
||||||
|
|
||||||
|
if (query_str) free (query_str);
|
||||||
|
if (qtype_str) free(qtype_str);
|
||||||
|
succp = false;
|
||||||
|
goto retblock;
|
||||||
|
}
|
||||||
/**
|
/**
|
||||||
* It is not sure if the session command in question requires
|
* It is not sure if the session command in question requires
|
||||||
* response. Statement is examined in route_session_write.
|
* response. Statement is examined in route_session_write.
|
||||||
|
Reference in New Issue
Block a user