Remove redundant error handling code from routers
The routers no longer need to track the number of errors each DCB receives. This is now done by the protocol modules. The type of the DCB no longer needs to be checked in the handleError implementation as the function is only called when a backend DCB fails.
This commit is contained in:
parent
29ece502f5
commit
f18a40ce73
@ -1732,6 +1732,7 @@ errorReply(MXS_ROUTER *instance,
|
||||
mxs_error_action_t action,
|
||||
bool *succp)
|
||||
{
|
||||
ss_dassert(backend_dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER);
|
||||
ROUTER_INSTANCE *router = (ROUTER_INSTANCE *)instance;
|
||||
int error;
|
||||
socklen_t len;
|
||||
@ -1742,54 +1743,46 @@ errorReply(MXS_ROUTER *instance,
|
||||
mysql_errno = (unsigned long) extract_field(((uint8_t *)GWBUF_DATA(message) + 5), 16);
|
||||
errmsg = extract_message(message);
|
||||
|
||||
/** Don't handle same error twice on same DCB */
|
||||
if (backend_dcb->dcb_errhandle_called)
|
||||
/** Check router state and set errno an message */
|
||||
if (router->master_state < BLRM_BINLOGDUMP || router->master_state != BLRM_SLAVE_STOPPED)
|
||||
{
|
||||
/** Check router state and set errno an message */
|
||||
if (router->master_state < BLRM_BINLOGDUMP || router->master_state != BLRM_SLAVE_STOPPED)
|
||||
/* Authentication failed */
|
||||
if (router->master_state == BLRM_TIMESTAMP)
|
||||
{
|
||||
/* Authentication failed */
|
||||
if (router->master_state == BLRM_TIMESTAMP)
|
||||
spinlock_acquire(&router->lock);
|
||||
/* set io error message */
|
||||
if (router->m_errmsg)
|
||||
{
|
||||
spinlock_acquire(&router->lock);
|
||||
/* set io error message */
|
||||
if (router->m_errmsg)
|
||||
{
|
||||
free(router->m_errmsg);
|
||||
}
|
||||
router->m_errmsg = mxs_strdup("#28000 Authentication with master server failed");
|
||||
/* set mysql_errno */
|
||||
router->m_errno = 1045;
|
||||
|
||||
/* Stop replication */
|
||||
router->master_state = BLRM_SLAVE_STOPPED;
|
||||
spinlock_release(&router->lock);
|
||||
|
||||
/* Force backend DCB close */
|
||||
dcb_close(backend_dcb);
|
||||
|
||||
MXS_ERROR("%s: Master connection error %lu '%s' in state '%s', "
|
||||
"%s while connecting to master %s:%d",
|
||||
router->service->name, router->m_errno, router->m_errmsg,
|
||||
blrm_states[BLRM_TIMESTAMP], msg,
|
||||
router->service->dbref->server->name,
|
||||
router->service->dbref->server->port);
|
||||
free(router->m_errmsg);
|
||||
}
|
||||
}
|
||||
if (errmsg)
|
||||
{
|
||||
free(errmsg);
|
||||
}
|
||||
router->m_errmsg = mxs_strdup("#28000 Authentication with master server failed");
|
||||
/* set mysql_errno */
|
||||
router->m_errno = 1045;
|
||||
|
||||
/** we optimistically assume that previous call succeed */
|
||||
*succp = true;
|
||||
return;
|
||||
/* Stop replication */
|
||||
router->master_state = BLRM_SLAVE_STOPPED;
|
||||
spinlock_release(&router->lock);
|
||||
|
||||
/* Force backend DCB close */
|
||||
dcb_close(backend_dcb);
|
||||
|
||||
MXS_ERROR("%s: Master connection error %lu '%s' in state '%s', "
|
||||
"%s while connecting to master %s:%d",
|
||||
router->service->name, router->m_errno, router->m_errmsg,
|
||||
blrm_states[BLRM_TIMESTAMP], msg,
|
||||
router->service->dbref->server->name,
|
||||
router->service->dbref->server->port);
|
||||
}
|
||||
}
|
||||
else
|
||||
if (errmsg)
|
||||
{
|
||||
backend_dcb->dcb_errhandle_called = true;
|
||||
free(errmsg);
|
||||
}
|
||||
|
||||
/** we optimistically assume that previous call succeed */
|
||||
*succp = true;
|
||||
return;
|
||||
|
||||
len = sizeof(error);
|
||||
if (router->master &&
|
||||
getsockopt(router->master->fd, SOL_SOCKET, SO_ERROR, &error, &len) == 0 &&
|
||||
|
@ -51,5 +51,6 @@ void HintRouterSession::handleError(GWBUF* pMessage,
|
||||
mxs_error_action_t action,
|
||||
bool* pSuccess)
|
||||
{
|
||||
ss_dassert(pProblem->dcb_role == DCB_ROLE_BACKEND_HANDLER);
|
||||
MXS_ERROR("handleError not implemented yet.");
|
||||
}
|
||||
|
@ -276,26 +276,13 @@ static void handleError(MXS_ROUTER *instance,
|
||||
bool *succp)
|
||||
|
||||
{
|
||||
ss_dassert(backend_dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER);
|
||||
DCB *client_dcb;
|
||||
MXS_SESSION *session = backend_dcb->session;
|
||||
mxs_session_state_t sesstate;
|
||||
|
||||
/** Don't handle same error twice on same DCB */
|
||||
if (backend_dcb->dcb_errhandle_called)
|
||||
{
|
||||
/** we optimistically assume that previous call succeed */
|
||||
*succp = true;
|
||||
return;
|
||||
}
|
||||
else
|
||||
{
|
||||
backend_dcb->dcb_errhandle_called = true;
|
||||
}
|
||||
|
||||
sesstate = session->state;
|
||||
client_dcb = session->client_dcb;
|
||||
|
||||
if (sesstate == SESSION_STATE_ROUTER_READY)
|
||||
if (session->state == SESSION_STATE_ROUTER_READY)
|
||||
{
|
||||
CHK_DCB(client_dcb);
|
||||
client_dcb->func.write(client_dcb, gwbuf_clone(errbuf));
|
||||
|
@ -679,23 +679,12 @@ static void handleError(MXS_ROUTER *instance, MXS_ROUTER_SESSION *router_session
|
||||
DCB *problem_dcb, mxs_error_action_t action, bool *succp)
|
||||
|
||||
{
|
||||
ss_dassert(problem_dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER);
|
||||
DCB *client_dcb;
|
||||
MXS_SESSION *session = problem_dcb->session;
|
||||
mxs_session_state_t sesstate;
|
||||
ROUTER_CLIENT_SES *router_cli_ses = (ROUTER_CLIENT_SES *) router_session;
|
||||
|
||||
/** Don't handle same error twice on same DCB */
|
||||
if (problem_dcb->dcb_errhandle_called)
|
||||
{
|
||||
/** we optimistically assume that previous call succeed */
|
||||
*succp = true;
|
||||
return;
|
||||
}
|
||||
else
|
||||
{
|
||||
problem_dcb->dcb_errhandle_called = true;
|
||||
}
|
||||
|
||||
sesstate = session->state;
|
||||
client_dcb = session->client_dcb;
|
||||
|
||||
@ -706,11 +695,7 @@ static void handleError(MXS_ROUTER *instance, MXS_ROUTER_SESSION *router_session
|
||||
client_dcb->func.write(client_dcb, gwbuf_clone(errbuf));
|
||||
}
|
||||
|
||||
if (DCB_ROLE_CLIENT_HANDLER == problem_dcb->dcb_role)
|
||||
{
|
||||
dcb_close(problem_dcb);
|
||||
}
|
||||
else if (router_cli_ses && problem_dcb == router_cli_ses->backend_dcb)
|
||||
if (router_cli_ses && problem_dcb == router_cli_ses->backend_dcb)
|
||||
{
|
||||
router_cli_ses->backend_dcb = NULL;
|
||||
dcb_close(problem_dcb);
|
||||
|
@ -1180,6 +1180,7 @@ static void handleError(MXS_ROUTER *instance,
|
||||
mxs_error_action_t action,
|
||||
bool *succp)
|
||||
{
|
||||
ss_dassert(problem_dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER);
|
||||
ROUTER_INSTANCE *inst = (ROUTER_INSTANCE *)instance;
|
||||
ROUTER_CLIENT_SES *rses = (ROUTER_CLIENT_SES *)router_session;
|
||||
CHK_CLIENT_RSES(rses);
|
||||
@ -1187,150 +1188,124 @@ static void handleError(MXS_ROUTER *instance,
|
||||
|
||||
if (rses->rses_closed)
|
||||
{
|
||||
/** Session is already closed */
|
||||
problem_dcb->dcb_errhandle_called = true;
|
||||
*succp = false;
|
||||
return;
|
||||
}
|
||||
|
||||
/** Don't handle same error twice on same DCB */
|
||||
if (problem_dcb->dcb_errhandle_called)
|
||||
{
|
||||
/** we optimistically assume that previous call succeed */
|
||||
/*
|
||||
* The return of true is potentially misleading, but appears to
|
||||
* be safe with the code as it stands on 9 Sept 2015 - MNB
|
||||
*/
|
||||
*succp = true;
|
||||
return;
|
||||
}
|
||||
else
|
||||
{
|
||||
problem_dcb->dcb_errhandle_called = true;
|
||||
}
|
||||
|
||||
MXS_SESSION *session = problem_dcb->session;
|
||||
ss_dassert(session);
|
||||
|
||||
if (problem_dcb->dcb_role == DCB_ROLE_CLIENT_HANDLER)
|
||||
{
|
||||
dcb_close(problem_dcb);
|
||||
*succp = false;
|
||||
}
|
||||
else
|
||||
{
|
||||
backend_ref_t *bref = get_bref_from_dcb(rses, problem_dcb);
|
||||
backend_ref_t *bref = get_bref_from_dcb(rses, problem_dcb);
|
||||
|
||||
switch (action)
|
||||
{
|
||||
switch (action)
|
||||
{
|
||||
case ERRACT_NEW_CONNECTION:
|
||||
{
|
||||
/**
|
||||
* 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)
|
||||
{
|
||||
/**
|
||||
* 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 *srv = rses->rses_master_ref->ref->server;
|
||||
bool can_continue = false;
|
||||
|
||||
if (rses->rses_config.master_failure_mode != RW_FAIL_INSTANTLY &&
|
||||
(bref == NULL || !BREF_IS_WAITING_RESULT(bref)))
|
||||
{
|
||||
SERVER *srv = rses->rses_master_ref->ref->server;
|
||||
bool can_continue = false;
|
||||
|
||||
if (rses->rses_config.master_failure_mode != RW_FAIL_INSTANTLY &&
|
||||
(bref == NULL || !BREF_IS_WAITING_RESULT(bref)))
|
||||
{
|
||||
/** The failure of a master is not considered a critical
|
||||
* failure as partial functionality still remains. Reads
|
||||
* are allowed as long as slave servers are available
|
||||
* and writes will cause an error to be returned.
|
||||
*
|
||||
* If we were waiting for a response from the master, we
|
||||
* can't be sure whether it was executed or not. In this
|
||||
* case the safest thing to do is to close the client
|
||||
* connection. */
|
||||
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);
|
||||
RW_CHK_DCB(bref, problem_dcb);
|
||||
dcb_close(problem_dcb);
|
||||
RW_CLOSE_BREF(bref);
|
||||
close_failed_bref(bref, true);
|
||||
}
|
||||
else
|
||||
{
|
||||
MXS_ERROR("Server %s:%d lost the master status but could not locate the "
|
||||
"corresponding backend ref.", srv->name, srv->port);
|
||||
}
|
||||
/** The failure of a master is not considered a critical
|
||||
* failure as partial functionality still remains. Reads
|
||||
* are allowed as long as slave servers are available
|
||||
* and writes will cause an error to be returned.
|
||||
*
|
||||
* If we were waiting for a response from the master, we
|
||||
* can't be sure whether it was executed or not. In this
|
||||
* case the safest thing to do is to close the client
|
||||
* connection. */
|
||||
can_continue = true;
|
||||
}
|
||||
else if (bref)
|
||||
else if (!SERVER_IS_MASTER(srv) && !srv->master_err_is_logged)
|
||||
{
|
||||
/** Check whether problem_dcb is same as dcb of rses->forced_node
|
||||
* and within READ ONLY transaction:
|
||||
* if true reset rses->forced_node and close session
|
||||
*/
|
||||
if (rses->forced_node &&
|
||||
(rses->forced_node->bref_dcb == problem_dcb &&
|
||||
session_trx_is_read_only(problem_dcb->session)))
|
||||
{
|
||||
MXS_ERROR("forced_node SLAVE %s in opened READ ONLY transaction has failed:"
|
||||
" closing session",
|
||||
problem_dcb->server->unique_name);
|
||||
|
||||
rses->forced_node = NULL;
|
||||
*succp = false;
|
||||
break;
|
||||
}
|
||||
|
||||
/** We should reconnect only if we find a backend for this
|
||||
* DCB. If this DCB is an older DCB that has been closed,
|
||||
* we can ignore it. */
|
||||
*succp = handle_error_new_connection(inst, &rses, problem_dcb, errmsgbuf);
|
||||
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;
|
||||
}
|
||||
|
||||
if (bref)
|
||||
*succp = can_continue;
|
||||
|
||||
if (bref != NULL)
|
||||
{
|
||||
/** This is a valid DCB for a backend ref */
|
||||
if (BREF_IS_IN_USE(bref) && bref->bref_dcb == problem_dcb)
|
||||
{
|
||||
ss_dassert(false);
|
||||
MXS_ERROR("Backend '%s' is still in use and points to the problem DCB.",
|
||||
bref->ref->server->unique_name);
|
||||
}
|
||||
CHK_BACKEND_REF(bref);
|
||||
RW_CHK_DCB(bref, problem_dcb);
|
||||
dcb_close(problem_dcb);
|
||||
RW_CLOSE_BREF(bref);
|
||||
close_failed_bref(bref, true);
|
||||
}
|
||||
else
|
||||
{
|
||||
const char *remote = problem_dcb->state == DCB_STATE_POLLING &&
|
||||
problem_dcb->server ? problem_dcb->server->unique_name : "CLOSED";
|
||||
|
||||
MXS_ERROR("DCB connected to '%s' is not in use by the router "
|
||||
"session, not closing it. DCB is in state '%s'",
|
||||
remote, STRDCBSTATE(problem_dcb->state));
|
||||
MXS_ERROR("Server %s:%d lost the master status but could not locate the "
|
||||
"corresponding backend ref.", srv->name, srv->port);
|
||||
}
|
||||
break;
|
||||
}
|
||||
else if (bref)
|
||||
{
|
||||
/** Check whether problem_dcb is same as dcb of rses->forced_node
|
||||
* and within READ ONLY transaction:
|
||||
* if true reset rses->forced_node and close session
|
||||
*/
|
||||
if (rses->forced_node &&
|
||||
(rses->forced_node->bref_dcb == problem_dcb &&
|
||||
session_trx_is_read_only(problem_dcb->session)))
|
||||
{
|
||||
MXS_ERROR("forced_node SLAVE %s in opened READ ONLY transaction has failed:"
|
||||
" closing session",
|
||||
problem_dcb->server->unique_name);
|
||||
|
||||
rses->forced_node = NULL;
|
||||
*succp = false;
|
||||
break;
|
||||
}
|
||||
|
||||
/** We should reconnect only if we find a backend for this
|
||||
* DCB. If this DCB is an older DCB that has been closed,
|
||||
* we can ignore it. */
|
||||
*succp = handle_error_new_connection(inst, &rses, problem_dcb, errmsgbuf);
|
||||
}
|
||||
|
||||
if (bref)
|
||||
{
|
||||
/** This is a valid DCB for a backend ref */
|
||||
if (BREF_IS_IN_USE(bref) && bref->bref_dcb == problem_dcb)
|
||||
{
|
||||
ss_dassert(false);
|
||||
MXS_ERROR("Backend '%s' is still in use and points to the problem DCB.",
|
||||
bref->ref->server->unique_name);
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
const char *remote = problem_dcb->state == DCB_STATE_POLLING &&
|
||||
problem_dcb->server ? problem_dcb->server->unique_name : "CLOSED";
|
||||
|
||||
MXS_ERROR("DCB connected to '%s' is not in use by the router "
|
||||
"session, not closing it. DCB is in state '%s'",
|
||||
remote, STRDCBSTATE(problem_dcb->state));
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
case ERRACT_REPLY_CLIENT:
|
||||
{
|
||||
handle_error_reply_client(session, rses, problem_dcb, errmsgbuf);
|
||||
*succp = false; /*< no new backend servers were made available */
|
||||
break;
|
||||
}
|
||||
{
|
||||
handle_error_reply_client(session, rses, problem_dcb, errmsgbuf);
|
||||
*succp = false; /*< no new backend servers were made available */
|
||||
break;
|
||||
}
|
||||
|
||||
default:
|
||||
ss_dassert(!true);
|
||||
*succp = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -3557,33 +3557,19 @@ static void handleError(MXS_ROUTER* instance,
|
||||
mxs_error_action_t action,
|
||||
bool* succp)
|
||||
{
|
||||
ss_dassert(problem_dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER);
|
||||
MXS_SESSION* session;
|
||||
ROUTER_INSTANCE* inst = (ROUTER_INSTANCE *)instance;
|
||||
ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *)router_session;
|
||||
|
||||
CHK_DCB(problem_dcb);
|
||||
|
||||
/** Don't handle same error twice on same DCB */
|
||||
if (problem_dcb->dcb_errhandle_called)
|
||||
{
|
||||
/** we optimistically assume that previous call succeed */
|
||||
*succp = true;
|
||||
return;
|
||||
}
|
||||
else
|
||||
{
|
||||
problem_dcb->dcb_errhandle_called = true;
|
||||
}
|
||||
session = problem_dcb->session;
|
||||
|
||||
if (session == NULL || rses == NULL)
|
||||
{
|
||||
*succp = false;
|
||||
}
|
||||
else if (DCB_ROLE_CLIENT_HANDLER == problem_dcb->dcb_role)
|
||||
{
|
||||
*succp = false;
|
||||
}
|
||||
else
|
||||
{
|
||||
CHK_SESSION(session);
|
||||
|
Loading…
x
Reference in New Issue
Block a user