From 3769a029578e62365aa561c787df0d7ee22f9742 Mon Sep 17 00:00:00 2001 From: vraatikka Date: Thu, 31 Oct 2013 22:24:51 +0200 Subject: [PATCH] query_classifier.cc resolve_query_type, added GSYSVAR_FUNC type for functions that read system variables and can be executed in Maxscale instead of backend server. dcb.c dcb_read, if read returns value <= 0 and if error is EAGAIN/EWOULDBLOCK so there was nothing to read in sthe socket, that is not an error because some other thread may have read the data that was expected to be available. mysql_backend.c gw_read_backend_event, used dcb->authlock to ensure that dcb's protocol state is read and modified serially. This removes issues with false authentication failures which may happen when two threads modify and read the state without any control. mysql_client.c removed dead code. readconnroute.c, readwritesplit.c removed invalid assert which assumed that spinlock can not have larger value than one when itis locked. --- query_classifier/query_classifier.cc | 5 +- server/core/dcb.c | 9 + server/modules/protocol/mysql_backend.c | 290 ++++++++++-------- server/modules/protocol/mysql_client.c | 7 - server/modules/routing/readconnroute.c | 1 - .../routing/readwritesplit/readwritesplit.c | 1 - 6 files changed, 174 insertions(+), 139 deletions(-) diff --git a/query_classifier/query_classifier.cc b/query_classifier/query_classifier.cc index 7682f5ecd..0ed5f4fbf 100644 --- a/query_classifier/query_classifier.cc +++ b/query_classifier/query_classifier.cc @@ -450,9 +450,9 @@ static skygw_query_type_t resolve_query_type( item->name, STRITEMTYPE(itype)); - if (item->type() == Item::SUBSELECT_ITEM) { + if (itype == Item::SUBSELECT_ITEM) { continue; - } else if (item->type() == Item::FUNC_ITEM) { + } else if (itype == Item::FUNC_ITEM) { skygw_query_type_t func_qtype = QUERY_TYPE_UNKNOWN; /** @@ -526,6 +526,7 @@ static skygw_query_type_t resolve_query_type( pthread_self()); break; case Item_func::NOW_FUNC: + case Item_func::GSYSVAR_FUNC: func_qtype = QUERY_TYPE_LOCAL_READ; skygw_log_write( LOGFILE_DEBUG, diff --git a/server/core/dcb.c b/server/core/dcb.c index 6fef729b5..ec3ec7e53 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -608,6 +608,15 @@ int eno = 0; eno, strerror(eno)); } + else + { + /** + * If read would block it means that other thread + * has probably read the data. + */ + n = 0; + } + gwbuf_free(buffer); goto return_n; } diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index db3a598c3..01f1ac435 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -144,14 +144,14 @@ static int gw_read_backend_event(DCB *dcb) { CHK_DCB(dcb); CHK_SESSION(dcb->session); - - backend_protocol = (MySQLProtocol *) dcb->protocol; - CHK_PROTOCOL(backend_protocol); - + /** return only with complete session */ current_session = gw_get_shared_session_auth_info(dcb); ss_dassert(current_session != NULL); + backend_protocol = (MySQLProtocol *) dcb->protocol; + CHK_PROTOCOL(backend_protocol); + skygw_log_write( LOGFILE_DEBUG, "%lu [gw_read_backend_event] Read dcb %p fd %d protocol " @@ -161,31 +161,48 @@ static int gw_read_backend_event(DCB *dcb) { dcb->fd, backend_protocol->state, STRPROTOCOLSTATE(backend_protocol->state)); - + + /* backend is connected: * * 1. read server handshake * 2. if (success) write auth request * 3. and return */ - if (backend_protocol->state == MYSQL_CONNECTED) { - if (gw_read_backend_handshake(backend_protocol) != 0) { - backend_protocol->state = MYSQL_AUTH_FAILED; - } else { - /* handshake decoded, send the auth credentials */ - if (gw_send_authentication_to_backend( - current_session->db, - current_session->user, - current_session->client_sha1, - backend_protocol) != 0) - { + /** + * If starting to auhenticate with backend server, lock dcb + * to prevent overlapping processing of auth messages. + */ + if (backend_protocol->state == MYSQL_CONNECTED) { + + spinlock_acquire(&dcb->authlock); + + backend_protocol = (MySQLProtocol *) dcb->protocol; + CHK_PROTOCOL(backend_protocol); + + if (backend_protocol->state == MYSQL_CONNECTED) { + + if (gw_read_backend_handshake(backend_protocol) != 0) { backend_protocol->state = MYSQL_AUTH_FAILED; - } else { - backend_protocol->state = MYSQL_AUTH_RECV; + } else { + /* handshake decoded, send the auth credentials */ + if (gw_send_authentication_to_backend( + current_session->db, + current_session->user, + current_session->client_sha1, + backend_protocol) != 0) + { + backend_protocol->state = MYSQL_AUTH_FAILED; + } else { + backend_protocol->state = MYSQL_AUTH_RECV; + } } - } + } + spinlock_release(&dcb->authlock); } + + /* * Now: * -- check the authentication reply from backend @@ -195,123 +212,136 @@ static int gw_read_backend_event(DCB *dcb) { if (backend_protocol->state == MYSQL_AUTH_RECV || backend_protocol->state == MYSQL_AUTH_FAILED) { - ROUTER_OBJECT *router = NULL; - ROUTER *router_instance = NULL; - void *rsession = NULL; - SESSION *session = dcb->session; - int receive_rc = 0; + spinlock_acquire(&dcb->authlock); - CHK_SESSION(session); + backend_protocol = (MySQLProtocol *) dcb->protocol; + CHK_PROTOCOL(backend_protocol); + + if (backend_protocol->state == MYSQL_AUTH_RECV || + backend_protocol->state == MYSQL_AUTH_FAILED) + { + ROUTER_OBJECT *router = NULL; + ROUTER *router_instance = NULL; + void *rsession = NULL; + SESSION *session = dcb->session; + int receive_rc = 0; + + CHK_SESSION(session); - router = session->service->router; - router_instance = session->service->router_instance; + router = session->service->router; + router_instance = session->service->router_instance; - if (backend_protocol->state == MYSQL_AUTH_RECV) { - /** - * Read backed auth reply - */ - receive_rc = gw_receive_backend_auth(backend_protocol); + if (backend_protocol->state == MYSQL_AUTH_RECV) { + /** + * Read backed auth reply + */ + receive_rc = + gw_receive_backend_auth(backend_protocol); - switch (receive_rc) { - case -1: - backend_protocol->state = MYSQL_AUTH_FAILED; + switch (receive_rc) { + case -1: + backend_protocol->state = MYSQL_AUTH_FAILED; - skygw_log_write_flush( - LOGFILE_ERROR, - "Error : backend server didn't accept " - "authentication for user %s.", - current_session->user); - break; - case 1: - backend_protocol->state = MYSQL_IDLE; - skygw_log_write_flush( - LOGFILE_TRACE, - "%lu [gw_read_backend_event] " - "gw_receive_backend_auth succeed. dcb " - "%p fd %d, user %s.", - pthread_self(), - dcb, - dcb->fd, - current_session->user); - break; - default: - ss_dassert(receive_rc == 0); - skygw_log_write_flush( - LOGFILE_TRACE, - "%lu [gw_read_backend_event] " - "gw_receive_backend_auth read " - "successfully " - "nothing. dcb %p fd %d, user %s.", - pthread_self(), - dcb, - dcb->fd, - current_session->user); - rc = 0; - goto return_rc; - break; - } /* switch */ - } - - if (backend_protocol->state == MYSQL_AUTH_FAILED) { - /** vraa : errorHandle */ - /* check the delayq before the reply */ - spinlock_acquire(&dcb->authlock); - if (dcb->delayq != NULL) { - /* send an error to the client */ - mysql_send_custom_error( - dcb->session->client, - 1, - 0, - "Connection to backend lost."); - // consume all the delay queue - dcb->delayq = gwbuf_consume(dcb->delayq, - gwbuf_length( - dcb->delayq)); + skygw_log_write_flush( + LOGFILE_ERROR, + "Error : backend server didn't " + "accept authentication for user " + "%s.", + current_session->user); + break; + case 1: + backend_protocol->state = MYSQL_IDLE; + + skygw_log_write_flush( + LOGFILE_TRACE, + "%lu [gw_read_backend_event] " + "gw_receive_backend_auth succeed. " + "dcb %p fd %d, user %s.", + pthread_self(), + dcb, + dcb->fd, + current_session->user); + break; + default: + ss_dassert(receive_rc == 0); + skygw_log_write_flush( + LOGFILE_TRACE, + "%lu [gw_read_backend_event] " + "gw_receive_backend_auth read " + "successfully " + "nothing. dcb %p fd %d, user %s.", + pthread_self(), + dcb, + dcb->fd, + current_session->user); + rc = 0; + goto return_with_lock; + break; + } /* switch */ } - spinlock_release(&dcb->authlock); - rsession = session->router_session; - ss_dassert(rsession != NULL); - /** - * vraa : errorHandle - * rsession should never be NULL here. - **/ - ss_dassert(rsession != NULL); - skygw_log_write_flush( - LOGFILE_DEBUG, - "%lu [gw_read_backend_event] " + if (backend_protocol->state == MYSQL_AUTH_FAILED) { + /** + * vraa : errorHandle + * check the delayq before the reply + */ + if (dcb->delayq != NULL) { + /* send an error to the client */ + mysql_send_custom_error( + dcb->session->client, + 1, + 0, + "Connection to backend lost."); + // consume all the delay queue + dcb->delayq = gwbuf_consume( + dcb->delayq, + gwbuf_length(dcb->delayq)); + } + rsession = session->router_session; + ss_dassert(rsession != NULL); + /** + * vraa : errorHandle + * rsession should never be NULL here. + **/ + ss_dassert(rsession != NULL); + + skygw_log_write_flush( + LOGFILE_DEBUG, + "%lu [gw_read_backend_event] " "Call closeSession for backend's " - "router client session.", - pthread_self()); - /* close router_session */ - router->closeSession(router_instance, rsession); - rc = 1; - goto return_rc; - } else { - ss_dassert(backend_protocol->state == MYSQL_IDLE); - skygw_log_write_flush( - LOGFILE_DEBUG, - "%lu [gw_read_backend_event] " - "gw_receive_backend_auth succeed. Fd %d, " - "user %s.", - pthread_self(), - dcb->fd, - current_session->user); - - spinlock_acquire(&dcb->authlock); - /* check the delay queue and flush the data */ - if(dcb->delayq) { - backend_write_delayqueue(dcb); - spinlock_release(&dcb->authlock); + "router client session.", + pthread_self()); + /* close router_session */ + router->closeSession(router_instance, rsession); rc = 1; - goto return_rc; + goto return_with_lock; } - spinlock_release(&dcb->authlock); - rc = 0; - goto return_rc; - } /* MYSQL_AUTH_FAILED */ - } /* MYSQL_AUTH_RECV || MYSQL_AUTH_FAILED */ + else + { + ss_dassert(backend_protocol->state == MYSQL_IDLE); + skygw_log_write_flush( + LOGFILE_DEBUG, + "%lu [gw_read_backend_event] " + "gw_receive_backend_auth succeed. Fd %d, " + "user %s.", + pthread_self(), + dcb->fd, + current_session->user); + /* check the delay queue and flush the data */ + if (dcb->delayq) + { + backend_write_delayqueue(dcb); + rc = 1; + goto return_with_lock; + } + } + } /* MYSQL_AUTH_RECV || MYSQL_AUTH_FAILED */ + + spinlock_release(&dcb->authlock); + + } /* MYSQL_AUTH_RECV || MYSQL_AUTH_FAILED */ + /* reading MySQL command output from backend and writing to the client */ { GWBUF *writebuf = NULL; @@ -377,6 +407,10 @@ static int gw_read_backend_event(DCB *dcb) { return_rc: return rc; + +return_with_lock: + spinlock_release(&dcb->authlock); + goto return_rc; } /* @@ -473,8 +507,8 @@ gw_MySQLWrite_backend(DCB *dcb, GWBUF *queue) /** Free buffer memory */ gwbuf_consume(queue, GWBUF_LENGTH(queue)); - skygw_log_write_flush( - LOGFILE_ERROR, + skygw_log_write( + LOGFILE_TRACE, "%lu [gw_MySQLWrite_backend] Write to backend failed. " "Backend dcb %p fd %d is %s.", pthread_self(), @@ -569,7 +603,7 @@ static int gw_error_backend_event(DCB *dcb) { */ skygw_log_write_flush( LOGFILE_TRACE, - "%lu [gw_read_backend_event] " + "%lu [gw_error_backend_event] " "Call closeSession for backend " "session.", pthread_self()); diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index c7a915aa0..9b4f04b52 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -983,13 +983,6 @@ int gw_MySQLAccept(DCB *listener) c_sock); conn_open[c_sock] = true; #endif - /* - fprintf(stderr, - "Processing %i connection fd %i for listener %i\n", - listener->stats.n_accepts, - c_sock, - listener->fd); - */ /* set nonblocking */ setsockopt(c_sock, SOL_SOCKET, SO_SNDBUF, &sendbuf, optlen); setnonblocking(c_sock); diff --git a/server/modules/routing/readconnroute.c b/server/modules/routing/readconnroute.c index dd0b5968f..cd668c8f7 100644 --- a/server/modules/routing/readconnroute.c +++ b/server/modules/routing/readconnroute.c @@ -732,6 +732,5 @@ static void rses_exit_router_action( ROUTER_CLIENT_SES* rses) { CHK_CLIENT_RSES(rses); - ss_dassert(((SPINLOCK)rses->rses_lock).lock == 1); spinlock_release(&rses->rses_lock); } diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index d13831937..a6b6bb7e4 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -707,7 +707,6 @@ static void rses_exit_router_action( ROUTER_CLIENT_SES* rses) { CHK_CLIENT_RSES(rses); - ss_dassert(((SPINLOCK)rses->rses_lock).lock == 1); spinlock_release(&rses->rses_lock); }