fix(github): throttle_test failed frequently due to insufficient disk…

… space (#1530) #1529 Options used by onebox and function tests are made configurable. The default values for these options are unchanged. Once custom options are provided as the arguments of `run.sh`, these custom options would be used to substitute for the current configurations. Github workflow for cpp has used custom options to reduce the consumption of disk space. On the other hand, to solve the problem that `throttle_test` failed frequently fundamentally, `throttle_test` should be refactored: at the end of each test case, onebox for each replica server should be stopped and the data should be cleared to release the disk space. This is tracked by another issue: #1534
apache · Jun 14, 2023 · 96116f6 · 96116f6
1 parent a509e8a
commit 96116f6
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 16 deletions.
diff --git a/.github/workflows/lint_and_test_cpp.yaml b/.github/workflows/lint_and_test_cpp.yaml
@@ -46,6 +46,11 @@ on:
   # for manually triggering workflow
   workflow_dispatch:
 
+env:
+  # Update the options to reduce the consumption of the disk space
+  ONEBOX_OPTS: disk_min_available_space_ratio=5
+  TEST_OPTS: throttle_test_medium_value_kb=10,throttle_test_large_value_kb=25
+
 jobs:
   cpp_clang_format_linter:
     name: Lint
@@ -231,7 +236,7 @@ jobs:
           export LD_LIBRARY_PATH=`pwd`/thirdparty/output/lib:/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server
           ulimit -s unlimited
           . ./scripts/config_hdfs.sh
-          ./run.sh test -m ${{ matrix.test_module }}
+          ./run.sh test --onebox_opts "$ONEBOX_OPTS" --test_opts "$TEST_OPTS" -m ${{ matrix.test_module }}
 
   build_ASAN:
     name: Build ASAN
@@ -357,7 +362,7 @@ jobs:
           export LD_LIBRARY_PATH=`pwd`/thirdparty/output/lib:/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server
           ulimit -s unlimited
           . ./scripts/config_hdfs.sh
-          ./run.sh test -m ${{ matrix.test_module }}
+          ./run.sh test --onebox_opts "$ONEBOX_OPTS" --test_opts "$TEST_OPTS" -m ${{ matrix.test_module }}
 
 # TODO(yingchun): Build and test UBSAN version would cost a very long time, we will run these tests
 #                 when we are going to release a stable version. So we disable them in regular CI
@@ -482,7 +487,7 @@ jobs:
 #          export LD_LIBRARY_PATH=`pwd`/thirdparty/output/lib:/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server
 #          ulimit -s unlimited
 #          . ./scripts/config_hdfs.sh
-#          ./run.sh test -m ${{ matrix.test_module }}
+#          ./run.sh test --onebox_opts "$ONEBOX_OPTS" --test_opts "$TEST_OPTS" -m ${{ matrix.test_module }}
 
   build_with_jemalloc:
     name: Build with jemalloc
@@ -577,7 +582,7 @@ jobs:
           export LD_LIBRARY_PATH=`pwd`/thirdparty/output/lib:/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server
           ulimit -s unlimited
           . ./scripts/config_hdfs.sh
-          ./run.sh test -m ${{ matrix.test_module }}
+          ./run.sh test --onebox_opts "$ONEBOX_OPTS" --test_opts "$TEST_OPTS" -m ${{ matrix.test_module }}
 
   build_pegasus_on_macos:
     name: macOS

diff --git a/run.sh b/run.sh
@@ -343,6 +343,8 @@ function usage_test()
     echo "                     e.g., \"pegasus_unit_test,dsn_runtime_tests,dsn_meta_state_tests\","
     echo "                     if not set, then run all tests"
     echo "   -k|--keep_onebox  whether keep the onebox after the test[default false]"
+    echo "   --onebox_opts     update configs for onebox, e.g. key1=value1,key2=value2"
+    echo "   --test_opts       update configs for tests, e.g. key1=value1,key2=value2"
 }
 function run_test()
 {
@@ -383,6 +385,8 @@ function run_test()
       restore_test
       throttle_test
     )
+    local onebox_opts=""
+    local test_opts=""
     while [[ $# > 0 ]]; do
         key="$1"
         case $key in
@@ -400,6 +404,14 @@ function run_test()
             --enable_gcov)
                 enable_gcov="yes"
                 ;;
+            --onebox_opts)
+                onebox_opts=$2
+                shift
+                ;;
+            --test_opts)
+                test_opts=$2
+                shift
+                ;;
             *)
                 echo "Error: unknown option \"$key\""
                 echo
