Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: standby meta server exits abnormally with core dump after receiving the http request /meta/cluster #1816

Merged
merged 3 commits into from
Dec 25, 2023
Merged

fix: standby meta server exits abnormally with core dump after receiving the http request /meta/cluster #1816

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68082d6
fix: define DSN_MOCK_TEST only while building Pegasus for testing and…
empiredan Dec 22, 2023
9328e33
fix
empiredan Dec 22, 2023
7e207a6
comments
empiredan Dec 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cmake_modules/BaseFunctions.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 0 additions & 2 deletions src/meta/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
23 changes: 19 additions & 4 deletions src/meta/meta_http_service.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -827,12 +828,26 @@ 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
// Once MOCK_TEST is defined, the meta server must has been built with `./run.sh build --test`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to say BUILD_TEST or MOCK_TEST is enabled, because it's possible to build by cmake directly, not using ./run.sh build.

//
// 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()) {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/api_utilities.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ extern void dsn_coredump();
} \
} while (0)

#ifdef DSN_MOCK_TEST
#ifdef MOCK_TEST
#define mock_private public
#define mock_virtual virtual
#else
Expand Down
2 changes: 1 addition & 1 deletion src/utils/metrics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1404,7 +1404,7 @@ class percentile : public closeable_metric
_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;
Expand Down
Loading