hcq/MaxScale
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Close the DCB and the related backend at the same time

Browse Source 
Closing the DCB and the backend reference that uses it at the same time
makes the error handling code clearer and removes some of the assumptions
that the code made. It will cause the DCB to be closed in multiple places
but the logic of why a DCB is being closed is more visible from the code.

This change should remove all cases where a DCB is closed without a
tightly coupled backend reference.
This commit is contained in:
Markus Makela
2016-12-07 08:53:43 +02:00
parent 970d70700d
commit 0fab454e66
1 changed files with 16 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

68
server/modules/routing/readwritesplit/readwritesplit.c
View File

@ -4473,15 +4473,17 @@ static void handleError(ROUTER *instance, void *router_session,
}
session = problem_dcb->session;
bool close_dcb = true;
backend_ref_t *bref = get_bref_from_dcb(rses, problem_dcb);
if (session == NULL || rses == NULL)
if (session == NULL)
{
MXS_ERROR("Session of DCB %p is NULL, won't close the DCB.", problem_dcb);
ss_dassert(false);
*succp = false;
}
else if (DCB_ROLE_CLIENT_HANDLER == problem_dcb->dcb_role)
{
dcb_close(problem_dcb);
*succp = false;
}
else
@ -4529,6 +4531,9 @@ static void handleError(ROUTER *instance, void *router_session,
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
@ -4545,21 +4550,13 @@ static void handleError(ROUTER *instance, void *router_session,
*succp = handle_error_new_connection(inst, &rses, problem_dcb, errmsgbuf);
}
RW_CHK_DCB(bref, problem_dcb);
if (bref)
{
/** This is a valid DCB for a backend ref */
if (!BREF_IS_IN_USE(bref) || bref->bref_dcb != problem_dcb)
if (BREF_IS_IN_USE(bref) && bref->bref_dcb == problem_dcb)
{
/** The backend is closed or the reference was replaced */
dcb_close(problem_dcb);
RW_CLOSE_BREF(bref);
}
else
{
MXS_ERROR("Backend '%s' is still in use and points to the problem DCB. Not closing.",
ss_dassert(false);
MXS_ERROR("Backend '%s' is still in use and points to the problem DCB.",
bref->bref_backend->backend_server->unique_name);
}
}
@ -4571,29 +4568,13 @@ static void handleError(ROUTER *instance, void *router_session,
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("Backends currently in use:");
for (int i = 0; i < rses->rses_nbackends; i++)
{
dcb_state_t state = DCB_STATE_UNDEFINED;
if (BREF_IS_IN_USE(&rses->rses_backend_ref[i]))
{
state = rses->rses_backend_ref[i].bref_dcb->state;
}
MXS_ERROR("%p: %s - %p", &rses->rses_backend_ref[i], STRDCBSTATE(state),
rses->rses_backend_ref[i].bref_dcb);
}
}
close_dcb = false;
break;
}
case ERRACT_REPLY_CLIENT:
{
handle_error_reply_client(session, rses, problem_dcb, errmsgbuf);
close_dcb = false;
*succp = false; /*< no new backend servers were made available */
break;
}
@ -4605,12 +4586,6 @@ static void handleError(ROUTER *instance, void *router_session,
}
}
if (close_dcb)
{
RW_CHK_DCB(bref, problem_dcb);
dcb_close(problem_dcb);
RW_CLOSE_BREF(bref);
}
rses_end_locked_router_action(rses);
}
@ -4686,9 +4661,12 @@ static bool handle_error_new_connection(ROUTER_INSTANCE *inst,
/**
* If bref == NULL it has been replaced already with another one.
*
* NOTE: This can never happen.
*/
if ((bref = get_bref_from_dcb(myrses, backend_dcb)) == NULL)
{
ss_dassert(false);
succp = true;
goto return_succp;
}
@ -4705,25 +4683,11 @@ static bool handle_error_new_connection(ROUTER_INSTANCE *inst,
client_dcb->func.write(client_dcb, gwbuf_clone(errmsg));
}
RW_CHK_DCB(bref, backend_dcb);
dcb_close(backend_dcb);
RW_CLOSE_BREF(bref);
close_failed_bref(bref, false);
/**
* Error handler is already called for this DCB because
* it's not polling anymore. It can be assumed that
* it succeed because rses isn't closed.
*/
if (backend_dcb->state != DCB_STATE_POLLING)
{
succp = true;
goto return_succp;
}
/**
* Remove callback because this DCB won't be used
* unless it is reconnected later, and then the callback
* is set again.
*/
dcb_remove_callback(backend_dcb, DCB_REASON_NOT_RESPONDING,
&router_handle_state_switch, (void *)bref);
router_nservers = router_get_servercount(inst);
max_nslaves = rses_get_max_slavecount(myrses, router_nservers);
max_slave_rlag = rses_get_max_replication_lag(myrses);