Merge pull request ceph#55328 from rosinL/wip-fix-64117

cmake/AddCephTest: bind crimson unittest to different cores

Reviewed-by: Chunmei Liu <[email protected]>
Reviewed-by: Samuel Just <[email protected]>
Reviewed-by: Kefu Chai <[email protected]>

Author: tchaikov

CMakeLists.txt

@@ -608,7 +608,7 @@ option(PG_DEBUG_REFS "PG Ref debugging is enabled" OFF)
 
 option(WITH_TESTS "enable the build of ceph-test package scripts/binaries" ON)
 set(UNIT_TESTS_BUILT ${WITH_TESTS})
-set(CEPH_TEST_TIMEOUT 3600 CACHE STRING 
+set(CEPH_TEST_TIMEOUT 7200 CACHE STRING
   "Maximum time before a CTest gets killed" )
 
 # fio

cmake/modules/AddCephTest.cmake

@@ -19,9 +19,27 @@ function(add_ceph_test test_name test_path)
     PATH=${CMAKE_RUNTIME_OUTPUT_DIRECTORY}:${CMAKE_SOURCE_DIR}/src:$ENV{PATH}
     PYTHONPATH=${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/cython_modules/lib.3:${CMAKE_SOURCE_DIR}/src/pybind
     CEPH_BUILD_VIRTUALENV=${CEPH_BUILD_VIRTUALENV})
-  # none of the tests should take more than 1 hour to complete
   set_property(TEST ${test_name}
     PROPERTY TIMEOUT ${CEPH_TEST_TIMEOUT})
+  # Crimson seastar unittest always run with --smp N to start N threads. By default, crimson seastar unittest
+  # will take cpu cores[0, N), starting one thread per core. When running many crimson seastar unittests
+  # parallely, the front N cpu cores are shared, and the left cpu cores are idle. Lots of cpu cores are wasted.
+  # Using CTest resource allocation feature(https://cmake.org/cmake/help/latest/manual/ctest.1.html#resource-allocation),
+  # ctest can specify cpu cores resources to crimson seastar unittests.
+  # 3 steps to enable CTest resource allocation feature:
+  #  Step 1: Generate a resource specification file to describe available resource, $(nproc) CPUs with id 0 to $(nproc) - 1
+  #  Step 2: Set RESOURCE_GROUPS property to a test with value "${smp_count},cpus:1"
+  #  Step 3: Read a series of environment variables CTEST_RESOURCE_GROUP_* and set seastar smp_opts while running a test
+  list(FIND ARGV "--smp" smp_pos)
+  if(smp_pos GREATER -1)
+    if(smp_pos EQUAL ARGC)
+      message(FATAL_ERROR "${test_name} --smp requires an argument")
+    endif()
+    math(EXPR i "${smp_pos} + 1")
+    list(GET ARGV ${i} smp_count)
+    set_property(TEST ${test_name}
+      PROPERTY RESOURCE_GROUPS "${smp_count},cpus:1")
+  endif()
 endfunction()
 
 option(WITH_GTEST_PARALLEL "Enable running gtest based tests in parallel" OFF)

run-make-check.sh

@@ -22,6 +22,14 @@ source src/script/run-make.sh
 
 set -e
 
+function gen_ctest_resource_file() {
+    local file_name=$(mktemp /tmp/ctest-resource-XXXXXX)
+    local max_cpuid=$(($(nproc) - 1))
+    jq -n '$ARGS.positional | map({id:., slots:1}) | {cpus:.} | {version: {major:1, minor:0}, local:[.]}' \
+        --args $(seq 0 $max_cpuid) > $file_name
+    echo "$file_name"
+}
+
 function run() {
     # to prevent OSD EMFILE death on tests, make sure ulimit >= 1024
     $DRY_RUN ulimit -n $(ulimit -Hn)
@@ -43,14 +51,16 @@ function run() {
     fi
 
     CHECK_MAKEOPTS=${CHECK_MAKEOPTS:-$DEFAULT_MAKEOPTS}
+    CTEST_RESOURCE_FILE=$(gen_ctest_resource_file)
+    CHECK_MAKEOPTS+=" --resource-spec-file ${CTEST_RESOURCE_FILE}"
     if in_jenkins; then
         if ! ctest $CHECK_MAKEOPTS --no-compress-output --output-on-failure --test-output-size-failed 1024000 -T Test; then
             # do not return failure, as the jenkins publisher will take care of this
-            rm -fr ${TMPDIR:-/tmp}/ceph-asok.*
+            rm -fr ${TMPDIR:-/tmp}/ceph-asok.* ${CTEST_RESOURCE_FILE}
         fi
     else
         if ! $DRY_RUN ctest $CHECK_MAKEOPTS --output-on-failure; then
-            rm -fr ${TMPDIR:-/tmp}/ceph-asok.*
+            rm -fr ${TMPDIR:-/tmp}/ceph-asok.* ${CTEST_RESOURCE_FILE}
             return 1
         fi
     fi

src/test/crimson/ctest_utils.h

@@ -0,0 +1,78 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#pragma once
+
+#include <cstdlib>
+#include <iostream>
+#include <optional>
+#include <regex>
+#include <string>
+#include <vector>
+
+#include <boost/algorithm/string.hpp>
+#include <fmt/format.h>
+#include <seastar/core/resource.hh>
+#include <seastar/core/app-template.hh>
+
+struct ctest_resource {
+  int id;
+  int slots;
+  ctest_resource(int id, int slots) : id(id), slots(slots) {}
+};
+
+static std::vector<ctest_resource> parse_ctest_resources(const std::string& resource_spec) {
+  std::vector<std::string> resources;
+  boost::split(resources, resource_spec, boost::is_any_of(";"));
+  std::regex res_regex("id:([0-9]+),slots:([0-9]+)");
+  std::vector<ctest_resource> ctest_resources;
+  for (auto& resource : resources) {
+    std::smatch matched;
+    if (std::regex_match(resource, matched, res_regex)) {
+      int id = std::stoi(matched[1].str());
+      int slots = std::stoi(matched[2].str());
+      ctest_resources.emplace_back(id, slots);
+    }
+  }
+  return ctest_resources;
+}
+
+static std::optional<seastar::resource::cpuset> get_cpuset_from_ctest_resource_group() {
+  int nr_groups = 0;
+  auto group_count = std::getenv("CTEST_RESOURCE_GROUP_COUNT");
+  if (group_count != nullptr) {
+    nr_groups = std::stoi(group_count);
+  } else {
+    return {};
+  }
+
+  seastar::resource::cpuset cpuset;
+  for (int num = 0; num < nr_groups; num++) {
+    std::string resource_type_name;
+    fmt::format_to(std::back_inserter(resource_type_name), "CTEST_RESOURCE_GROUP_{}", num);
+    // only a single resource type is supported for now
+    std::string resource_type = std::getenv(resource_type_name.data());
+    if (resource_type == "cpus") {
+      std::transform(resource_type.begin(), resource_type.end(), resource_type.begin(), ::toupper);
+      std::string resource_group;
+      fmt::format_to(std::back_inserter(resource_group), "CTEST_RESOURCE_GROUP_{}_{}", num, resource_type);
+      std::string resource_spec = std::getenv(resource_group.data());
+      for (auto& resource : parse_ctest_resources(resource_spec)) {
+        // each id has a single cpu slot
+        cpuset.insert(resource.id);
+      }
+    } else {
+      fmt::print(std::cerr, "unsupported resource type: {}", resource_type);
+    }
+  }
+  return cpuset;
+}
+
+static seastar::app_template::seastar_options get_smp_opts_from_ctest() {
+  seastar::app_template::seastar_options opts;
+  auto cpuset = get_cpuset_from_ctest_resource_group();
+  if (cpuset) {
+    opts.smp_opts.cpuset.set_value(*cpuset);
+  }
+  return opts;
+}

src/test/crimson/seastar_runner.h

@@ -13,6 +13,8 @@
 #include <seastar/core/alien.hh>
 #include <seastar/core/thread.hh>
 
+#include "test/crimson/ctest_utils.h"
+
 struct SeastarRunner {
   static constexpr eventfd_t APP_RUNNING = 1;
   static constexpr eventfd_t APP_NOT_RUN = 2;
@@ -26,7 +28,7 @@ struct SeastarRunner {
   bool begin_signaled = false;
 
   SeastarRunner() :
-    begin_fd{seastar::file_desc::eventfd(0, 0)} {}
+    app{get_smp_opts_from_ctest()}, begin_fd{seastar::file_desc::eventfd(0, 0)} {}
 
   ~SeastarRunner() {}
 

src/test/crimson/test_alien_echo.cc

@@ -8,6 +8,7 @@
 #include "crimson/net/Connection.h"
 #include "crimson/net/Dispatcher.h"
 #include "crimson/net/Messenger.h"
+#include "test/crimson/ctest_utils.h"
 
 #include <seastar/core/alien.hh>
 #include <seastar/core/app-template.hh>
@@ -266,7 +267,7 @@ int main(int argc, char** argv)
   }
 
   auto count = vm["count"].as<unsigned>();
-  seastar::app_template app;
+  seastar::app_template app{get_smp_opts_from_ctest()};
   SeastarContext sc;
   auto job = sc.with_seastar([&] {
     auto fut = seastar::alien::submit_to(app.alien(), 0, [addr, role, count] {

src/test/crimson/test_alienstore_thread_pool.cc

@@ -6,6 +6,7 @@
 #include "crimson/common/config_proxy.h"
 #include "crimson/os/alienstore/thread_pool.h"
 #include "include/msgr.h"
+#include "test/crimson/ctest_utils.h"
 
 using namespace std::chrono_literals;
 using ThreadPool = crimson::os::ThreadPool;
@@ -37,7 +38,7 @@ seastar::future<> test_void_return(ThreadPool& tp) {
 
 int main(int argc, char** argv)
 {
-  seastar::app_template app;
+  seastar::app_template app{get_smp_opts_from_ctest()};
   return app.run(argc, argv, [] {
     std::vector<const char*> args;
     std::string cluster;

src/test/crimson/test_buffer.cc

@@ -3,6 +3,7 @@
 #include <seastar/core/future-util.hh>
 #include <seastar/core/reactor.hh>
 #include "include/buffer.h"
+#include "test/crimson/ctest_utils.h"
 
 // allocate a foreign buffer on each cpu, collect them all into a bufferlist,
 // and destruct it on this cpu
@@ -36,7 +37,7 @@ seastar::future<> test_foreign_bufferlist()
 
 int main(int argc, char** argv)
 {
-  seastar::app_template app;
+  seastar::app_template app{get_smp_opts_from_ctest()};
   return app.run(argc, argv, [] {
     return seastar::now().then(
       &test_foreign_bufferlist

src/test/crimson/test_config.cc

@@ -6,6 +6,7 @@
 #include "common/ceph_argparse.h"
 #include "common/config_obs.h"
 #include "crimson/common/config_proxy.h"
+#include "test/crimson/ctest_utils.h"
 
 using namespace std::literals;
 using Config = crimson::common::ConfigProxy;
@@ -88,7 +89,7 @@ static seastar::future<> test_config()
 
 int main(int argc, char** argv)
 {
-  seastar::app_template app;
+  seastar::app_template app{get_smp_opts_from_ctest()};
   return app.run(argc, argv, [&] {
     return test_config().then([] {
       std::cout << "All tests succeeded" << std::endl;

src/test/crimson/test_messenger.cc

@@ -29,6 +29,7 @@
 #include <seastar/core/with_timeout.hh>
 
 #include "test_messenger.h"
+#include "test/crimson/ctest_utils.h"
 
 using namespace std::chrono_literals;
 namespace bpo = boost::program_options;
@@ -3845,7 +3846,7 @@ seastar::future<int> do_test(seastar::app_template& app)
 
 int main(int argc, char** argv)
 {
-  seastar::app_template app;
+  seastar::app_template app{get_smp_opts_from_ctest()};
   app.add_options()
     ("verbose,v", bpo::value<bool>()->default_value(false),
      "chatty if true")