Fixed bug in session command resul handling. In case where backend sent error message the session command cursor wasn't updated properly.

Added check to bref_clear_state, if bref's waiter counter would go negative, decrement to global operation counter is skipped.
This commit is contained in:
VilhoRaatikka 2014-07-03 00:43:30 +03:00
parent 9724a6d32e
commit e944ae2e70
4 changed files with 85 additions and 52 deletions

View File

@ -957,18 +957,34 @@ int below_water;
}
} /* if (dcb->writeq) */
if (saved_errno != 0 &&
queue != NULL &&
saved_errno != EAGAIN &&
if (saved_errno != 0 &&
queue != NULL &&
saved_errno != EAGAIN &&
saved_errno != EWOULDBLOCK)
{
LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR,
"Error : Writing to %s socket failed due %d, %s.",
dcb_isclient(dcb) ? "client" : "backend server",
saved_errno,
strerror(saved_errno))));
bool dolog = true;
/**
* Do not log if writing COM_QUIT to backend failed.
*/
if (GWBUF_IS_TYPE_MYSQL(queue))
{
uint8_t* data = GWBUF_DATA(queue);
if (data[4] == 0x01)
{
dolog = false;
}
}
if (dolog)
{
LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR,
"Error : Writing to %s socket failed due %d, %s.",
dcb_isclient(dcb) ? "client" : "backend server",
saved_errno,
strerror(saved_errno))));
}
spinlock_release(&dcb->writeqlock);
return 0;
}

View File

