fix: standby meta server exits abnormally with core dump after receiv…

…ing the http request `/meta/cluster` (#1816)

#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

@@ -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.

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)

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()) {

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

src/utils/metrics.h

@@ -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;