Move responsibility for closing DCB on error to router error handling. Check that routers remove or disable links to closed DCB.

This commit is contained in:
counterpoint
2015-09-09 09:33:00 +01:00
parent 2e50dfd484
commit f6916a23bd
10 changed files with 89 additions and 85 deletions

View File

@ -55,6 +55,7 @@
* fixes for various error situations, * fixes for various error situations,
* remove dcb_set_state etc, simplifications. * remove dcb_set_state etc, simplifications.
* 10/07/2015 Martin Brampton Simplify, merge dcb_read and dcb_read_n * 10/07/2015 Martin Brampton Simplify, merge dcb_read and dcb_read_n
* 04/09/2015 Martin Brampton Changes to ensure DCB always has session pointer
* *
* @endverbatim * @endverbatim
*/ */

View File

@ -74,8 +74,7 @@ int max_poll_sleep;
* thread utilisation and fairer scheduling of the event * thread utilisation and fairer scheduling of the event
* processing. * processing.
* 07/07/15 Martin Brampton Simplified add and remove DCB, improve error handling. * 07/07/15 Martin Brampton Simplified add and remove DCB, improve error handling.
* 23/08/15 Martin Brampton Provisionally added test so only DCB with a * 23/08/15 Martin Brampton Added test so only DCB with a session link can be added to the poll list
* session link can be added to the poll list
* *
* @endverbatim * @endverbatim
*/ */

View File

@ -415,7 +415,6 @@ static int gw_read_backend_event(DCB *dcb) {
session->state = SESSION_STATE_STOPPING; session->state = SESSION_STATE_STOPPING;
spinlock_release(&session->ses_lock); spinlock_release(&session->ses_lock);
ss_dassert(dcb->dcb_errhandle_called); ss_dassert(dcb->dcb_errhandle_called);
dcb_close(dcb);
rc = 1; rc = 1;
goto return_rc; goto return_rc;
} }
@ -488,8 +487,6 @@ static int gw_read_backend_event(DCB *dcb) {
session->state = SESSION_STATE_STOPPING; session->state = SESSION_STATE_STOPPING;
spinlock_release(&session->ses_lock); spinlock_release(&session->ses_lock);
} }
ss_dassert(dcb->dcb_errhandle_called);
dcb_close(dcb);
rc = 0; rc = 0;
goto return_rc; goto return_rc;
} }
@ -912,8 +909,6 @@ static int gw_error_backend_event(DCB *dcb)
session->state = SESSION_STATE_STOPPING; session->state = SESSION_STATE_STOPPING;
spinlock_release(&session->ses_lock); spinlock_release(&session->ses_lock);
} }
ss_dassert(dcb->dcb_errhandle_called);
dcb_close(dcb);
retblock: retblock:
return 1; return 1;
@ -1144,7 +1139,6 @@ gw_backend_hangup(DCB *dcb)
session->state = SESSION_STATE_STOPPING; session->state = SESSION_STATE_STOPPING;
spinlock_release(&session->ses_lock); spinlock_release(&session->ses_lock);
} }
ss_dassert(dcb->dcb_errhandle_called);
retblock: retblock:
return 1; return 1;
@ -1324,8 +1318,6 @@ static int backend_write_delayqueue(DCB *dcb)
spinlock_acquire(&session->ses_lock); spinlock_acquire(&session->ses_lock);
session->state = SESSION_STATE_STOPPING; session->state = SESSION_STATE_STOPPING;
spinlock_release(&session->ses_lock); spinlock_release(&session->ses_lock);
ss_dassert(dcb->dcb_errhandle_called);
dcb_close(dcb);
} }
} }
} }

View File

@ -39,6 +39,8 @@
* 10/11/2014 Massimiliano Pinto Added: client charset added to protocol struct * 10/11/2014 Massimiliano Pinto Added: client charset added to protocol struct
* 29/05/2015 Markus Makela Added SSL support * 29/05/2015 Markus Makela Added SSL support
* 11/06/2015 Martin Brampton COM_QUIT suppressed for persistent connections * 11/06/2015 Martin Brampton COM_QUIT suppressed for persistent connections
* 04/09/2015 Martin Brampton Introduce DUMMY session to fulfill guarantee DCB always has session
* 09/09/2015 Martin Brampton Modify error handler calls
*/ */
#include <skygw_utils.h> #include <skygw_utils.h>
#include <log_manager.h> #include <log_manager.h>
@ -813,7 +815,6 @@ int gw_read_client_event(
LOGFILE_ERROR, LOGFILE_ERROR,
"Error : Routing the query failed. " "Error : Routing the query failed. "
"Session will be closed."))); "Session will be closed.")));
dcb_close(dcb);
} }
rc = 1; rc = 1;
goto return_rc; goto return_rc;
@ -1192,7 +1193,6 @@ int gw_read_client_event(
"Error : Routing the query failed. " "Error : Routing the query failed. "
"Session will be closed."))); "Session will be closed.")));
dcb_close(dcb);
} }
} }
} }

