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 a397c07ed fix: standby meta server exits abnormally with core dump
after receiving the http request `/meta/cluster` (#1816)
a397c07ed is described below
commit a397c07edb7144958e33266d44133b336b75d499
Author: Dan Wang <[email protected]>
AuthorDate: Mon Dec 25 14:31:26 2023 +0800
fix: standby meta server exits abnormally with core dump after receiving
the http request `/meta/cluster` (#1816)
https://github.com/apache/incubator-pegasus/issues/1817
If meta server is built without testing(i.e. `./run.sh build` without
`--test`),
the macro `MOCK_TEST` dedicated to test code should not be enabled,
which might lead to unexpected behaviors.
On the other hand, meta server should not exit abnormally with core
dump even if it is built for testing(i.e. `./run.sh build --test`). Once it
finds that balancer is not initialized by mocking, it should go through
the normal process to prevent from core dump.
---
cmake_modules/BaseFunctions.cmake | 4 ++++
src/meta/CMakeLists.txt | 2 --
src/meta/meta_http_service.cpp | 26 ++++++++++++++++++++++----
src/utils/api_utilities.h | 2 +-
src/utils/metrics.h | 2 +-
5 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/cmake_modules/BaseFunctions.cmake
b/cmake_modules/BaseFunctions.cmake
index dc0e3b28f..b1dff9b60 100644
--- a/cmake_modules/BaseFunctions.cmake
+++ b/cmake_modules/BaseFunctions.cmake
@@ -205,6 +205,10 @@ function(dsn_setup_compiler_flags)
# We want access to the PRI* print format macros.
add_definitions(-D__STDC_FORMAT_MACROS)
+ if(${BUILD_TEST})
+ add_definitions(-DMOCK_TEST)
+ endif()
+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17 -gdwarf-4" CACHE STRING
"" FORCE)
# -Wall: Enable all warnings.
diff --git a/src/meta/CMakeLists.txt b/src/meta/CMakeLists.txt
index fcc3a1232..a52a607f6 100644
--- a/src/meta/CMakeLists.txt
+++ b/src/meta/CMakeLists.txt
@@ -57,8 +57,6 @@ set(MY_BOOST_LIBS Boost::system Boost::filesystem)
# Extra files that will be installed
set(MY_BINPLACES "")
-add_definitions(-DDSN_MOCK_TEST)
-
dsn_add_shared_library()
add_subdirectory(test)
diff --git a/src/meta/meta_http_service.cpp b/src/meta/meta_http_service.cpp
index 902822d79..400fd160e 100644
--- a/src/meta/meta_http_service.cpp
+++ b/src/meta/meta_http_service.cpp
@@ -460,8 +460,9 @@ void meta_http_service::list_node_handler(const
http_request &req, http_response
void meta_http_service::get_cluster_info_handler(const http_request &req,
http_response &resp)
{
- if (!redirect_if_not_primary(req, resp))
+ if (!redirect_if_not_primary(req, resp)) {
return;
+ }
dsn::utils::table_printer tp;
std::ostringstream out;
@@ -827,12 +828,29 @@ void meta_http_service::update_scenario_handler(const
http_request &req, http_re
bool meta_http_service::redirect_if_not_primary(const http_request &req,
http_response &resp)
{
-#ifdef DSN_MOCK_TEST
- return true;
+#ifdef MOCK_TEST
+ // To enable MOCK_TEST, the option BUILD_TEST for cmake should be opened
by:
+ // cmake -DBUILD_TEST=ON ...
+ // which could be done by building Pegasus with tests by:
+ // ./run.sh build --test ...
+ //
+ // If `_service->_balancer` is not null, it must has been initialized by
mocking, in which case
+ // just returning true is ok.
+ //
+ // Otherwise, once `_service->_balancer` is null, which means this must be
a standby meta
+ // server, returning true would lead to coredump due to null
`_service->_balancer` while
+ // processing requests in `get_cluster_info_handler`. Thus it should go
through the following
+ // normal process instead of just returning true.
+ if (_service->_balancer) {
+ return true;
+ }
#endif
+
rpc_address leader;
- if (_service->_failure_detector->get_leader(&leader))
+ if (_service->_failure_detector->get_leader(&leader)) {
return true;
+ }
+
// set redirect response
resp.location = "http://"; + leader.to_std_string() + '/' + req.path;
if (!req.query_args.empty()) {
diff --git a/src/utils/api_utilities.h b/src/utils/api_utilities.h
index 29cf232a4..537ceaaed 100644
--- a/src/utils/api_utilities.h
+++ b/src/utils/api_utilities.h
@@ -84,7 +84,7 @@ extern void dsn_coredump();
}
\
} while (0)
-#ifdef DSN_MOCK_TEST
+#ifdef MOCK_TEST
#define mock_private public
#define mock_virtual virtual
#else
diff --git a/src/utils/metrics.h b/src/utils/metrics.h
index d2789e51b..93ed703e7 100644
--- a/src/utils/metrics.h
+++ b/src/utils/metrics.h
@@ -1404,7 +1404,7 @@ protected:
_full_nth_elements[i].store(value_type{},
std::memory_order_relaxed);
}
-#ifdef DSN_MOCK_TEST
+#ifdef MOCK_TEST
if (interval_ms == 0) {
// Timer is disabled.
return;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
