From b48bb4fc5e403575c1fb1ff1c032fa919b1d3010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 15 Jun 2017 20:30:11 +0300 Subject: [PATCH] Refactor auxiliary routing functions Refactored some of the functions used to calculate servers. Removed redundant checks and moved the ack_write() call to the right place. --- .../routing/readwritesplit/readwritesplit.cc | 54 ++++++++----------- .../readwritesplit/rwsplit_internal.hh | 2 +- .../readwritesplit/rwsplit_select_backends.cc | 6 +-- 3 files changed, 26 insertions(+), 36 deletions(-) diff --git a/server/modules/routing/readwritesplit/readwritesplit.cc b/server/modules/routing/readwritesplit/readwritesplit.cc index d0062eec0..b53b454ad 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.cc +++ b/server/modules/routing/readwritesplit/readwritesplit.cc @@ -102,11 +102,10 @@ static const MXS_ENUM_VALUE master_failure_mode_values[] = * @param rses Router client session * @param router_nservers The number of backend servers in total */ -int rses_get_max_slavecount(ROUTER_CLIENT_SES *rses, - int router_nservers) +int rses_get_max_slavecount(ROUTER_CLIENT_SES *rses) { int conf_max_nslaves; - int max_nslaves; + int router_nservers = rses->rses_nbackends; CHK_CLIENT_RSES(rses); @@ -118,9 +117,8 @@ int rses_get_max_slavecount(ROUTER_CLIENT_SES *rses, { conf_max_nslaves = (router_nservers * rses->rses_config.rw_max_slave_conn_percent) / 100; } - max_nslaves = MXS_MIN(router_nservers - 1, MXS_MAX(1, conf_max_nslaves)); - return max_nslaves; + return MXS_MIN(router_nservers - 1, MXS_MAX(1, conf_max_nslaves)); } /* @@ -474,7 +472,7 @@ static bool handle_error_new_connection(ROUTER_INSTANCE *inst, /** Close the current connection */ bref->close(); - int max_nslaves = rses_get_max_slavecount(myrses, myrses->rses_nbackends); + int max_nslaves = rses_get_max_slavecount(myrses); bool succp; /** * Try to get replacement slave or at least the minimum @@ -796,10 +794,11 @@ static MXS_ROUTER_SESSION *newSession(MXS_ROUTER *router_inst, MXS_SESSION *sess } } - int max_nslaves = rses_get_max_slavecount(client_rses, router_nservers); - client_rses->rses_nbackends = router_nservers; /*< # of backend servers */ + int max_nslaves = rses_get_max_slavecount(client_rses); + + if (!select_connect_backend_servers(router_nservers, max_nslaves, client_rses->rses_config.slave_selection_criteria, session, router, client_rses, false)) @@ -1122,7 +1121,8 @@ static void clientReply(MXS_ROUTER *instance, if (reply_is_complete(bref, writebuf)) { - /** Got a complete reply, decrement expected response count */ + /** Got a complete reply, acknowledge the write decrement expected response count */ + bref->ack_write(); router_cli_ses->expected_responses--; ss_dassert(router_cli_ses->expected_responses >= 0); ss_dassert(bref->get_reply_state() == REPLY_STATE_DONE); @@ -1140,33 +1140,23 @@ static void clientReply(MXS_ROUTER *instance, { check_session_command_reply(writebuf, bref); - if (GWBUF_IS_TYPE_SESCMD_RESPONSE(writebuf)) - { - /** - * Discard all those responses that have already been sent to - * the client. Return with buffer including response that - * needs to be sent to client or NULL. - */ - bool rconn = false; - process_sescmd_response(router_cli_ses, bref, &writebuf, &rconn); + /** This discards all responses that have already been sent to the client */ + bool rconn = false; + process_sescmd_response(router_cli_ses, bref, &writebuf, &rconn); - if (rconn && !router_inst->rwsplit_config.disable_sescmd_history) - { - select_connect_backend_servers( - router_cli_ses->rses_nbackends, - router_cli_ses->rses_config.max_slave_connections, - router_cli_ses->rses_config.slave_selection_criteria, - router_cli_ses->client_dcb->session, - router_cli_ses->router, - router_cli_ses, - true); - } + if (rconn && !router_inst->rwsplit_config.disable_sescmd_history) + { + select_connect_backend_servers( + router_cli_ses->rses_nbackends, + router_cli_ses->rses_config.max_slave_connections, + router_cli_ses->rses_config.slave_selection_criteria, + router_cli_ses->client_dcb->session, + router_cli_ses->router, + router_cli_ses, + true); } } - /** Complete the write */ - bref->ack_write(); - bool queue_routed = false; if (router_cli_ses->expected_responses == 0) diff --git a/server/modules/routing/readwritesplit/rwsplit_internal.hh b/server/modules/routing/readwritesplit/rwsplit_internal.hh index 028af9ce2..e41dbb300 100644 --- a/server/modules/routing/readwritesplit/rwsplit_internal.hh +++ b/server/modules/routing/readwritesplit/rwsplit_internal.hh @@ -52,7 +52,7 @@ bool send_readonly_error(DCB *dcb); */ int router_handle_state_switch(DCB *dcb, DCB_REASON reason, void *data); SRWBackend& get_bref_from_dcb(ROUTER_CLIENT_SES *rses, DCB *dcb); -int rses_get_max_slavecount(ROUTER_CLIENT_SES *rses, int router_nservers); +int rses_get_max_slavecount(ROUTER_CLIENT_SES *rses); int rses_get_max_replication_lag(ROUTER_CLIENT_SES *rses); /* diff --git a/server/modules/routing/readwritesplit/rwsplit_select_backends.cc b/server/modules/routing/readwritesplit/rwsplit_select_backends.cc index 28e03b843..c24084220 100644 --- a/server/modules/routing/readwritesplit/rwsplit_select_backends.cc +++ b/server/modules/routing/readwritesplit/rwsplit_select_backends.cc @@ -79,8 +79,8 @@ static bool valid_for_slave(const SERVER *server, const SERVER *master_host) * @param cmpfun qsort() compatible comparison function * @return The best slave backend reference or NULL if no candidates could be found */ -SRWBackend* get_slave_candidate(ROUTER_CLIENT_SES* rses, const SERVER *master, - int (*cmpfun)(const void *, const void *)) +SRWBackend get_slave_candidate(ROUTER_CLIENT_SES* rses, const SERVER *master, + int (*cmpfun)(const void *, const void *)) { SRWBackend candidate; @@ -210,7 +210,7 @@ bool select_connect_backend_servers(int router_nservers, ss_dassert(slaves_connected < max_nslaves || max_nslaves == 0); /** Connect to all possible slaves */ - for (SRWBackend bref(get_slave_candidate(rses, router_nservers, master_host, cmpfun)); + for (SRWBackend bref(get_slave_candidate(rses, master_host, cmpfun)); bref && slaves_connected < max_nslaves; bref = get_slave_candidate(rses, master_host, cmpfun)) {