From d3a34712dfdf689948763bd5f012c728aced2c9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Thu, 20 Jul 2023 14:04:44 +0200 Subject: [PATCH] Improve function resource acquisition readability - Improved readability for the locked map acquisition for function 'search_rules_fast_routing_dest_hg'. - Improved function documentation and added note about usage. --- include/query_processor.h | 15 ++++++++++++++- lib/Query_Processor.cpp | 11 +++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/include/query_processor.h b/include/query_processor.h index 5fd659a9a..c9dd826a3 100644 --- a/include/query_processor.h +++ b/include/query_processor.h @@ -387,8 +387,21 @@ class Query_Processor { * @return Old 'fast_routing_resultset' that has been replaced. Required to be freed by caller. */ SQLite3_result* load_fast_routing(const fast_routing_hashmap_t& fast_routing_hashmap); + /** + * @brief Searches for a matching rule in the supplied map, returning the destination hostgroup. + * @details This functions takes a pointer to the hashmap pointer. This is because it performs a + * conditional internal locking of member 'rwlock'. Since the original pointer value could be modified + * after the function call, we must perform the resource acquisition (dereference) after we have + * acquired the internal locking. + * @param khStrInt The map to be used for performing the search. See @details. + * @param u Username, used for the search as part of the map key. + * @param s Schemaname, used for the search as part of the map key. + * @param flagIN FlagIn, used for the search as part of the map key. + * @param lock Whether or not the member lock 'rwlock' should be taken for the search. + * @return If a matching rule is found, the target destination hostgroup, -1 otherwise. + */ int search_rules_fast_routing_dest_hg( - khash_t(khStrInt)*& _rules_fast_routing, const char* u, const char* s, int flagIN, bool lock + khash_t(khStrInt)** __rules_fast_routing, const char* u, const char* s, int flagIN, bool lock ); SQLite3_result * get_current_query_rules_fast_routing(); SQLite3_result * get_current_query_rules_fast_routing_inner(); diff --git a/lib/Query_Processor.cpp b/lib/Query_Processor.cpp index 998947659..fbe289da2 100644 --- a/lib/Query_Processor.cpp +++ b/lib/Query_Processor.cpp @@ -981,7 +981,7 @@ SQLite3_result * Query_Processor::get_current_query_rules_fast_routing() { } int Query_Processor::search_rules_fast_routing_dest_hg( - khash_t(khStrInt)*& _rules_fast_routing, const char* u, const char* s, int flagIN, bool lock + khash_t(khStrInt)** __rules_fast_routing, const char* u, const char* s, int flagIN, bool lock ) { int dest_hg = -1; const size_t u_len = strlen(u); @@ -998,6 +998,7 @@ int Query_Processor::search_rules_fast_routing_dest_hg( if (lock) { pthread_rwlock_rdlock(&this->rwlock); } + khash_t(khStrInt)* _rules_fast_routing = *__rules_fast_routing; khiter_t k = kh_get(khStrInt, _rules_fast_routing, keybuf_ptr); if (k == kh_end(_rules_fast_routing)) { khiter_t k2 = kh_get(khStrInt, _rules_fast_routing, keybuf_ptr + u_len); @@ -2133,10 +2134,12 @@ __exit_process_mysql_query: if (_thr_SQP_rules_fast_routing != nullptr) { proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 7, "Searching thread-local 'rules_fast_routing' hashmap with: user='%s', schema='%s', and flagIN='%d'\n", u, s, flagIN); - dst_hg = search_rules_fast_routing_dest_hg(_thr_SQP_rules_fast_routing, u, s, flagIN, false); + dst_hg = search_rules_fast_routing_dest_hg(&_thr_SQP_rules_fast_routing, u, s, flagIN, false); } else if (rules_fast_routing != nullptr) { proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 7, "Searching global 'rules_fast_routing' hashmap with: user='%s', schema='%s', and flagIN='%d'\n", u, s, flagIN); - dst_hg = search_rules_fast_routing_dest_hg(rules_fast_routing, u, s, flagIN, true); + // NOTE: A pointer to the member 'this->rules_fast_routing' is required, since the value of the + // member could have changed before the function acquires the internal lock. See function doc. + dst_hg = search_rules_fast_routing_dest_hg(&this->rules_fast_routing, u, s, flagIN, true); } if (dst_hg != -1) { @@ -3263,7 +3266,7 @@ int Query_Processor::testing___find_HG_in_mysql_query_rules_fast_routing_dual( khash_t(khStrInt)* rules_fast_routing = _rules_fast_routing ? _rules_fast_routing : this->rules_fast_routing; if (rules_fast_routing) { - ret = search_rules_fast_routing_dest_hg(rules_fast_routing, username, schemaname, flagIN, lock); + ret = search_rules_fast_routing_dest_hg(&rules_fast_routing, username, schemaname, flagIN, lock); } return ret;