Fix the fail start of tiflash because of the zero division when getti…

…ng cpu number (#9602) (#9625) close #9212 Co-authored-by: xzhangxian1008 <[email protected]> Co-authored-by: JaySon <[email protected]>
pingcap · Nov 21, 2024 · f36d293 · f36d293
1 parent 30683ef
commit f36d293
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 2 deletions.
diff --git a/dbms/src/Common/getNumberOfCPUCores.cpp b/dbms/src/Common/getNumberOfCPUCores.cpp
@@ -46,6 +46,8 @@ void computeAndSetNumberOfPhysicalCPUCores(
     UInt16 number_of_logical_cpu_cores_,
     UInt16 number_of_hardware_physical_cores)
 {
+    number_of_hardware_physical_cores = std::max<UInt16>(1, number_of_hardware_physical_cores);
+
     // First of all, we need to take consideration of two situation:
     //   1. tiflash on physical machine.
     //      In old codes, tiflash needs to set max_threads which is equal to
@@ -62,8 +64,11 @@ void computeAndSetNumberOfPhysicalCPUCores(
     // - `(hardware_logical_cpu_cores / number_of_hardware_physical_cores)` means how many logical cpu core a physical cpu core has.
     // - `number_of_logical_cpu_cores_ / (hardware_logical_cpu_cores / number_of_hardware_physical_cores)` means how many physical cpu cores the tiflash process could use. (Actually, it's needless to get physical cpu cores in virtual environment, but we must ensure the behavior `1` is not broken)
     auto hardware_logical_cpu_cores = std::thread::hardware_concurrency();
-    UInt16 physical_cpu_cores
-        = number_of_logical_cpu_cores_ / (hardware_logical_cpu_cores / number_of_hardware_physical_cores);
+
+    UInt16 thread_num_per_physical_core = hardware_logical_cpu_cores / number_of_hardware_physical_cores;
+    thread_num_per_physical_core = std::max<UInt16>(1, thread_num_per_physical_core);
+
+    UInt16 physical_cpu_cores = number_of_logical_cpu_cores_ / thread_num_per_physical_core;
     CPUCores::number_of_physical_cpu_cores = physical_cpu_cores > 0 ? physical_cpu_cores : 1;
     LOG_INFO(
         DB::Logger::get(),

diff --git a/dbms/src/Common/tests/gtest_miscellaneous.cpp b/dbms/src/Common/tests/gtest_miscellaneous.cpp
@@ -0,0 +1,46 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <Common/getNumberOfCPUCores.h>
+#include <TestUtils/TiFlashTestBasic.h>
+
+namespace DB
+{
+namespace tests
+{
+
+TEST(CommonMiscellaneousTest, cpuRelated)
+{
+    auto hardware_logical_cpu_cores = std::thread::hardware_concurrency();
+    UInt16 number_of_logical_cpu_cores = std::thread::hardware_concurrency();
+
+    computeAndSetNumberOfPhysicalCPUCores(number_of_logical_cpu_cores, hardware_logical_cpu_cores);
+    ASSERT_EQ(getNumberOfPhysicalCPUCores(), hardware_logical_cpu_cores);
+
+    computeAndSetNumberOfPhysicalCPUCores(number_of_logical_cpu_cores, 0);
+    ASSERT_EQ(1, getNumberOfPhysicalCPUCores());
+
+    UInt16 test_num = 123;
+    computeAndSetNumberOfPhysicalCPUCores(test_num, hardware_logical_cpu_cores * 2);
+    ASSERT_EQ(test_num, getNumberOfPhysicalCPUCores());
+
+    computeAndSetNumberOfPhysicalCPUCores(1, hardware_logical_cpu_cores * 2);
+    ASSERT_EQ(1, getNumberOfPhysicalCPUCores());
+
+    computeAndSetNumberOfPhysicalCPUCores(0, hardware_logical_cpu_cores);
+    ASSERT_EQ(1, getNumberOfPhysicalCPUCores());
+}
+
+} // namespace tests
+} // namespace DB