@ -289,6 +289,7 @@ typedef struct {
#define MYSQL_GET_STMTOK_NPARAM(payload) (gw_mysql_get_byte2(&payload[9]))
#define MYSQL_GET_STMTOK_NATTR(payload) (gw_mysql_get_byte2(&payload[11]))
#define MYSQL_IS_ERROR_PACKET(payload) (MYSQL_GET_COMMAND(payload)==0xff)
#define MYSQL_IS_COM_QUIT(payload) (MYSQL_GET_COMMAND(payload)==0x01)
#define MYSQL_GET_NATTR(payload) ((int)payload[4])
#endif /** _MYSQL_PROTOCOL_H */

View File

@ -430,7 +430,6 @@ static int gw_read_backend_event(DCB *dcb) {
CHK_SESSION(session);
router = session->service->router;
router_instance = session->service->router_instance;
rsession = session->router_session;
/* read available backend data */
rc = dcb_read(dcb, &read_buffer);
@ -450,7 +449,7 @@ static int gw_read_backend_event(DCB *dcb) {
"Read from backend failed");
router->handleError(router_instance,
rsession,
session->router_session,
errbuf,
dcb,
ERRACT_NEW_CONNECTION,
@ -525,9 +524,10 @@ static int gw_read_backend_event(DCB *dcb) {
MYSQL_IDLE)
{
gwbuf_set_type(read_buffer, GWBUF_TYPE_MYSQL);
router->clientReply(
router_instance,
rsession,
session->router_session,
read_buffer,
dcb);
rc = 1;
@ -537,7 +537,7 @@ static int gw_read_backend_event(DCB *dcb) {
else if (dcb->session->client->dcb_role == DCB_ROLE_INTERNAL)
{
gwbuf_set_type(read_buffer, GWBUF_TYPE_MYSQL);
router->clientReply(router_instance, rsession, read_buffer, dcb);
router->clientReply(router_instance, session->router_session, read_buffer, dcb);
rc = 1;
}
}
@ -565,32 +565,42 @@ static int gw_write_backend_event(DCB *dcb) {
* Don't write to backend if backend_dcb is not in poll set anymore.
*/
if (dcb->state != DCB_STATE_POLLING) {
if (dcb->writeq != NULL) {
/*< vraa : errorHandle */
mysql_send_custom_error(
dcb->session->client,
1,
0,
"Writing to backend failed due invalid Maxscale "
"state.");
LOGIF(LD, (skygw_log_write(
LOGFILE_DEBUG,
"%lu [gw_write_backend_event] Write to backend "
"dcb %p fd %d "
"failed due invalid state %s.",
pthread_self(),
dcb,
dcb->fd,
STRDCBSTATE(dcb->state))));
uint8_t* data;
LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR,
"Error : Attempt to write buffered data to backend "
"failed "
"due internal inconsistent state.")));
if (dcb->writeq != NULL)
{
data = (uint8_t *)GWBUF_DATA(dcb->writeq);
rc = 0;
} else {
if (!(MYSQL_IS_COM_QUIT(data)))
{
/*< vraa : errorHandle */
mysql_send_custom_error(
dcb->session->client,
1,
0,
"Writing to backend failed due invalid Maxscale "
"state.");
LOGIF(LD, (skygw_log_write(
LOGFILE_DEBUG,
"%lu [gw_write_backend_event] Write to backend "
"dcb %p fd %d "
"failed due invalid state %s.",
pthread_self(),
dcb,
dcb->fd,
STRDCBSTATE(dcb->state))));
LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR,
"Error : Attempt to write buffered data to backend "
"failed "
"due internal inconsistent state.")));
rc = 0;
}
}
else
{
LOGIF(LD, (skygw_log_write(
LOGFILE_DEBUG,
"%lu [gw_write_backend_event] Dcb %p in state %s "
@ -914,6 +924,7 @@ gw_backend_hangup(DCB *dcb)
CHK_DCB(dcb);
session = dcb->session;
CHK_SESSION(session);
rsession = session->router_session;
router = session->service->router;
router_instance = session->service->router_instance;
@ -971,6 +982,7 @@ gw_backend_close(DCB *dcb)
CHK_SESSION(session);
quitbuf = mysql_create_com_quit(NULL, 0);
gwbuf_set_type(quitbuf, GWBUF_TYPE_MYSQL);
/** Send COM_QUIT to the backend being closed */
mysql_send_com_quit(dcb, 0, quitbuf);

View File

@ -1528,7 +1528,8 @@ static void clientReply (
*/
if (sescmd_cursor_is_active(scur))
{
if (MYSQL_IS_ERROR_PACKET(((uint8_t *)GWBUF_DATA(writebuf))))
if (LOG_IS_ENABLED(LOGFILE_ERROR) &&
MYSQL_IS_ERROR_PACKET(((uint8_t *)GWBUF_DATA(writebuf))))
{
SESSION* ses = backend_dcb->session;
uint8_t* buf =
@ -1546,14 +1547,9 @@ static void clientReply (
bref->bref_backend->backend_server->port)));
free(cmdstr);
/** Inform the client */
handle_error_reply_client(ses, writebuf);
/** Unlock router session */
rses_end_locked_router_action(router_cli_ses);
goto lock_failed;
}
else if (GWBUF_IS_TYPE_SESCMD_RESPONSE(writebuf))
if (GWBUF_IS_TYPE_SESCMD_RESPONSE(writebuf))
{
/**
* Discard all those responses that have already been sent to
@ -1691,12 +1687,17 @@ static void bref_clear_state(
/** Decrease waiter count */
prev1 = atomic_add(&bref->bref_num_result_wait, -1);
ss_dassert(prev1 > 0);
/** Decrease global operation count */
prev2 = atomic_add(
&bref->bref_backend->backend_server->stats.n_current_ops, -1);
ss_dassert(prev2 > 0);
if (prev1 <= 0) {
atomic_add(&bref->bref_num_result_wait, 1);
}
else
{
/** Decrease global operation count */
prev2 = atomic_add(
&bref->bref_backend->backend_server->stats.n_current_ops, -1);
ss_dassert(prev2 > 0);
}
}
}
@ -3145,12 +3146,15 @@ static void handleError (
backend_dcb->dcb_errhandle_called = true;
#endif
session = backend_dcb->session;
CHK_SESSION(session);
if (session != NULL)
CHK_SESSION(session);
switch (action) {
case ERRACT_NEW_CONNECTION:
{
CHK_CLIENT_RSES(rses);
if (rses != NULL)
CHK_CLIENT_RSES(rses);
if (!rses_begin_locked_router_action(rses))
{