This is an automated email from the ASF dual-hosted git repository.
wangdan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git
The following commit(s) were added to refs/heads/master by this push:
new 96868ed93 refactor: remove the unnecessary `zookeeper_session_mgr`
class (#2356)
96868ed93 is described below
commit 96868ed93b2a1acb26f1241f2f36d6a976c22c43
Author: Dan Wang <[email protected]>
AuthorDate: Wed Jan 21 20:11:55 2026 +0800
refactor: remove the unnecessary `zookeeper_session_mgr` class (#2356)
The `zookeeper_session_mgr::get_session(...)` can be made static, so it can
be extracted
and implemented as a separate global function. Setting the ZooKeeper C
client’s log file
handle can be done together with the log level, ensuring it is executed
only once globally
and before any `zhandle_t` object is created. Thus entire
`zookeeper_session_mgr` class
could be removed.
In addition, error info would be printed if the the ZooKeeper C client's
log file failed to be
opened. Also introduce mutex to support concurrent operations to obtain
ZooKeeper sessions
by multiple threads.
---
.licenserc.yaml | 2 -
src/meta/meta_state_service_zookeeper.cpp | 4 +-
.../distributed_lock_service_zookeeper.cpp | 4 +-
src/zookeeper/zookeeper_session.cpp | 56 +++++++++++++++++---
src/zookeeper/zookeeper_session.h | 5 ++
src/zookeeper/zookeeper_session_mgr.cpp | 61 ----------------------
src/zookeeper/zookeeper_session_mgr.h | 52 ------------------
7 files changed, 56 insertions(+), 128 deletions(-)
diff --git a/.licenserc.yaml b/.licenserc.yaml
index 2501b21c4..fdbd33004 100644
--- a/.licenserc.yaml
+++ b/.licenserc.yaml
@@ -668,8 +668,6 @@ header:
- 'src/zookeeper/zookeeper_error.h'
- 'src/zookeeper/zookeeper_session.cpp'
- 'src/zookeeper/zookeeper_session.h'
- - 'src/zookeeper/zookeeper_session_mgr.cpp'
- - 'src/zookeeper/zookeeper_session_mgr.h'
# Apache License, Version 2.0, Copyright 2018 Google LLC
- 'src/gutil/test/map_traits_test.cpp'
- 'src/gutil/test/map_util_test.h'
diff --git a/src/meta/meta_state_service_zookeeper.cpp
b/src/meta/meta_state_service_zookeeper.cpp
index c84fa86b9..1dd7ce3d6 100644
--- a/src/meta/meta_state_service_zookeeper.cpp
+++ b/src/meta/meta_state_service_zookeeper.cpp
@@ -41,7 +41,6 @@
#include "utils/utils.h"
#include "zookeeper/zookeeper_error.h"
#include "zookeeper/zookeeper_session.h"
-#include "zookeeper/zookeeper_session_mgr.h"
DSN_DECLARE_int32(timeout_ms);
@@ -160,8 +159,7 @@
meta_state_service_zookeeper::~meta_state_service_zookeeper()
error_code meta_state_service_zookeeper::initialize(const
std::vector<std::string> &)
{
- _session =
-
zookeeper_session_mgr::instance().get_session(service_app::current_service_app_info());
+ _session = get_zookeeper_session(service_app::current_service_app_info());
_zoo_state = _session->attach(this,
std::bind(&meta_state_service_zookeeper::on_zoo_session_evt,
ref_this(this),
diff --git a/src/zookeeper/distributed_lock_service_zookeeper.cpp
b/src/zookeeper/distributed_lock_service_zookeeper.cpp
index f6ed6313e..e578072fb 100644
--- a/src/zookeeper/distributed_lock_service_zookeeper.cpp
+++ b/src/zookeeper/distributed_lock_service_zookeeper.cpp
@@ -39,7 +39,6 @@
#include "utils/strings.h"
#include "zookeeper/lock_struct.h"
#include "zookeeper/lock_types.h"
-#include "zookeeper/zookeeper_session_mgr.h"
#include "zookeeper_error.h"
#include "zookeeper_session.h"
@@ -94,8 +93,7 @@ error_code
distributed_lock_service_zookeeper::initialize(const std::vector<std:
}
const char *lock_root = args[0].c_str();
- _session =
-
zookeeper_session_mgr::instance().get_session(service_app::current_service_app_info());
+ _session = get_zookeeper_session(service_app::current_service_app_info());
_zoo_state = _session->attach(this,
std::bind(&distributed_lock_service_zookeeper::on_zoo_session_evt,
lock_srv_ptr(this),
diff --git a/src/zookeeper/zookeeper_session.cpp
b/src/zookeeper/zookeeper_session.cpp
index 5bc728d5a..dfd8bb7ec 100644
--- a/src/zookeeper/zookeeper_session.cpp
+++ b/src/zookeeper/zookeeper_session.cpp
@@ -31,7 +31,9 @@
#include <zookeeper/zookeeper.h>
#include <algorithm>
#include <cerrno>
+#include <cstdio>
#include <cstdlib>
+#include <mutex>
#include <utility>
#include "rpc/rpc_address.h"
@@ -42,6 +44,7 @@
#include "utils/flags.h"
#include "utils/fmt_logging.h"
#include "utils/safe_strerror_posix.h"
+#include "utils/singleton_store.h"
#include "utils/strings.h"
#include "zookeeper/proto.h"
#include "zookeeper/zookeeper.jute.h"
@@ -70,6 +73,7 @@ DSN_DEFINE_int32(zookeeper,
timeout_ms,
30000,
"The timeout of accessing ZooKeeper, in milliseconds");
+DSN_DEFINE_string(zookeeper, logfile, "zoo.log", "The path of the log file for
ZooKeeper C client");
DSN_DEFINE_string(zookeeper, zoo_log_level, "ZOO_LOG_LEVEL_INFO", "ZooKeeper
log level");
DSN_DEFINE_string(zookeeper, hosts_list, "", "ZooKeeper hosts list");
DSN_DEFINE_string(zookeeper, sasl_service_name, "zookeeper", "");
@@ -119,8 +123,7 @@
DSN_DEFINE_group_validator(consistency_between_configurations, [](std::string &m
return true;
});
-namespace dsn {
-namespace dist {
+namespace dsn::dist {
zookeeper_session::zoo_atomic_packet::zoo_atomic_packet(unsigned int size)
{
@@ -274,9 +277,32 @@ bool is_password_file_plaintext()
zhandle_t *create_zookeeper_handle(watcher_fn watcher, void *context)
{
- const auto zoo_log_level = enum_from_string(FLAGS_zoo_log_level,
INVALID_ZOO_LOG_LEVEL);
- CHECK(zoo_log_level != INVALID_ZOO_LOG_LEVEL, "Invalid zoo log level: {}",
FLAGS_zoo_log_level);
- zoo_set_debug_level(zoo_log_level);
+ static std::once_flag flag;
+ std::call_once(flag, []() {
+ FILE *zoo_log_file = std::fopen(FLAGS_logfile, "a");
+ if (zoo_log_file == nullptr) {
+ LOG_ERROR("failed to open the log file for ZooKeeper C Client, use
stderr instead: "
+ "path = {}, error = {}",
+ FLAGS_logfile,
+ utils::safe_strerror(errno));
+ zoo_set_log_stream(stderr);
+ } else {
+ // The file handle pointed to by `zoo_log_file` will never be
released because:
+ // 1. Each meta server process holds only one log file handle.
+ // 2. It cannot be guaranteed that the handle will no longer be
used in some
+ // background thread of ZooKeeper C Client after being released
even by atexit(),
+ // since zhandle_t objects are not closed in the end. Once
`fclose` is called on
+ // the log file handle, any subsequent write operations on it
would result in
+ // undefined behavior.
+ zoo_set_log_stream(zoo_log_file);
+ }
+
+ const auto zoo_log_level = enum_from_string(FLAGS_zoo_log_level,
INVALID_ZOO_LOG_LEVEL);
+ CHECK(zoo_log_level != INVALID_ZOO_LOG_LEVEL,
+ "Invalid zoo log level: {}",
+ FLAGS_zoo_log_level);
+ zoo_set_debug_level(zoo_log_level);
+ });
// SASL auth is enabled iff FLAGS_sasl_mechanisms_type is non-empty.
if (dsn::utils::is_empty(FLAGS_sasl_mechanisms_type)) {
@@ -565,5 +591,21 @@ void zookeeper_session::global_void_completion(int rc,
const void *data)
release_ref(op_ctx);
}
-} // namespace dist
-} // namespace dsn
+zookeeper_session *get_zookeeper_session(const service_app_info &info)
+{
+ static std::mutex mtx;
+ auto &store = utils::singleton_store<int, zookeeper_session *>::instance();
+
+ zookeeper_session *session{nullptr};
+ {
+ std::lock_guard<std::mutex> lock(mtx);
+ if (!store.get(info.entity_id, session)) {
+ session = new zookeeper_session(info);
+ store.put(info.entity_id, session);
+ }
+ }
+
+ return session;
+}
+
+} // namespace dsn::dist
diff --git a/src/zookeeper/zookeeper_session.h
b/src/zookeeper/zookeeper_session.h
index 2dd602cbe..6792974ca 100644
--- a/src/zookeeper/zookeeper_session.h
+++ b/src/zookeeper/zookeeper_session.h
@@ -205,6 +205,11 @@ private:
DISALLOW_MOVE_AND_ASSIGN(zookeeper_session);
};
+// Obtain a ZooKeeper session. Each service node has at most one ZooKeeper
session; if none
+// exists for the service app, one will be created. The obtained ZooKeeper
session can be
+// shared across multiple threads in one service node. This function is
thread-safe.
+zookeeper_session *get_zookeeper_session(const service_app_info &info);
+
} // namespace dsn::dist
USER_DEFINED_STRUCTURE_FORMATTER(::dsn::dist::zookeeper_session);
diff --git a/src/zookeeper/zookeeper_session_mgr.cpp
b/src/zookeeper/zookeeper_session_mgr.cpp
deleted file mode 100644
index ea44dbd5f..000000000
--- a/src/zookeeper/zookeeper_session_mgr.cpp
+++ /dev/null
@@ -1,61 +0,0 @@
-/*
- * The MIT License (MIT)
- *
- * Copyright (c) 2015 Microsoft Corporation
- *
- * -=- Robust Distributed System Nucleus (rDSN) -=-
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#include "zookeeper_session_mgr.h"
-
-#include <stdio.h>
-#include <zookeeper/zookeeper.h>
-#include <functional>
-
-#include "runtime/service_app.h"
-#include "utils/flags.h"
-#include "utils/singleton_store.h"
-#include "zookeeper_session.h"
-
-DSN_DEFINE_string(zookeeper, logfile, "zoo.log", "The Zookeeper logfile");
-
-namespace dsn {
-namespace dist {
-
-zookeeper_session_mgr::zookeeper_session_mgr()
-{
- FILE *fp = fopen(FLAGS_logfile, "a");
- if (fp != nullptr)
- zoo_set_log_stream(fp);
-}
-
-zookeeper_session *zookeeper_session_mgr::get_session(const service_app_info
&info)
-{
- auto &store = utils::singleton_store<int, zookeeper_session *>::instance();
- zookeeper_session *ans = nullptr;
- if (!store.get(info.entity_id, ans)) {
- ans = new zookeeper_session(info);
- store.put(info.entity_id, ans);
- }
- return ans;
-}
-} // namespace dist
-} // namespace dsn
diff --git a/src/zookeeper/zookeeper_session_mgr.h
b/src/zookeeper/zookeeper_session_mgr.h
deleted file mode 100644
index ada6bc4f7..000000000
--- a/src/zookeeper/zookeeper_session_mgr.h
+++ /dev/null
@@ -1,52 +0,0 @@
-/*
- * The MIT License (MIT)
- *
- * Copyright (c) 2015 Microsoft Corporation
- *
- * -=- Robust Distributed System Nucleus (rDSN) -=-
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#include "utils/singleton.h"
-
-#pragma once
-
-namespace dsn {
-struct service_app_info;
-
-namespace dist {
-
-class zookeeper_session;
-
-// A singleton to manage all zookeeper sessions, so that each zookeeper session
-// can be shared by all threads in one service-node.
-class zookeeper_session_mgr : public utils::singleton<zookeeper_session_mgr>
-{
-public:
- zookeeper_session *get_session(const service_app_info &info);
-
-private:
- zookeeper_session_mgr();
- ~zookeeper_session_mgr() = default;
-
- friend class utils::singleton<zookeeper_session_mgr>;
-};
-} // namespace dist
-} // namespace dsn
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
