From c444bf454ba8dd031a2d18ed50b5b746066588a2 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Mon, 27 Jan 2014 22:56:33 +0200 Subject: [PATCH] Bug #385 http://bugs.skysql.com/show_bug.cgi?id=385 dcb.c:dcb_write accept also dcb state DCB_STATE_NOPOLLING since it only means that dcb has been removed from epoll set but it is still possible to write to it. Bug #384 http://bugs.skysql.com/show_bug.cgi?id=384 session.h:added new state for SESSION, SESSION_STATE_ROUTER_READY which follows SESSION_STATE_READY. The difference is that ROUTER_READY is set only after router session is successfully created while READY means that session still lacks router. session.c:set SESSION_STATE_ROUTER_READY when router is created. mysql_backend.c:gw_read_backend_event, added SESSION_STATE_ROUTER_READY check before router session is closed. Changed chec kso that it doesn't block in infinite loop (although it shouldn't be possible anyway). mysql_backend.c:gw_error_backend_event, added similar check before session is closed. --- query_classifier/query_classifier.cc | 5 +- server/core/dcb.c | 3 +- server/core/session.c | 3 +- server/include/session.h | 11 ++-- server/modules/protocol/mysql_backend.c | 66 ++++++++++++------- .../routing/readwritesplit/readwritesplit.c | 3 +- 6 files changed, 55 insertions(+), 36 deletions(-) diff --git a/query_classifier/query_classifier.cc b/query_classifier/query_classifier.cc index a2956de27..b86646ab1 100644 --- a/query_classifier/query_classifier.cc +++ b/query_classifier/query_classifier.cc @@ -585,9 +585,10 @@ static skygw_query_type_t resolve_query_type( LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, "%lu [resolve_query_type] " - "Unknown functype. Something " + "Unknown functype %d. Something " "has gone wrong.", - pthread_self()))); + pthread_self(), + ftype))); break; } /**< switch */ /**< Set new query type */ diff --git a/server/core/dcb.c b/server/core/dcb.c index 6de4fa1b7..2a86c04fa 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -659,7 +659,8 @@ dcb_write(DCB *dcb, GWBUF *queue) if (queue == NULL || (dcb->state != DCB_STATE_ALLOC && dcb->state != DCB_STATE_POLLING && - dcb->state != DCB_STATE_LISTENING)) + dcb->state != DCB_STATE_LISTENING && + dcb->state != DCB_STATE_NOPOLLING)) { return 0; } diff --git a/server/core/session.c b/server/core/session.c index 7f32fe937..062c63f27 100644 --- a/server/core/session.c +++ b/server/core/session.c @@ -105,7 +105,7 @@ session_alloc(SERVICE *service, DCB *client_dcb) session->refcount = 1; /*< * This indicates that session is ready to be shared with backend - * DCBs. + * DCBs. Note that this doesn't mean that router is initialized yet! */ session->state = SESSION_STATE_READY; @@ -145,6 +145,7 @@ session_alloc(SERVICE *service, DCB *client_dcb) } } spinlock_acquire(&session_spin); + session->state = SESSION_STATE_ROUTER_READY; session->next = allSessions; allSessions = session; spinlock_release(&session_spin); diff --git a/server/include/session.h b/server/include/session.h index ad00ed850..6cafa160d 100644 --- a/server/include/session.h +++ b/server/include/session.h @@ -50,11 +50,12 @@ typedef struct { } SESSION_STATS; typedef enum { - SESSION_STATE_ALLOC, - SESSION_STATE_READY, - SESSION_STATE_LISTENER, - SESSION_STATE_LISTENER_STOPPED, - SESSION_STATE_FREE + SESSION_STATE_ALLOC, /*< for all sessions */ + SESSION_STATE_READY, /*< for router session */ + SESSION_STATE_ROUTER_READY, /*< for router session */ + SESSION_STATE_LISTENER, /*< for listener session */ + SESSION_STATE_LISTENER_STOPPED, /*< for listener session */ + SESSION_STATE_FREE /*< for all sessions */ } session_state_t; /** diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index fc11484c7..a031648c7 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -299,18 +299,32 @@ static int gw_read_backend_event(DCB *dcb) { dcb->delayq, gwbuf_length(dcb->delayq)); } - ss_dassert(session->state == SESSION_READY); - /** - * vraa : errorHandle - * rsession may be NULL if session is being created - * in parallel by another thread. - */ - while(session->router_session == NULL) - { + + while (session->state != SESSION_STATE_ROUTER_READY) + { + ss_dassert( + session->state == SESSION_STATE_READY || + session->state == + SESSION_STATE_ROUTER_READY); + /** + * Session shouldn't be NULL at this point + * anymore. Just checking.. + */ + if (session->client->session == NULL) + { + rc = 1; + goto return_with_lock; + } usleep(1); } + /** + * rsession shouldn't be NULL since session + * state indicates that it was initialized + * successfully. + */ rsession = session->router_session; - + ss_dassert(rsession != NULL); + LOGIF(LD, (skygw_log_write_flush( LOGFILE_DEBUG, "%lu [gw_read_backend_event] " @@ -596,26 +610,28 @@ static int gw_error_backend_event(DCB *dcb) { rc = 1; } LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, + LOGFILE_DEBUG, "%lu [gw_error_backend_event] Some error occurred in backend. " "rc = %d", pthread_self(), rc))); - rsession = session->router_session; - ss_dassert(rsession != NULL); - /*< - * vraa : errorHandle - * rsession should never be NULL here. - */ - LOGIF(LD, (skygw_log_write_flush( - LOGFILE_DEBUG, - "%lu [gw_error_backend_event] " - "Call closeSession for backend " - "session.", - pthread_self()))); - - router->closeSession(router_instance, rsession); - + + if (session->state == SESSION_STATE_ROUTER_READY) + { + rsession = session->router_session; + /*< + * rsession should never be NULL here. + */ + ss_dassert(rsession != NULL); + LOGIF(LD, (skygw_log_write_flush( + LOGFILE_DEBUG, + "%lu [gw_error_backend_event] " + "Call closeSession for backend " + "session.", + pthread_self()))); + + router->closeSession(router_instance, rsession); + } return rc; } diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 9eac3a294..b6ed199ff 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -315,7 +315,6 @@ static void* newSession( client_rses->master_dcb = dcb_connect(be_master->backend_server, session, be_master->backend_server->protocol); - if (client_rses->master_dcb == NULL) { /** Close slave connection first. */ @@ -543,7 +542,7 @@ static int routeQuery( if (rses_is_closed || (master_dcb == NULL && slave_dcb == NULL)) { - LOGIF(LE, (skygw_log_write( + LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error: Failed to route %s:%s:\"%s\" to backend server. " "%s.",