View File

@ -37,7 +37,7 @@
* 18/02/2015 Massimiliano Pinto Addition of dcb_close in closeSession * 18/02/2015 Massimiliano Pinto Addition of dcb_close in closeSession
* 07/05/2015 Massimiliano Pinto Addition of MariaDB 10 compatibility support * 07/05/2015 Massimiliano Pinto Addition of MariaDB 10 compatibility support
* 12/06/2015 Massimiliano Pinto Addition of MariaDB 10 events in diagnostics() * 12/06/2015 Massimiliano Pinto Addition of MariaDB 10 events in diagnostics()
* 09/09/2015 Martin Brampton Modify error handler
* *
* @endverbatim * @endverbatim
*/ */
@ -1114,6 +1114,7 @@ char msg[85], *errmsg;
if (errmsg) if (errmsg)
free(errmsg); free(errmsg);
*succp = true; *succp = true;
dcb_close(backend_dcb);
LOGIF(LM, (skygw_log_write_flush( LOGIF(LM, (skygw_log_write_flush(
LOGFILE_MESSAGE, LOGFILE_MESSAGE,
"%s: Master %s disconnected after %ld seconds. " "%s: Master %s disconnected after %ld seconds. "

View File

@ -26,6 +26,7 @@
* Date Who Description * Date Who Description
* 16/02/15 Mark Riddoch Initial implementation * 16/02/15 Mark Riddoch Initial implementation
* 27/02/15 Massimiliano Pinto Added maxinfo_add_mysql_user * 27/02/15 Massimiliano Pinto Added maxinfo_add_mysql_user
* 09/09/2015 Martin Brampton Modify error handler
* *
* @endverbatim * @endverbatim
*/ */
@ -335,6 +336,7 @@ static void handleError(
} }
/** false because connection is not available anymore */ /** false because connection is not available anymore */
dcb_close(backend_dcb);
*succp = false; *succp = false;
} }

View File

@ -67,7 +67,8 @@
* 06/03/2014 Massimiliano Pinto Server connection counter is now updated in closeSession * 06/03/2014 Massimiliano Pinto Server connection counter is now updated in closeSession
* 24/06/2014 Massimiliano Pinto New rules for selecting the Master server * 24/06/2014 Massimiliano Pinto New rules for selecting the Master server
* 27/06/2014 Mark Riddoch Addition of server weighting * 27/06/2014 Mark Riddoch Addition of server weighting
* 11/06/2015 Martin Brampton Remove decrement n_current (moved to dcb.c) * 11/06/2015 Martin Brampton Remove decrement n_current (moved to dcb.c)
* 09/09/2015 Martin Brampton Modify error handler
* *
* @endverbatim * @endverbatim
*/ */

View File

@ -67,6 +67,8 @@ extern __thread log_info_t tls_log_info;
* 18/07/2013 Massimiliano Pinto routeQuery now handles COM_QUIT * 18/07/2013 Massimiliano Pinto routeQuery now handles COM_QUIT
* as QUERY_TYPE_SESSION_WRITE * as QUERY_TYPE_SESSION_WRITE
* 17/07/2014 Massimiliano Pinto Server connection counter is updated in closeSession * 17/07/2014 Massimiliano Pinto Server connection counter is updated in closeSession
*
* 09/09/2015 Martin Brampton Modify error handler
* *
* @endverbatim * @endverbatim
*/ */
@ -4825,17 +4827,14 @@ static void handleError (
CHK_DCB(backend_dcb); CHK_DCB(backend_dcb);
/** Reset error handle flag from a given DCB */
if (action == ERRACT_RESET)
{
backend_dcb->dcb_errhandle_called = false;
return;
}
/** Don't handle same error twice on same DCB */ /** Don't handle same error twice on same DCB */
if (backend_dcb->dcb_errhandle_called) if (backend_dcb->dcb_errhandle_called)
{ {
/** we optimistically assume that previous call succeed */ /** 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; *succp = true;
return; return;
} }
@ -4848,12 +4847,13 @@ static void handleError (
if (session == NULL || rses == NULL) if (session == NULL || rses == NULL)
{ {
*succp = false; *succp = false;
return;
} }
CHK_SESSION(session); else
CHK_CLIENT_RSES(rses); {
CHK_SESSION(session);
CHK_CLIENT_RSES(rses);
switch (action) { switch (action) {
case ERRACT_NEW_CONNECTION: case ERRACT_NEW_CONNECTION:
{ {
SERVER* srv; SERVER* srv;
@ -4861,7 +4861,7 @@ static void handleError (
if (!rses_begin_locked_router_action(rses)) if (!rses_begin_locked_router_action(rses))
{ {
*succp = false; *succp = false;
return; break;
} }
srv = rses->rses_master_ref->bref_backend->backend_server; srv = rses->rses_master_ref->bref_backend->backend_server;
/** /**
@ -4914,7 +4914,9 @@ static void handleError (
default: default:
*succp = false; *succp = false;
break; break;
}
} }
dcb_close(backend_dcb);
} }

View File

@ -58,6 +58,7 @@ extern __thread log_info_t tls_log_info;
* *
* Date Who Description * Date Who Description
* 01/12/2014 Vilho Raatikka/Markus Mäkelä Initial implementation * 01/12/2014 Vilho Raatikka/Markus Mäkelä Initial implementation
* 09/09/2015 Martin Brampton Modify error handler
* *
* @endverbatim * @endverbatim
*/ */
@ -4103,37 +4104,38 @@ return_succp:
* tell whether router has enough master/slave connections to continue work. * tell whether router has enough master/slave connections to continue work.
*/ */
static void handleError ( static void handleError (
ROUTER* instance, ROUTER* instance,
void* router_session, void* router_session,
GWBUF* errmsgbuf, GWBUF* errmsgbuf,
DCB* backend_dcb, DCB* backend_dcb,
error_action_t action, error_action_t action,
bool* succp) bool* succp)
{ {
SESSION* session; SESSION* session;
ROUTER_INSTANCE* inst = (ROUTER_INSTANCE *)instance; ROUTER_INSTANCE* inst = (ROUTER_INSTANCE *)instance;
ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *)router_session; ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *)router_session;
CHK_DCB(backend_dcb); CHK_DCB(backend_dcb);
/** Don't handle same error twice on same DCB */ /** Don't handle same error twice on same DCB */
if (backend_dcb->dcb_errhandle_called) if (backend_dcb->dcb_errhandle_called)
{ {
/** we optimistically assume that previous call succeed */ /** we optimistically assume that previous call succeed */
*succp = true; *succp = true;
return; return;
} }
else else
{ {
backend_dcb->dcb_errhandle_called = true; backend_dcb->dcb_errhandle_called = true;
} }
session = backend_dcb->session; session = backend_dcb->session;
if (session == NULL || rses == NULL) if (session == NULL || rses == NULL)
{ {
*succp = false; *succp = false;
return; }
} else
{
CHK_SESSION(session); CHK_SESSION(session);
CHK_CLIENT_RSES(rses); CHK_CLIENT_RSES(rses);
@ -4143,7 +4145,7 @@ static void handleError (
if (!rses_begin_locked_router_action(rses)) if (!rses_begin_locked_router_action(rses))
{ {
*succp = false; *succp = false;
return; break;
} }
/** /**
* This is called in hope of getting replacement for * This is called in hope of getting replacement for
@ -4171,6 +4173,8 @@ static void handleError (
*succp = false; *succp = false;
break; break;
} }
}
dcb_close(backend_dcb);
} }

View File

@ -63,6 +63,7 @@ extern __thread log_info_t tls_log_info;
* *
* Date Who Description * Date Who Description
* 20/01/2015 Markus Mäkelä/Vilho Raatikka Initial implementation * 20/01/2015 Markus Mäkelä/Vilho Raatikka Initial implementation
* 09/09/2015 Martin Brampton Modify error handler
* *
* @endverbatim * @endverbatim
*/ */
@ -2826,38 +2827,39 @@ handleError(
if(session == NULL || rses == NULL) if(session == NULL || rses == NULL)
{ {
if(succp)
*succp = false;
return;
}
CHK_SESSION(session);
CHK_CLIENT_RSES(rses);
switch(action)
{
case ERRACT_NEW_CONNECTION:
{
if(!rses_begin_locked_router_action(rses))
{
*succp = false;
return;
}
rses_end_locked_router_action(rses);
break;
}
case ERRACT_REPLY_CLIENT:
{
*succp = false; /*< no new backend servers were made available */
break;
}
default:
*succp = false; *succp = false;
break;
} }
else
{
CHK_SESSION(session);
CHK_CLIENT_RSES(rses);
switch(action)
{
case ERRACT_NEW_CONNECTION:
{
if(!rses_begin_locked_router_action(rses))
{
*succp = false;
break;
}
rses_end_locked_router_action(rses);
break;
}
case ERRACT_REPLY_CLIENT:
{
*succp = false; /*< no new backend servers were made available */
break;
}
default:
*succp = false;
break;
}
}
dcb_close(backend_dcb);
} }