From 9be2212d96f3dde155e4982014e3165057223b14 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Sat, 3 Jan 2015 01:21:30 +0200 Subject: [PATCH 1/2] Fix to bug #676, http://bugs.mariadb.com/show_bug.cgi?id=676 readwritesplit.c:route_session_write failed if the last backend on all backends list was not in use. THe situation where not all backends are used by routing session is normal especially if max_slave_connections is not set to 100%. Thus session commands may have failed if user was bit unlucky. Changed the logic so that the function fails (and session is closed) if routing fails to any such backend which is in use in the session. --- .../routing/readwritesplit/readwritesplit.c | 60 ++++++++++++------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 751d251d1..165814fec 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -4061,6 +4061,15 @@ return_rc: * The first OK packet is replied to the client. * Return true if succeed, false is returned if router session was closed or * if execute_sescmd_in_backend failed. + * + * @param router_cli_ses Client's router session pointer + * @param querybuf GWBUF including the query to be routed + * @param inst Router instance + * @param packet_type Type of MySQL packet + * @param qtype Query type from query_classifier + * + * @return True if routing succeed to all backends being used, otherwise false. + * */ static bool route_session_write( ROUTER_CLIENT_SES* router_cli_ses, @@ -4073,11 +4082,18 @@ static bool route_session_write( rses_property_t* prop; backend_ref_t* backend_ref; int i; + int max_nslaves; + int nbackends; + int nsucc; LOGIF(LT, (skygw_log_write( LOGFILE_TRACE, "Session write, routing to all servers."))); - + /** Maximum number of slaves in this router client session */ + max_nslaves = rses_get_max_slavecount(router_cli_ses, + router_cli_ses->rses_nbackends); + nsucc = 0; + nbackends = 0; backend_ref = router_cli_ses->rses_backend_ref; /** @@ -4091,10 +4107,8 @@ static bool route_session_write( packet_type == MYSQL_COM_STMT_CLOSE) { int rc; - - succp = true; - - /** Lock router session */ + + /** Lock router session */ if (!rses_begin_locked_router_action(router_cli_ses)) { succp = false; @@ -4119,12 +4133,11 @@ static bool route_session_write( if (BREF_IS_IN_USE((&backend_ref[i]))) { - rc = dcb->func.write(dcb, gwbuf_clone(querybuf)); - - if (rc != 1) - { - succp = false; - } + nbackends += 1; + if ((rc = dcb->func.write(dcb, gwbuf_clone(querybuf))) == 1) + { + nsucc += 1; + } } } rses_end_locked_router_action(router_cli_ses); @@ -4160,6 +4173,8 @@ static bool route_session_write( { sescmd_cursor_t* scur; + nbackends += 1; + if (LOG_IS_ENABLED(LOGFILE_TRACE)) { LOGIF(LT, (skygw_log_write( @@ -4187,8 +4202,7 @@ static bool route_session_write( */ if (sescmd_cursor_is_active(scur)) { - succp = true; - + nsucc += 1; LOGIF(LT, (skygw_log_write( LOGFILE_TRACE, "Backend %s:%d already executing sescmd.", @@ -4197,10 +4211,12 @@ static bool route_session_write( } else { - succp = execute_sescmd_in_backend(&backend_ref[i]); - - if (!succp) - { + if (execute_sescmd_in_backend(&backend_ref[i])) + { + nsucc += 1; + } + else + { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error : Failed to execute session " @@ -4210,18 +4226,20 @@ static bool route_session_write( } } } - else - { - succp = false; - } } /** Unlock router session */ rses_end_locked_router_action(router_cli_ses); return_succp: + /** + * Routing must succeed to all backends that are used. + * There must be at most max_nslaves+1 backends. + */ + succp = (nsucc == nbackends && nbackends <= max_nslaves+1); return succp; } + #if defined(NOT_USED) static bool router_option_configured( From 2d3491e12307038e08205ddc1106199670970408 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Sat, 3 Jan 2015 01:47:11 +0200 Subject: [PATCH 2/2] Additional fix to bug #676 readwritesplit.c:route_session_write: added check that at least one backend is being used. --- .../routing/readwritesplit/readwritesplit.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 165814fec..e9000a71f 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -4059,8 +4059,6 @@ return_rc: * Suppress redundant OK packets sent by backends. * * The first OK packet is replied to the client. - * Return true if succeed, false is returned if router session was closed or - * if execute_sescmd_in_backend failed. * * @param router_cli_ses Client's router session pointer * @param querybuf GWBUF including the query to be routed @@ -4068,7 +4066,8 @@ return_rc: * @param packet_type Type of MySQL packet * @param qtype Query type from query_classifier * - * @return True if routing succeed to all backends being used, otherwise false. + * @return True if at least one backend is used and routing succeed to all + * backends being used, otherwise false. * */ static bool route_session_write( @@ -4111,7 +4110,6 @@ static bool route_session_write( /** Lock router session */ if (!rses_begin_locked_router_action(router_cli_ses)) { - succp = false; goto return_succp; } @@ -4147,13 +4145,16 @@ static bool route_session_write( /** Lock router session */ if (!rses_begin_locked_router_action(router_cli_ses)) { - succp = false; goto return_succp; } if (router_cli_ses->rses_nbackends <= 0) { - succp = false; + LOGIF(LT, (skygw_log_write( + LOGFILE_TRACE, + "Router session doesn't have any backends in use. " + "Routing failed. <"))); + goto return_succp; } /** @@ -4233,9 +4234,9 @@ static bool route_session_write( return_succp: /** * Routing must succeed to all backends that are used. - * There must be at most max_nslaves+1 backends. + * There must be at leas one and at most max_nslaves+1 backends. */ - succp = (nsucc == nbackends && nbackends <= max_nslaves+1); + succp = (nbackends > 0 && nsucc == nbackends && nbackends <= max_nslaves+1); return succp; }