@@ -459,6 +471,7 @@ function run_test()
             if [ "${module}" == "restore_test" ]; then
                 opts="cold_backup_disabled=false,cold_backup_checkpoint_reserve_minutes=0,cold_backup_root=mycluster"
             fi
+            [ -z ${onebox_opts} ] || opts="${opts},${onebox_opts}"
             if ! run_start_onebox -m ${m_count} -w -c --opts ${opts}; then
                 echo "ERROR: unable to continue on testing because starting onebox failed"
                 exit 1
@@ -470,7 +483,7 @@ function run_test()
             run_start_zk
         fi
         pushd ${BUILD_LATEST_DIR}/bin/$module
-        REPORT_DIR=$REPORT_DIR TEST_BIN=$module ./run.sh
+        REPORT_DIR=${REPORT_DIR} TEST_BIN=${module} TEST_OPTS=${test_opts} ./run.sh
         if [ $? != 0 ]; then
             echo "run test \"$module\" in `pwd` failed"
             exit 1

diff --git a/src/server/config.min.ini b/src/server/config.min.ini
@@ -135,6 +135,7 @@
 
 [replication]
   mutation_2pc_min_replica_count = 1
+  disk_min_available_space_ratio = 10
   cold_backup_root = onebox
   cluster_name = onebox
 

diff --git a/src/test/function_test/config.ini b/src/test/function_test/config.ini
@@ -79,3 +79,7 @@ lb_interval_ms = 3000
 mycluster = 127.0.0.1:34601,127.0.0.1:34602,127.0.0.1:34603
 onebox = 127.0.0.1:34601,127.0.0.1:34602,127.0.0.1:34603
 single_master_cluster = 127.0.0.1:34601
+
+[function_test.throttle_test]
+  throttle_test_medium_value_kb = 20
+  throttle_test_large_value_kb = 50
diff --git a/src/test/function_test/run.sh b/src/test/function_test/run.sh
@@ -24,6 +24,25 @@ if [ -z ${TEST_BIN} ]; then
     exit 1
 fi
 
