From ee97e6f4b33a74747699827b26f786fa5391f00b Mon Sep 17 00:00:00 2001 From: obdev Date: Mon, 3 Apr 2023 20:11:34 +0000 Subject: [PATCH] fix user connection count bug --- src/observer/mysql/obmp_change_user.cpp | 8 +-- src/sql/session/ob_sql_session_info.cpp | 4 +- src/sql/session/ob_sql_session_info.h | 12 +++- src/sql/session/ob_sql_session_mgr.cpp | 3 +- src/sql/session/ob_user_resource_mgr.cpp | 92 +++++++++++++----------- src/sql/session/ob_user_resource_mgr.h | 5 +- 6 files changed, 70 insertions(+), 54 deletions(-) diff --git a/src/observer/mysql/obmp_change_user.cpp b/src/observer/mysql/obmp_change_user.cpp index 5e6e21a85a..8d27a9a2e4 100644 --- a/src/observer/mysql/obmp_change_user.cpp +++ b/src/observer/mysql/obmp_change_user.cpp @@ -308,11 +308,9 @@ int ObMPChangeUser::load_privilege_info(ObSQLSessionInfo *session) SSL *ssl_st = SQL_REQ_OP.get_sql_ssl_st(req_); share::schema::ObSessionPrivInfo session_priv; - if (session->has_got_conn_res()) { - // disconnect previous user connection first. - if (OB_FAIL(session->on_user_disconnect())) { - LOG_WARN("user disconnect failed", K(ret)); - } + // disconnect previous user connection first. + if (OB_FAIL(session->on_user_disconnect())) { + LOG_WARN("user disconnect failed", K(ret)); } const ObUserInfo *user_info = NULL; if (OB_FAIL(ret)) { diff --git a/src/sql/session/ob_sql_session_info.cpp b/src/sql/session/ob_sql_session_info.cpp index d06eebdb70..3a4033a582 100644 --- a/src/sql/session/ob_sql_session_info.cpp +++ b/src/sql/session/ob_sql_session_info.cpp @@ -171,7 +171,9 @@ ObSQLSessionInfo::ObSQLSessionInfo() : is_load_data_exec_session_(false), pl_exact_err_msg_(), is_ps_prepare_stage_(false), - got_conn_res_(false), + got_tenant_conn_res_(false), + got_user_conn_res_(false), + conn_res_user_id_(OB_INVALID_ID), mem_context_(nullptr), cur_exec_ctx_(nullptr), restore_auto_commit_(false), diff --git a/src/sql/session/ob_sql_session_info.h b/src/sql/session/ob_sql_session_info.h index cf1bcc8e29..28cda705b9 100644 --- a/src/sql/session/ob_sql_session_info.h +++ b/src/sql/session/ob_sql_session_info.h @@ -995,8 +995,12 @@ public: void set_load_data_exec_session(bool v) { is_load_data_exec_session_ = v; } bool is_load_data_exec_session() const { return is_load_data_exec_session_; } inline ObSqlString &get_pl_exact_err_msg() { return pl_exact_err_msg_; } - void set_got_conn_res(bool v) { got_conn_res_ = v; } - bool has_got_conn_res() const { return got_conn_res_; } + void set_got_tenant_conn_res(bool v) { got_tenant_conn_res_ = v; } + bool has_got_tenant_conn_res() const { return got_tenant_conn_res_; } + void set_got_user_conn_res(bool v) { got_user_conn_res_ = v; } + bool has_got_user_conn_res() const { return got_user_conn_res_; } + void set_conn_res_user_id(uint64_t v) { conn_res_user_id_ = v; } + uint64_t get_conn_res_user_id() const { return conn_res_user_id_; } int on_user_connect(share::schema::ObSessionPrivInfo &priv_info, const ObUserInfo *user_info); int on_user_disconnect(); virtual void reset_tx_variable(); @@ -1143,7 +1147,9 @@ private: // It's used for on_user_disconnect. // No matter whether apply for resource successfully, a session will call on_user_disconnect when disconnect. // While only session got connection resource can release connection resource and decrease connections count. - bool got_conn_res_; + bool got_tenant_conn_res_; + bool got_user_conn_res_; + uint64_t conn_res_user_id_; bool tx_level_temp_table_; // get_session_allocator can only apply for fixed-length memory. // To customize the memory length, you need to use malloc_alloctor of mem_context diff --git a/src/sql/session/ob_sql_session_mgr.cpp b/src/sql/session/ob_sql_session_mgr.cpp index 6a64e311f9..d254fd0541 100644 --- a/src/sql/session/ob_sql_session_mgr.cpp +++ b/src/sql/session/ob_sql_session_mgr.cpp @@ -433,8 +433,7 @@ int ObSQLSessionMgr::free_session(const ObFreeSessionCtx &ctx) ObSQLSessionInfo *sess_info = NULL; sessinfo_map_.get(Key(sessid), sess_info); if (NULL != sess_info) { - if (sess_info->has_got_conn_res() - && OB_UNLIKELY(OB_SUCCESS != sess_info->on_user_disconnect())) { + if (OB_UNLIKELY(OB_SUCCESS != sess_info->on_user_disconnect())) { LOG_WARN("user disconnect failed", K(ret), K(sess_info->get_user_id())); } sessinfo_map_.revert(sess_info); diff --git a/src/sql/session/ob_user_resource_mgr.cpp b/src/sql/session/ob_user_resource_mgr.cpp index f17657218a..59b2162779 100644 --- a/src/sql/session/ob_user_resource_mgr.cpp +++ b/src/sql/session/ob_user_resource_mgr.cpp @@ -196,7 +196,7 @@ int ObConnectResourceMgr::get_or_insert_user_resource(const uint64_t tenant_id, const uint64_t user_id, const uint64_t max_user_connections, const uint64_t max_connections_per_hour, - ObConnectResource *&user_res, bool &has_insert) + ObConnectResource *&user_res, bool &has_insert, bool &user_conn_increased) { int ret = OB_SUCCESS; user_res = NULL; @@ -224,6 +224,7 @@ int ObConnectResourceMgr::get_or_insert_user_resource(const uint64_t tenant_id, } } else { has_insert = true; + user_conn_increased = max_user_connections != 0; user_res_map_.revert(user_res); } } else { @@ -237,7 +238,8 @@ int ObConnectResourceMgr::increase_user_connections_count( const uint64_t max_user_connections, const uint64_t max_connections_per_hour, const ObString &user_name, - ObConnectResource *user_res) + ObConnectResource *user_res, + bool &user_conn_increased) { int ret = OB_SUCCESS; if (OB_ISNULL(user_res)) { @@ -270,6 +272,7 @@ int ObConnectResourceMgr::increase_user_connections_count( if (OB_SUCC(ret)) { user_res->history_connections_ += 0 == max_connections_per_hour ? 0 : 1; user_res->cur_connections_ += 0 == max_user_connections ? 0 : 1; + user_conn_increased = 0 != max_user_connections; } } if (OB_SUCC(ret)) { @@ -293,36 +296,44 @@ int ObConnectResourceMgr::on_user_connect( { int ret = OB_SUCCESS; lib::Worker::CompatMode compat_mode = lib::Worker::CompatMode::ORACLE; - if (OB_UNLIKELY(session.has_got_conn_res())) { - ret = OB_ERR_UNEXPECTED; - LOG_ERROR("session trying to connect already occupy connection resource", K(tenant_id), - K(user_id), K(session.get_sessid())); + if (!session.is_user_session()) { + // do not limit connection count for inner sesion. } else if (OB_FAIL(share::ObCompatModeGetter::get_tenant_mode(tenant_id, compat_mode))) { LOG_WARN("get tenant mode failed", K(ret), K(tenant_id)); - } else if (compat_mode != lib::Worker::CompatMode::MYSQL) { - } else if (OB_FAIL(apply_for_tenant_conn_resource(tenant_id, priv, max_tenant_connections))) { - LOG_WARN("reach teannt max connections", K(ret)); - } else if (0 == max_connections_per_hour && 0 == max_user_connections) { - session.set_got_conn_res(true); - } else { - // only increase cur_connections_ if max_user_connections is not zero - // only record history_connections_ if max_connections_per_hour is not zero. - ObConnectResource *user_res = NULL; - bool has_insert = false; - if (OB_FAIL(get_or_insert_user_resource(tenant_id, user_id, max_user_connections, - max_connections_per_hour, user_res, has_insert))) { - LOG_WARN("get or insert user resource failed", K(ret)); - } else if (!has_insert) { - // if user resource already exists in the hash map, increase its connections count. - if (OB_FAIL(increase_user_connections_count(max_user_connections, max_connections_per_hour, - user_name, user_res))) { - LOG_WARN("increase user connection count failed", K(ret)); + } else if (compat_mode == lib::Worker::CompatMode::MYSQL) { + if (!session.has_got_tenant_conn_res()) { + if (OB_FAIL(apply_for_tenant_conn_resource(tenant_id, priv, max_tenant_connections))) { + LOG_WARN("reach teannt max connections", K(ret)); + } else { + session.set_got_tenant_conn_res(true); } } if (OB_FAIL(ret)) { - release_tenant_conn_resource(tenant_id); + } else if (session.has_got_user_conn_res()) { + } else if (0 == max_connections_per_hour && 0 == max_user_connections) { } else { - session.set_got_conn_res(true); + // According to document of MySQL: + // "Resource-use counting takes place when any account has a nonzero limit placed on its use of any of the resources." + // only increase cur_connections_ if max_user_connections is not zero + // only record history_connections_ if max_connections_per_hour is not zero. + ObConnectResource *user_res = NULL; + bool has_insert = false; + bool user_conn_increased = false; + if (OB_FAIL(get_or_insert_user_resource(tenant_id, user_id, max_user_connections, + max_connections_per_hour, user_res, + has_insert, user_conn_increased))) { + LOG_WARN("get or insert user resource failed", K(ret)); + } else if (!has_insert) { + // if user resource already exists in the hash map, increase its connections count. + if (OB_FAIL(increase_user_connections_count(max_user_connections, max_connections_per_hour, + user_name, user_res, user_conn_increased))) { + LOG_WARN("increase user connection count failed", K(ret)); + } + } + if (user_conn_increased) { + session.set_got_user_conn_res(true); + session.set_conn_res_user_id(user_id); + } } } return ret; @@ -335,21 +346,20 @@ int ObConnectResourceMgr::on_user_connect( int ObConnectResourceMgr::on_user_disconnect(ObSQLSessionInfo &session) { int ret = OB_SUCCESS; - lib::Worker::CompatMode compat_mode; - uint64_t max_user_connections = 0; - uint64_t user_id = session.get_user_id(); - uint64_t tenant_id = session.get_login_tenant_id(); - if (OB_UNLIKELY(!session.has_got_conn_res())) { - // do nothing - } else if (OB_FAIL(share::ObCompatModeGetter::get_tenant_mode(tenant_id, compat_mode))) { - LOG_ERROR("get tenant mode failed", K(ret), K(tenant_id)); - } else if (compat_mode != lib::Worker::CompatMode::MYSQL) { - } else if (OB_FAIL(session.get_sys_variable(share::SYS_VAR_MAX_USER_CONNECTIONS, - max_user_connections))) { - LOG_ERROR("get system variable SYS_VAR_MAX_USER_CONNECTIONS failed", K(ret)); + if (!session.is_user_session()) { + // do not limit connection count for inner sesion. + if (OB_UNLIKELY(session.has_got_tenant_conn_res() || session.has_got_user_conn_res())) { + LOG_ERROR("inner session expect no connection resource", K(session.get_conn_res_user_id()), + K(session.has_got_tenant_conn_res())); + } } else { - release_tenant_conn_resource(tenant_id); - if (0 != max_user_connections) { + uint64_t tenant_id = session.get_login_tenant_id(); + if (session.has_got_tenant_conn_res()) { + release_tenant_conn_resource(tenant_id); + session.set_got_tenant_conn_res(false); + } + if (session.has_got_user_conn_res()) { + uint64_t user_id = session.get_conn_res_user_id(); ObConnectResource *user_res = NULL; ObTenantUserKey user_key(tenant_id, user_id); if (OB_FAIL(user_res_map_.get(user_key, user_res))) { @@ -367,8 +377,8 @@ int ObConnectResourceMgr::on_user_disconnect(ObSQLSessionInfo &session) } user_res_map_.revert(user_res); } + session.set_got_user_conn_res(false); } - session.set_got_conn_res(false); } return ret; } diff --git a/src/sql/session/ob_user_resource_mgr.h b/src/sql/session/ob_user_resource_mgr.h index 8c3b7d407d..515fe46b3f 100644 --- a/src/sql/session/ob_user_resource_mgr.h +++ b/src/sql/session/ob_user_resource_mgr.h @@ -125,12 +125,13 @@ public: const uint64_t user_id, const uint64_t max_user_connections, const uint64_t max_connections_per_hour, - ObConnectResource *&user_res, bool &has_insert); + ObConnectResource *&user_res, bool &has_insert, bool &user_conn_increased); int increase_user_connections_count( const uint64_t max_user_connections, const uint64_t max_connections_per_hour, const ObString &user_name, - ObConnectResource *user_res); + ObConnectResource *user_res, + bool &user_conn_increased); int on_user_connect(const uint64_t tenant_id, const uint64_t user_id, const ObPrivSet &priv,