From dbfd631fedd3a3671b6895c78eac99deb89adae9 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Tue, 16 May 2017 13:24:00 +0300 Subject: [PATCH] Change session registry to a template class The template class wraps a HashMap such that only a few operations are allowed. Usage requires specializing a RegistryTraits class template for each entry type. --- include/maxscale/utils.hh | 91 +++++++++++++++++++ include/maxscale/worker.h | 2 +- server/core/maxscale/session.hh | 39 ++++++++ server/core/maxscale/worker.hh | 28 ++---- server/core/session.cc | 2 +- server/core/worker.cc | 45 ++------- .../protocol/MySQL/MySQLClient/mysql_client.c | 4 +- 7 files changed, 148 insertions(+), 63 deletions(-) create mode 100644 server/core/maxscale/session.hh diff --git a/include/maxscale/utils.hh b/include/maxscale/utils.hh index a5a508f88..631522dcf 100644 --- a/include/maxscale/utils.hh +++ b/include/maxscale/utils.hh @@ -14,6 +14,7 @@ #include #include +#include namespace maxscale { @@ -177,4 +178,94 @@ struct CloserTraits } }; +/* Helper type for Registry. Must be specialized for each EntryType. The types + * listed below are just examples and will not compile. */ +template +struct RegistryTraits +{ + typedef int id_type; + typedef EntryType* entry_type; + + static id_type get_id(entry_type entry) + { + static_assert(sizeof(EntryType) != sizeof(EntryType), "get_id() and the" + " surrounding struct must be specialized for every EntryType!"); + return 0; + } + static entry_type null_entry() + { + return NULL; + } +}; + +/** + * Class Registy wraps a map, allowing only a few operations on it. The intended + * use is simple registries, such as the session registry in Worker. The owner + * can expose a reference to this class without exposing all the methods the + * underlying container implements. When instantiating with a new EntryType, the + * traits-class RegistryTraits should be specialized for the new type as well. + */ +template +class Registry +{ + Registry(const Registry&); + Registry& operator = (const Registry&); +public: + typedef typename RegistryTraits::id_type id_type; + typedef typename RegistryTraits::entry_type entry_type; + + Registry() + { + } + /** + * Find an entry in the registry. + * + * @param id Entry key + * @return The found entry, or NULL if not found + */ + entry_type lookup(id_type id) const + { + entry_type rval = RegistryTraits::null_entry(); + typename ContainerType::const_iterator iter = m_registry.find(id); + if (iter != m_registry.end()) + { + rval = iter->second; + } + return rval; + } + + /** + * Add an entry to the registry. + * + * @param entry The entry to add + * @return True if successful, false if id was already in + */ + bool add(entry_type entry) + { + id_type id = RegistryTraits::get_id(entry); + typename ContainerType::value_type new_value(id, entry); + return m_registry.insert(new_value).second; + } + + /** + * Remove an entry from the registry. + * + * @param id Entry id + * @return True if an entry was removed, false if not found + */ + bool remove(id_type id) + { + entry_type rval = lookup(id); + if (rval) + { + m_registry.erase(id); + } + return rval; + } + +private: + typedef typename std::tr1::unordered_map ContainerType; + ContainerType m_registry; +}; + } diff --git a/include/maxscale/worker.h b/include/maxscale/worker.h index f2a6d6f9f..484abd4f4 100644 --- a/include/maxscale/worker.h +++ b/include/maxscale/worker.h @@ -133,7 +133,7 @@ bool mxs_worker_register_session(MXS_SESSION* session); * @param id Which id to remove. * @return The removed session or NULL if not found. */ -MXS_SESSION* mxs_worker_deregister_session(uint64_t id); +bool mxs_worker_deregister_session(uint64_t id); /** * Find a session in the current worker's session container. diff --git a/server/core/maxscale/session.hh b/server/core/maxscale/session.hh new file mode 100644 index 000000000..f5e0b48e0 --- /dev/null +++ b/server/core/maxscale/session.hh @@ -0,0 +1,39 @@ +#pragma once +/* + * Copyright (c) 2016 MariaDB Corporation Ab + * + * Use of this software is governed by the Business Source License included + * in the LICENSE.TXT file and at www.mariadb.com/bsl11. + * + * Change Date: 2019-07-01 + * + * On the date above, in accordance with the Business Source License, use + * of this software will be governed by version 2 or later of the General + * Public License. + */ + +#include +#include + +namespace maxscale +{ +/** + * Specialization of RegistryTraits for the session registry. + */ +template<> +struct RegistryTraits +{ + typedef uint64_t id_type; + typedef MXS_SESSION* entry_type; + + static id_type get_id(entry_type entry) + { + return entry->ses_id; + } + static entry_type null_entry() + { + return NULL; + } +}; + +} diff --git a/server/core/maxscale/worker.hh b/server/core/maxscale/worker.hh index 35ff8ebd2..98ab353f3 100644 --- a/server/core/maxscale/worker.hh +++ b/server/core/maxscale/worker.hh @@ -14,13 +14,14 @@ #include #include -#include #include #include +#include #include "messagequeue.hh" #include "poll.h" #include "worker.h" #include "workertask.hh" +#include "session.hh" namespace maxscale { @@ -69,7 +70,7 @@ public: typedef WORKER_STATISTICS STATISTICS; typedef WorkerTask Task; typedef WorkerDisposableTask DisposableTask; - typedef std::tr1::unordered_map SessionsById; + typedef Registry SessionsById; enum state_t { @@ -397,28 +398,11 @@ public: bool post_message(uint32_t msg_id, intptr_t arg1, intptr_t arg2); /** - * Add a session to the session container. + * Return a reference to the session registry of this worker. * - * @param session The session to add - * @return true if successful + * @return Session registry. */ - bool register_session(MXS_SESSION* session); - - /** - * Remove a session from the session container. - * - * @param id Session id - * @return The removed session, or NULL if not found - */ - MXS_SESSION* deregister_session(uint64_t id); - - /** - * Find a session in the session container. - * - * @param id Session id - * @return The found session, or NULL if not found - */ - MXS_SESSION* find_session(uint64_t id); + SessionsById& session_registry(); /** * Broadcast a message to all worker. diff --git a/server/core/session.cc b/server/core/session.cc index 61d48bbe5..a5693c1ec 100644 --- a/server/core/session.cc +++ b/server/core/session.cc @@ -109,7 +109,7 @@ public: void execute(maxscale::Worker& worker) { - MXS_SESSION* target = worker.find_session(m_target_id); + MXS_SESSION* target = worker.session_registry().lookup(m_target_id); if (target && issuer_can_kill_target(m_issuer_user, m_issuer_host, target)) { poll_fake_hangup_event(target->client_dcb); diff --git a/server/core/worker.cc b/server/core/worker.cc index 1c8840103..fe27244de 100644 --- a/server/core/worker.cc +++ b/server/core/worker.cc @@ -728,55 +728,26 @@ bool mxs_worker_register_session(MXS_SESSION* session) { Worker* worker = Worker::get_current(); ss_dassert(worker); - return worker->register_session(session); + return worker->session_registry().add(session); } -MXS_SESSION* mxs_worker_deregister_session(uint64_t id) +bool mxs_worker_deregister_session(uint64_t id) { - MXS_SESSION* rval = NULL; Worker* worker = Worker::get_current(); - if (worker) - { - rval = worker->deregister_session(id); - } - return rval; + ss_dassert(worker); + return worker->session_registry().remove(id); } MXS_SESSION* mxs_worker_find_session(uint64_t id) { - MXS_SESSION* rval = NULL; Worker* worker = Worker::get_current(); - if (worker) - { - rval = worker->find_session(id); - } - return rval; + ss_dassert(worker); + return worker->session_registry().lookup(id); } -bool Worker::register_session(MXS_SESSION* session) +Worker::SessionsById& Worker::session_registry() { - return m_sessions.insert(SessionsById::value_type(session->ses_id, session)).second; -} - -MXS_SESSION* Worker::deregister_session(uint64_t id) -{ - MXS_SESSION* rval = find_session(id); - if (rval) - { - m_sessions.erase(id); - } - return rval; -} - -MXS_SESSION* Worker::find_session(uint64_t id) -{ - MXS_SESSION* rval = NULL; - SessionsById::const_iterator iter = m_sessions.find(id); - if (iter != m_sessions.end()) - { - rval = iter->second; - } - return rval; + return m_sessions; } class WorkerInfoTask: public maxscale::WorkerTask diff --git a/server/modules/protocol/MySQL/MySQLClient/mysql_client.c b/server/modules/protocol/MySQL/MySQLClient/mysql_client.c index e1fd3e802..1ba2954ac 100644 --- a/server/modules/protocol/MySQL/MySQLClient/mysql_client.c +++ b/server/modules/protocol/MySQL/MySQLClient/mysql_client.c @@ -1305,8 +1305,8 @@ static int gw_client_close(DCB *dcb) { ss_dassert(target->state == SESSION_STATE_ROUTER_READY || target->state == SESSION_STATE_STOPPING); - ss_debug(MXS_SESSION* removed =) mxs_worker_deregister_session(target->ses_id); - ss_dassert(removed == target); + ss_debug(bool removed =) mxs_worker_deregister_session(target->ses_id); + ss_dassert(removed); session_close(target); } }