+if [ -n ${TEST_OPTS} ]; then
+    if [ ! -f ./config.ini ]; then
+        echo "./config.ini does not exists !"
+        exit 1
+    fi
+
+    OPTS=`echo ${TEST_OPTS} | xargs`
+    config_kvs=(${OPTS//,/ })
+    for config_kv in ${config_kvs[@]}; do
+        config_kv=`echo $config_kv | xargs`
+        kv=(${config_kv//=/ })
+        if [ ! ${#kv[*]} -eq 2 ]; then
+            echo "Invalid config kv !"
+            exit 1
+        fi
+        sed -i '/^\s*'"${kv[0]}"'/c '"${kv[0]}"' = '"${kv[1]}" ./config.ini
+    done
+fi
+
 loop_count=0
 last_ret=0
 while [ $loop_count -le 5 ]

diff --git a/src/test/function_test/throttle_test/test_throttle.cpp b/src/test/function_test/throttle_test/test_throttle.cpp
@@ -43,6 +43,7 @@
 #include "test_util/test_util.h"
 #include "utils/error_code.h"
 #include "utils/errors.h"
+#include "utils/flags.h"
 #include "utils/fmt_logging.h"
 #include "utils/rand.h"
 
@@ -51,6 +52,18 @@ using namespace dsn::replication;
 using namespace pegasus;
 using std::string;
 
+static const uint64_t kLimitDurationMs = 10 * 1000;
+
+DSN_DEFINE_int32(function_test.throttle_test,
+                 throttle_test_medium_value_kb,
+                 20,
+                 "The size of generated medium value for test");
+
+DSN_DEFINE_int32(function_test.throttle_test,
+                 throttle_test_large_value_kb,
+                 50,
+                 "The size of generated large value for test");
+
 enum class throttle_type
 {
     read_by_qps,
@@ -80,8 +93,6 @@ struct throttle_test_plan
 
 struct throttle_test_recorder
 {
-    static const uint64_t limit_duration_ms = 10 * 1000;
-
     string test_name;
     uint64_t start_time_ms;
     std::atomic<uint64_t> total_successful_times;
@@ -101,7 +112,7 @@ struct throttle_test_recorder
         total_size_per_sec = 0;
     }
 
-    bool is_time_up() { return dsn_now_ms() - start_time_ms > limit_duration_ms; }
+    bool is_time_up() { return dsn_now_ms() - start_time_ms > kLimitDurationMs; }
 
     void record(uint64_t size, bool is_reject)
     {
@@ -113,8 +124,8 @@ struct throttle_test_recorder
 
     void finalize()
     {
-        total_qps = total_successful_times / (limit_duration_ms / 1000);
-        total_size_per_sec = total_successful_size / (limit_duration_ms / 1000);
+        total_qps = total_successful_times / (kLimitDurationMs / 1000);
+        total_size_per_sec = total_successful_size / (kLimitDurationMs / 1000);
     }
 };
 
@@ -163,7 +174,8 @@ class throttle_test : public test_util
 
     void start_test(const throttle_test_plan &test_plan)
     {
-        std::cout << fmt::format("start test, on {}", test_plan.test_plan_case) << std::endl;
+        fmt::print(
+            "start test, on {}, duration {} ms\n", test_plan.test_plan_case, kLimitDurationMs);
 
         result.reset(test_plan.test_plan_case);
 
@@ -308,10 +320,13 @@ class throttle_test : public test_util
                 ASSERT_EQ(0, ref_count.load());
                 ASSERT_TRUE(last_error == PERR_OK || last_error == PERR_APP_BUSY) << last_error;
             },
-            3 * throttle_test_recorder::limit_duration_ms / 1000);
+            3 * kLimitDurationMs / 1000);
     }
 };
 
+#define TEST_PLAN_DESC_custom_kb(op, sz)                                                           \
+    fmt::format(#op " test / throttle by size / {}kb value size", sz)
+
 TEST_F(throttle_test, test)
 {
     throttle_test_plan plan;
@@ -485,7 +500,11 @@ TEST_F(throttle_test, test)
     ASSERT_NEAR(result.total_size_per_sec, actual_value, actual_value * 0.3);
 
     // big value test can't run normally in the function test
-    plan = {"set test / throttle by size / 20kb value size", operation_type::set, 1024 * 20, 1, 50};
+    plan = {TEST_PLAN_DESC_custom_kb(set, FLAGS_throttle_test_medium_value_kb),
+            operation_type::set,
+            1024 * FLAGS_throttle_test_medium_value_kb,
+            1,
+            50};
     ASSERT_NO_FATAL_FAILURE(
         set_throttle(throttle_type::write_by_size,
                      (uint64_t)(plan.single_value_sz + test_hashkey_len + test_sortkey_len) *
@@ -499,7 +518,11 @@ TEST_F(throttle_test, test)
     result.finalize();
     ASSERT_NEAR(result.total_size_per_sec, actual_value, actual_value * 0.3);
 
-    plan = {"get test / throttle by size / 20kb value size", operation_type::get, 1024 * 20, 1, 50};
+    plan = {TEST_PLAN_DESC_custom_kb(get, FLAGS_throttle_test_medium_value_kb),
+            operation_type::get,
+            1024 * FLAGS_throttle_test_medium_value_kb,
+            1,
+            50};
     ASSERT_NO_FATAL_FAILURE(
         set_throttle(throttle_type::read_by_size,
                      (uint64_t)(plan.single_value_sz + test_hashkey_len + test_sortkey_len) *
@@ -513,7 +536,11 @@ TEST_F(throttle_test, test)
     result.finalize();
     ASSERT_NEAR(result.total_size_per_sec, actual_value, actual_value * 0.3);
 
-    plan = {"set test / throttle by size / 50kb value size", operation_type::set, 1024 * 50, 1, 50};
+    plan = {TEST_PLAN_DESC_custom_kb(set, FLAGS_throttle_test_large_value_kb),
+            operation_type::set,
+            1024 * FLAGS_throttle_test_large_value_kb,
+            1,
+            50};
     ASSERT_NO_FATAL_FAILURE(
         set_throttle(throttle_type::write_by_size,
                      (uint64_t)(plan.single_value_sz + test_hashkey_len + test_sortkey_len) *
@@ -527,7 +554,11 @@ TEST_F(throttle_test, test)
     result.finalize();
     ASSERT_NEAR(result.total_size_per_sec, actual_value, actual_value * 0.3);
 
-    plan = {"get test / throttle by size / 50kb value size", operation_type::get, 1024 * 50, 1, 50};
+    plan = {TEST_PLAN_DESC_custom_kb(get, FLAGS_throttle_test_large_value_kb),
+            operation_type::get,
+            1024 * FLAGS_throttle_test_large_value_kb,
+            1,
+            50};
     ASSERT_NO_FATAL_FAILURE(
         set_throttle(throttle_type::read_by_size,
                      (uint64_t)(plan.single_value_sz + test_hashkey_len + test_sortkey_len) *