From 1020064f84cab9ce937c7b269060812e8a72b91f Mon Sep 17 00:00:00 2001 From: Ti Chi Robot Date: Thu, 14 Apr 2022 21:44:36 +0800 Subject: [PATCH] Fix invalid storage dir configurations lead to unexpected behavior (#4105) (#4133) close pingcap/tics#4093 --- dbms/src/Common/tests/gtest_cpptoml.cpp | 149 ++++++++ dbms/src/Server/StorageConfigParser.cpp | 53 ++- .../src/Server/tests/gtest_storage_config.cpp | 321 +++++++++++++++--- dbms/src/Storages/PathCapacityMetrics.cpp | 4 +- 4 files changed, 462 insertions(+), 65 deletions(-) create mode 100644 dbms/src/Common/tests/gtest_cpptoml.cpp diff --git a/dbms/src/Common/tests/gtest_cpptoml.cpp b/dbms/src/Common/tests/gtest_cpptoml.cpp new file mode 100644 index 00000000000..5cc43aa6fdc --- /dev/null +++ b/dbms/src/Common/tests/gtest_cpptoml.cpp @@ -0,0 +1,149 @@ +#include +#include +/// Suppress gcc warning: ‘*((void*)& +4)’ may be used uninitialized in this function +#if !__clang__ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif +#include +#if !__clang__ +#pragma GCC diagnostic pop +#endif + +#include + +namespace DB::tests +{ + +TEST(CPPTomlTest, ContainsQualifiedArray) +{ + auto * log = &Poco::Logger::get("CPPTomlTest"); + + Strings failure_tests = { + R"( +[a] +[a.b] +c = "abc" + )", + R"( +[a] +[a.b] +c = 123 + )", + R"( +[a] +[a.b] +c = 123.45 + )", + }; + + for (size_t i = 0; i < failure_tests.size(); ++i) + { + const auto & test_case = failure_tests[i]; + SCOPED_TRACE(fmt::format("[index={}] [content={}]", i, test_case)); + LOG_INFO(log, "parsing [index=" << i << "] [content=" << test_case << "]"); + + std::istringstream ss(test_case); + cpptoml::parser p(ss); + auto table = p.parse(); + + const char * key = "a.b.c"; + ASSERT_TRUE(table->contains_qualified(key)); + auto qualified = table->get_qualified(key); + ASSERT_TRUE(qualified); + // not array + ASSERT_FALSE(qualified->is_array()); + // try to parse as vector, return false + cpptoml::option array = table->get_qualified_array_of(key); + ASSERT_FALSE(array); + } +} + +TEST(CPPTomlTest, ContainsQualifiedStringArray) +{ + auto * log = &Poco::Logger::get("CPPTomlTest"); + + Strings failure_tests = { + R"( +[a] +[a.b] +c = [["abc", "def"], ["z"]] + )", + R"( +[a] +[a.b] +c = [123, 456] + )", + }; + + for (size_t i = 0; i < failure_tests.size(); ++i) + { + const auto & test_case = failure_tests[i]; + SCOPED_TRACE(fmt::format("[index={}] [content={}]", i, test_case)); + LOG_INFO(log, "parsing [index=" << i << "] [content=" << test_case << "]"); + + std::istringstream ss(test_case); + cpptoml::parser p(ss); + auto table = p.parse(); + + const char * key = "a.b.c"; + ASSERT_TRUE(table->contains_qualified(key)); + auto qualified = table->get_qualified(key); + ASSERT_TRUE(qualified); + // is non-empty array but not string array + ASSERT_TRUE(qualified->is_array()); + auto qualified_array = qualified->as_array(); + ASSERT_NE(qualified_array->begin(), qualified_array->end()); + // try to parse as vector, return false + cpptoml::option array = table->get_qualified_array_of(key); + ASSERT_FALSE(array); + } +} + +TEST(CPPTomlTest, ContainsQualifiedStringArrayOrEmpty) +{ + auto * log = &Poco::Logger::get("CPPTomlTest"); + + Strings failure_tests = { + // a.b.c is not empty + R"( +[a] +[a.b] +c = ["abc", "def", "z"] + )", + // a.b.c is empty + R"( +[a] +[a.b] +c = [] + )", + }; + + for (size_t i = 0; i < failure_tests.size(); ++i) + { + const auto & test_case = failure_tests[i]; + SCOPED_TRACE(fmt::format("[index={}] [content={}]", i, test_case)); + LOG_INFO(log, "parsing [index=" << i << "] [content=" << test_case << "]"); + + std::istringstream ss(test_case); + cpptoml::parser p(ss); + auto table = p.parse(); + + const char * key = "a.b.c"; + ASSERT_TRUE(table->contains_qualified(key)); + auto qualified = table->get_qualified(key); + ASSERT_TRUE(qualified); + // is non-empty array but not string array + ASSERT_TRUE(qualified->is_array()); + + // try to parse as vector, return true + cpptoml::option array = table->get_qualified_array_of(key); + ASSERT_TRUE(array); + if (auto qualified_array = qualified->as_array(); qualified_array->begin() != qualified_array->end()) + { + ASSERT_EQ(array->size(), 3); + } + } +} + +} // namespace DB::tests diff --git a/dbms/src/Server/StorageConfigParser.cpp b/dbms/src/Server/StorageConfigParser.cpp index b55681cf608..17eab377e79 100644 --- a/dbms/src/Server/StorageConfigParser.cpp +++ b/dbms/src/Server/StorageConfigParser.cpp @@ -50,8 +50,34 @@ void TiFlashStorageConfig::parseStoragePath(const String & storage, Poco::Logger cpptoml::parser p(ss); auto table = p.parse(); + auto get_checked_qualified_array = [log](const std::shared_ptr table, const char * key) -> cpptoml::option { + auto throw_invalid_value = [log, key]() { + String error_msg + = fmt::format("The configuration \"storage.{}\" should be an array of strings. Please check your configuration file.", key); + LOG_ERROR(log, error_msg); + throw Exception(error_msg, ErrorCodes::INVALID_CONFIG_PARAMETER); + }; + // not exist key + if (!table->contains_qualified(key)) + return cpptoml::option(); + + // key exist, but not array + auto qualified_ptr = table->get_qualified(key); + if (!qualified_ptr->is_array()) + { + throw_invalid_value(); + } + // key exist, but can not convert to string array, maybe it is an int array + auto string_array = table->get_qualified_array_of(key); + if (!string_array) + { + throw_invalid_value(); + } + return string_array; + }; + // main - if (auto main_paths = table->get_qualified_array_of("main.dir"); main_paths) + if (auto main_paths = get_checked_qualified_array(table, "main.dir"); main_paths) main_data_paths = *main_paths; if (auto main_capacity = table->get_qualified_array_of("main.capacity"); main_capacity) { @@ -66,9 +92,11 @@ void TiFlashStorageConfig::parseStoragePath(const String & storage, Poco::Logger } if (!main_capacity_quota.empty() && main_capacity_quota.size() != main_data_paths.size()) { - String error_msg = "The array size of \"storage.main.dir\"[size=" + toString(main_data_paths.size()) - + "] is not equal to \"storage.main.capacity\"[size=" + toString(main_capacity_quota.size()) - + "]. Please check your configuration file."; + String error_msg = fmt::format("The array size of \"storage.main.dir\"[size={}] " + "is not equal to \"storage.main.capacity\"[size={}]. " + "Please check your configuration file.", + main_data_paths.size(), + main_capacity_quota.size()); LOG_ERROR(log, error_msg); throw Exception(error_msg, ErrorCodes::INVALID_CONFIG_PARAMETER); } @@ -82,13 +110,14 @@ void TiFlashStorageConfig::parseStoragePath(const String & storage, Poco::Logger } // latest - if (auto latest_paths = table->get_qualified_array_of("latest.dir"); latest_paths) + if (auto latest_paths = get_checked_qualified_array(table, "latest.dir"); latest_paths) latest_data_paths = *latest_paths; if (auto latest_capacity = table->get_qualified_array_of("latest.capacity"); latest_capacity) { for (const auto & c : *latest_capacity) latest_capacity_quota.emplace_back((size_t)c); } + // If it is empty, use the same dir as "main.dir" if (latest_data_paths.empty()) { LOG_INFO(log, "The configuration \"storage.latest.dir\" is empty, use the same dir and capacity of \"storage.main.dir\""); @@ -97,9 +126,11 @@ void TiFlashStorageConfig::parseStoragePath(const String & storage, Poco::Logger } if (!latest_capacity_quota.empty() && latest_capacity_quota.size() != latest_data_paths.size()) { - String error_msg = "The array size of \"storage.main.dir\"[size=" + toString(latest_data_paths.size()) - + "] is not euqal to \"storage.main.capacity\"[size=" + toString(latest_capacity_quota.size()) - + "]. Please check your configuration file."; + String error_msg = fmt::format("The array size of \"storage.latest.dir\"[size={}] " + "is not equal to \"storage.latest.capacity\"[size={}]. " + "Please check your configuration file.", + latest_data_paths.size(), + latest_capacity_quota.size()); LOG_ERROR(log, error_msg); throw Exception(error_msg, ErrorCodes::INVALID_CONFIG_PARAMETER); } @@ -113,7 +144,7 @@ void TiFlashStorageConfig::parseStoragePath(const String & storage, Poco::Logger } // Raft - if (auto kvstore_paths = table->get_qualified_array_of("raft.dir"); kvstore_paths) + if (auto kvstore_paths = get_checked_qualified_array(table, "raft.dir"); kvstore_paths) kvstore_data_path = *kvstore_paths; if (kvstore_data_path.empty()) { @@ -307,7 +338,7 @@ std::tuple TiFlashStorageConfig::parseSettings(Poc if (!storage_config.parseFromDeprecatedConfiguration(config, log)) { // Can not parse from the deprecated configuration "path". - String msg = "The configuration \"storage\" section is not defined. Please check your configuration file."; + String msg = "The configuration \"storage.main\" section is not defined. Please check your configuration file."; LOG_ERROR(log, msg); throw Exception(msg, ErrorCodes::INVALID_CONFIG_PARAMETER); } @@ -335,7 +366,7 @@ void StorageIORateLimitConfig::parse(const String & storage_io_rate_limit, Poco: readConfig("foreground_write_weight", fg_write_weight); readConfig("background_write_weight", bg_write_weight); readConfig("foreground_read_weight", fg_read_weight); - readConfig("background_read_weight", bg_read_weight); + readConfig("background_read_weight", bg_read_weight); readConfig("emergency_pct", emergency_pct); readConfig("high_pct", high_pct); readConfig("medium_pct", medium_pct); diff --git a/dbms/src/Server/tests/gtest_storage_config.cpp b/dbms/src/Server/tests/gtest_storage_config.cpp index e15d96bc086..fbe75d2715c 100644 --- a/dbms/src/Server/tests/gtest_storage_config.cpp +++ b/dbms/src/Server/tests/gtest_storage_config.cpp @@ -32,10 +32,10 @@ static auto loadConfigFromString(const String & s) return config; } -class StorageConfig_test : public ::testing::Test +class StorageConfigTest : public ::testing::Test { public: - StorageConfig_test() : log(&Poco::Logger::get("StorageConfig_test")) {} + StorageConfigTest() : log(&Poco::Logger::get("StorageConfigTest")) {} static void SetUpTestCase() {} @@ -43,7 +43,118 @@ class StorageConfig_test : public ::testing::Test Poco::Logger * log; }; -TEST_F(StorageConfig_test, MultiSSDSettings) +TEST_F(StorageConfigTest, SimpleSinglePath) +try +{ + Strings tests = { + // Deprecated style + R"( +path="/data0/tiflash" + )", + // Deprecated style with capacity + R"( +path="/data0/tiflash" +capacity=1024000000 + )", + // New style + R"( +[storage] +[storage.main] +dir=["/data0/tiflash"] + )", + }; + + for (size_t i = 0; i < tests.size(); ++i) + { + const auto & test_case = tests[i]; + auto config = loadConfigFromString(test_case); + + LOG_INFO(log, "parsing [index=" << i << "] [content=" << test_case << "]"); + + size_t global_capacity_quota = 0; + TiFlashStorageConfig storage; + std::tie(global_capacity_quota, storage) = TiFlashStorageConfig::parseSettings(*config, log); + + ASSERT_EQ(storage.main_data_paths.size(), 1); + EXPECT_EQ(storage.main_data_paths[0], "/data0/tiflash/"); + + ASSERT_EQ(storage.latest_data_paths.size(), 1); + EXPECT_EQ(storage.latest_data_paths[0], "/data0/tiflash/"); + + ASSERT_EQ(storage.kvstore_data_path.size(), 1); + EXPECT_EQ(storage.kvstore_data_path[0], "/data0/tiflash/kvstore/"); + + auto all_paths = storage.getAllNormalPaths(); + EXPECT_EQ(all_paths[0], "/data0/tiflash/"); + + // Ensure that creating PathCapacityMetrics is OK. + PathCapacityMetrics path_capacity(global_capacity_quota, storage.main_data_paths, storage.main_capacity_quota, + storage.latest_data_paths, storage.latest_capacity_quota); + } +} +CATCH + +TEST_F(StorageConfigTest, ExplicitKVStorePath) +try +{ + Strings tests = { + // Deprecated style + R"( +path="/data0/tiflash" +[raft] +kvstore_path="/data1111/kvstore" + )", + // New style + R"( +[storage] +[storage.main] +dir=["/data0/tiflash"] +[storage.raft] +dir=["/data1111/kvstore"] + )", + // New style with remaining `raft.kvstore_path`, will be overwrite for backward compatibility + R"( +[raft] +kvstore_path="/data1111/kvstore" +[storage] +[storage.main] +dir=["/data0/tiflash"] +[storage.raft] +dir=["/data222/kvstore"] + )", + }; + + for (size_t i = 0; i < tests.size(); ++i) + { + const auto & test_case = tests[i]; + auto config = loadConfigFromString(test_case); + + LOG_INFO(log, "parsing [index=" << i << "] [content=" << test_case << "]"); + + size_t global_capacity_quota = 0; + TiFlashStorageConfig storage; + std::tie(global_capacity_quota, storage) = TiFlashStorageConfig::parseSettings(*config, log); + + ASSERT_EQ(storage.main_data_paths.size(), 1); + EXPECT_EQ(storage.main_data_paths[0], "/data0/tiflash/"); + + ASSERT_EQ(storage.latest_data_paths.size(), 1); + EXPECT_EQ(storage.latest_data_paths[0], "/data0/tiflash/"); + + ASSERT_EQ(storage.kvstore_data_path.size(), 1); + EXPECT_EQ(storage.kvstore_data_path[0], "/data1111/kvstore/"); + + auto all_paths = storage.getAllNormalPaths(); + EXPECT_EQ(all_paths[0], "/data0/tiflash/"); + + // Ensure that creating PathCapacityMetrics is OK. + PathCapacityMetrics path_capacity(global_capacity_quota, storage.main_data_paths, storage.main_capacity_quota, + storage.latest_data_paths, storage.latest_capacity_quota); + } +} +CATCH + +TEST_F(StorageConfigTest, MultiSSDSettings) try { Strings tests = { @@ -73,14 +184,17 @@ dir=["/data0/tiflash"] TiFlashStorageConfig storage; std::tie(global_capacity_quota, storage) = TiFlashStorageConfig::parseSettings(*config, log); - ASSERT_EQ(storage.main_data_paths.size(), 3UL); + ASSERT_EQ(storage.main_data_paths.size(), 3); EXPECT_EQ(storage.main_data_paths[0], "/data0/tiflash/"); EXPECT_EQ(storage.main_data_paths[1], "/data1/tiflash/"); EXPECT_EQ(storage.main_data_paths[2], "/data2/tiflash/"); - ASSERT_EQ(storage.latest_data_paths.size(), 1UL); + ASSERT_EQ(storage.latest_data_paths.size(), 1); EXPECT_EQ(storage.latest_data_paths[0], "/data0/tiflash/"); + ASSERT_EQ(storage.kvstore_data_path.size(), 1); + EXPECT_EQ(storage.kvstore_data_path[0], "/data0/tiflash/kvstore/"); + auto all_paths = storage.getAllNormalPaths(); EXPECT_EQ(all_paths[0], "/data0/tiflash/"); @@ -91,7 +205,61 @@ dir=["/data0/tiflash"] } CATCH -TEST_F(StorageConfig_test, SSD_HDD_Settings) +TEST_F(StorageConfigTest, MultiNVMeSSDSettings) +try +{ + Strings tests = { + R"( +[storage] +[storage.main] +dir=["/data0/tiflash", "/data1/tiflash", "/data2/tiflash"] + )", + R"( +[storage] +[storage.main] +dir=["/data0/tiflash", "/data1/tiflash", "/data2/tiflash"] +[storage.latest] +dir=["/data0/tiflash", "/data1/tiflash", "/data2/tiflash"] + )", + }; + + for (size_t i = 0; i < tests.size(); ++i) + { + const auto & test_case = tests[i]; + auto config = loadConfigFromString(test_case); + + LOG_INFO(log, "parsing [index=" << i << "] [content=" << test_case << "]"); + + size_t global_capacity_quota = 0; + TiFlashStorageConfig storage; + std::tie(global_capacity_quota, storage) = TiFlashStorageConfig::parseSettings(*config, log); + + ASSERT_EQ(storage.main_data_paths.size(), 3); + EXPECT_EQ(storage.main_data_paths[0], "/data0/tiflash/"); + EXPECT_EQ(storage.main_data_paths[1], "/data1/tiflash/"); + EXPECT_EQ(storage.main_data_paths[2], "/data2/tiflash/"); + + ASSERT_EQ(storage.latest_data_paths.size(), 3); + EXPECT_EQ(storage.latest_data_paths[0], "/data0/tiflash/"); + EXPECT_EQ(storage.latest_data_paths[1], "/data1/tiflash/"); + EXPECT_EQ(storage.latest_data_paths[2], "/data2/tiflash/"); + + ASSERT_EQ(storage.kvstore_data_path.size(), 3); + EXPECT_EQ(storage.kvstore_data_path[0], "/data0/tiflash/kvstore/"); + EXPECT_EQ(storage.kvstore_data_path[1], "/data1/tiflash/kvstore/"); + EXPECT_EQ(storage.kvstore_data_path[2], "/data2/tiflash/kvstore/"); + + auto all_paths = storage.getAllNormalPaths(); + EXPECT_EQ(all_paths[0], "/data0/tiflash/"); + + // Ensure that creating PathCapacityMetrics is OK. + PathCapacityMetrics path_capacity(global_capacity_quota, storage.main_data_paths, storage.main_capacity_quota, + storage.latest_data_paths, storage.latest_capacity_quota); + } +} +CATCH + +TEST_F(StorageConfigTest, SSD_HDD_Settings) try { Strings tests = { @@ -121,13 +289,16 @@ dir=["/ssd0/tiflash"] TiFlashStorageConfig storage; std::tie(global_capacity_quota, storage) = TiFlashStorageConfig::parseSettings(*config, log); - ASSERT_EQ(storage.main_data_paths.size(), 2UL); + ASSERT_EQ(storage.main_data_paths.size(), 2); EXPECT_EQ(storage.main_data_paths[0], "/hdd0/tiflash/"); EXPECT_EQ(storage.main_data_paths[1], "/hdd1/tiflash/"); - ASSERT_EQ(storage.latest_data_paths.size(), 1UL); + ASSERT_EQ(storage.latest_data_paths.size(), 1); EXPECT_EQ(storage.latest_data_paths[0], "/ssd0/tiflash/"); + ASSERT_EQ(storage.kvstore_data_path.size(), 1); + EXPECT_EQ(storage.kvstore_data_path[0], "/ssd0/tiflash/kvstore/"); + auto all_paths = storage.getAllNormalPaths(); EXPECT_EQ(all_paths[0], "/ssd0/tiflash/"); @@ -138,7 +309,7 @@ dir=["/ssd0/tiflash"] } CATCH -TEST_F(StorageConfig_test, ParseMaybeBrokenCases) +TEST_F(StorageConfigTest, ParseMaybeBrokenCases) try { Strings tests = { @@ -183,6 +354,57 @@ capacity = [ 10737418240 ] # [storage.raft] # dir = [ ] )", + // case for the length of storage.latest.dir is not the same with storage.latest.capacity + R"( +path = "/data0/tiflash,/data1/tiflash" +[storage] +[storage.main] +dir = [ "/data0/tiflash", "/data1/tiflash" ] +capacity = [ 10737418240, 10737418240 ] +[storage.latest] +dir = [ "/data0/tiflash", "/data1/tiflash" ] +capacity = [ 10737418240 ] + )", + // case for storage.main.dir is not an string array + R"( +[storage] +[storage.main] +dir = "/data0/tiflash,/data1/tiflash" + )", + // case for storage.latest.dir is not an string array + R"( +[storage] +[storage.main] +dir = [ "/data0/tiflash", "/data1/tiflash" ] +[storage.latest] +dir = "/data0/tiflash" + )", + // case for storage.raft.dir is not an string array + R"( +[storage] +[storage.main] +dir = [ "/data0/tiflash", "/data1/tiflash" ] +[storage.raft] +dir = "/data0/tiflash" + )", + // case for storage.main.dir is not an string array + R"( +[storage] +[storage.main] +dir = 123 + )", + // case for storage.main.dir is not an string array + R"( +[storage] +[storage.main] +dir = [["/data0/tiflash", "/data1/tiflash"], ["/data2/tiflash", ]] + )", + // case for storage.main.dir is not an string array + R"( +[storage] +[storage.main] +dir = [1,2,3] + )", }; for (size_t i = 0; i < tests.size(); ++i) @@ -199,7 +421,7 @@ capacity = [ 10737418240 ] } CATCH -TEST(PathCapacityMetrics_test, Quota) +TEST(PathCapacityMetricsTest, Quota) try { Strings tests = { @@ -234,7 +456,7 @@ dir=["/data0/tiflash"] capacity=[ 1024 ] )", }; - Poco::Logger * log = &Poco::Logger::get("PathCapacityMetrics_test"); + Poco::Logger * log = &Poco::Logger::get("PathCapacityMetricsTest"); for (size_t i = 0; i < tests.size(); ++i) { @@ -247,12 +469,12 @@ capacity=[ 1024 ] TiFlashStorageConfig storage; std::tie(global_capacity_quota, storage) = TiFlashStorageConfig::parseSettings(*config, log); - ASSERT_EQ(storage.main_data_paths.size(), 3UL); + ASSERT_EQ(storage.main_data_paths.size(), 3); EXPECT_EQ(storage.main_data_paths[0], "/data0/tiflash/"); EXPECT_EQ(storage.main_data_paths[1], "/data1/tiflash/"); EXPECT_EQ(storage.main_data_paths[2], "/data2/tiflash/"); - ASSERT_EQ(storage.latest_data_paths.size(), 1UL); + ASSERT_EQ(storage.latest_data_paths.size(), 1); EXPECT_EQ(storage.latest_data_paths[0], "/data0/tiflash/"); auto all_paths = storage.getAllNormalPaths(); @@ -268,26 +490,26 @@ capacity=[ 1024 ] { case 0: case 1: - EXPECT_EQ(path_capacity.path_infos[idx].capacity_bytes, 0UL); + EXPECT_EQ(path_capacity.path_infos[idx].capacity_bytes, 0); break; case 2: - EXPECT_EQ(path_capacity.path_infos[idx].capacity_bytes, 2048UL); + EXPECT_EQ(path_capacity.path_infos[idx].capacity_bytes, 2048); break; } idx = path_capacity.locatePath("/data1/tiflash/"); ASSERT_NE(idx, PathCapacityMetrics::INVALID_INDEX); - EXPECT_EQ(path_capacity.path_infos[idx].capacity_bytes, 3072UL); + EXPECT_EQ(path_capacity.path_infos[idx].capacity_bytes, 3072); idx = path_capacity.locatePath("/data2/tiflash/"); ASSERT_NE(idx, PathCapacityMetrics::INVALID_INDEX); - EXPECT_EQ(path_capacity.path_infos[idx].capacity_bytes, 4196UL); + EXPECT_EQ(path_capacity.path_infos[idx].capacity_bytes, 4196); } } CATCH -class UsersConfigParser_test : public ::testing::Test +class UsersConfigParserTest : public ::testing::Test { public: - UsersConfigParser_test() : log(&Poco::Logger::get("UsersConfigParser_test")) {} + UsersConfigParserTest() : log(&Poco::Logger::get("UsersConfigParserTest")) {} static void SetUpTestCase() {} @@ -296,7 +518,7 @@ class UsersConfigParser_test : public ::testing::Test }; -TEST_F(UsersConfigParser_test, ParseConfigs) +TEST_F(UsersConfigParserTest, ParseConfigs) try { Strings tests = { @@ -392,7 +614,7 @@ dt_enable_rough_set_filter = false } CATCH -TEST_F(StorageConfig_test, CompatibilityWithIORateLimitConfig) +TEST_F(StorageConfigTest, CompatibilityWithIORateLimitConfig) try { Strings tests = { @@ -499,8 +721,7 @@ background_read_weight=2 Poco::Logger * log = &Poco::Logger::get("StorageIORateLimitConfig_test"); - auto verifyDefault = [](const StorageIORateLimitConfig& io_config) - { + auto verify_default = [](const StorageIORateLimitConfig & io_config) { ASSERT_EQ(io_config.max_bytes_per_sec, 0); ASSERT_EQ(io_config.max_read_bytes_per_sec, 0); ASSERT_EQ(io_config.max_write_bytes_per_sec, 0); @@ -512,14 +733,13 @@ background_read_weight=2 ASSERT_EQ(io_config.readWeight(), 50); ASSERT_EQ(io_config.writeWeight(), 50); ASSERT_EQ(io_config.totalWeight(), 100); - ASSERT_EQ(io_config.getFgReadMaxBytesPerSec(), 0); - ASSERT_EQ(io_config.getFgWriteMaxBytesPerSec(), 0); - ASSERT_EQ(io_config.getBgReadMaxBytesPerSec(), 0); + ASSERT_EQ(io_config.getFgReadMaxBytesPerSec(), 0); + ASSERT_EQ(io_config.getFgWriteMaxBytesPerSec(), 0); + ASSERT_EQ(io_config.getBgReadMaxBytesPerSec(), 0); ASSERT_EQ(io_config.getBgWriteMaxBytesPerSec(), 0); }; - auto verifyCase0 = [](const StorageIORateLimitConfig& io_config) - { + auto verify_case0 = [](const StorageIORateLimitConfig & io_config) { ASSERT_EQ(io_config.max_bytes_per_sec, 0); ASSERT_EQ(io_config.max_read_bytes_per_sec, 0); ASSERT_EQ(io_config.max_write_bytes_per_sec, 0); @@ -531,14 +751,13 @@ background_read_weight=2 ASSERT_EQ(io_config.readWeight(), 7); ASSERT_EQ(io_config.writeWeight(), 3); ASSERT_EQ(io_config.totalWeight(), 10); - ASSERT_EQ(io_config.getFgReadMaxBytesPerSec(), 0); - ASSERT_EQ(io_config.getFgWriteMaxBytesPerSec(), 0); - ASSERT_EQ(io_config.getBgReadMaxBytesPerSec(), 0); + ASSERT_EQ(io_config.getFgReadMaxBytesPerSec(), 0); + ASSERT_EQ(io_config.getFgWriteMaxBytesPerSec(), 0); + ASSERT_EQ(io_config.getBgReadMaxBytesPerSec(), 0); ASSERT_EQ(io_config.getBgWriteMaxBytesPerSec(), 0); }; - auto verifyCase1 = [](const StorageIORateLimitConfig& io_config) - { + auto verify_case1 = [](const StorageIORateLimitConfig & io_config) { ASSERT_EQ(io_config.max_bytes_per_sec, 1024000); ASSERT_EQ(io_config.max_read_bytes_per_sec, 0); ASSERT_EQ(io_config.max_write_bytes_per_sec, 0); @@ -552,12 +771,11 @@ background_read_weight=2 ASSERT_EQ(io_config.totalWeight(), 10); ASSERT_EQ(io_config.getFgWriteMaxBytesPerSec(), 102400); ASSERT_EQ(io_config.getBgWriteMaxBytesPerSec(), 102400 * 2); - ASSERT_EQ(io_config.getFgReadMaxBytesPerSec(), 102400 * 5); - ASSERT_EQ(io_config.getBgReadMaxBytesPerSec(), 102400 * 2); + ASSERT_EQ(io_config.getFgReadMaxBytesPerSec(), 102400 * 5); + ASSERT_EQ(io_config.getBgReadMaxBytesPerSec(), 102400 * 2); }; - auto verifyCase2 = [](const StorageIORateLimitConfig& io_config) - { + auto verify_case2 = [](const StorageIORateLimitConfig & io_config) { ASSERT_EQ(io_config.max_bytes_per_sec, 0); ASSERT_EQ(io_config.max_read_bytes_per_sec, 1024000); ASSERT_EQ(io_config.max_write_bytes_per_sec, 1024000); @@ -569,14 +787,13 @@ background_read_weight=2 ASSERT_EQ(io_config.readWeight(), 7); ASSERT_EQ(io_config.writeWeight(), 3); ASSERT_EQ(io_config.totalWeight(), 10); - ASSERT_EQ(io_config.getFgReadMaxBytesPerSec(), 731428); - ASSERT_EQ(io_config.getFgWriteMaxBytesPerSec(), 341333); - ASSERT_EQ(io_config.getBgReadMaxBytesPerSec(), 292571); + ASSERT_EQ(io_config.getFgReadMaxBytesPerSec(), 731428); + ASSERT_EQ(io_config.getFgWriteMaxBytesPerSec(), 341333); + ASSERT_EQ(io_config.getBgReadMaxBytesPerSec(), 292571); ASSERT_EQ(io_config.getBgWriteMaxBytesPerSec(), 682666); }; - auto verifyCase3 = [](const StorageIORateLimitConfig& io_config) - { + auto verify_case3 = [](const StorageIORateLimitConfig & io_config) { ASSERT_EQ(io_config.max_bytes_per_sec, 1024000); ASSERT_EQ(io_config.max_read_bytes_per_sec, 1024000); ASSERT_EQ(io_config.max_write_bytes_per_sec, 1024000); @@ -588,17 +805,17 @@ background_read_weight=2 ASSERT_EQ(io_config.readWeight(), 7); ASSERT_EQ(io_config.writeWeight(), 3); ASSERT_EQ(io_config.totalWeight(), 10); - ASSERT_EQ(io_config.getFgReadMaxBytesPerSec(), 102400); - ASSERT_EQ(io_config.getFgWriteMaxBytesPerSec(), 102400 * 2); - ASSERT_EQ(io_config.getBgReadMaxBytesPerSec(), 102400 * 5); + ASSERT_EQ(io_config.getFgReadMaxBytesPerSec(), 102400); + ASSERT_EQ(io_config.getFgWriteMaxBytesPerSec(), 102400 * 2); + ASSERT_EQ(io_config.getBgReadMaxBytesPerSec(), 102400 * 5); ASSERT_EQ(io_config.getBgWriteMaxBytesPerSec(), 102400 * 2); }; - std::vector> case_verifiers; - case_verifiers.push_back(verifyCase0); - case_verifiers.push_back(verifyCase1); - case_verifiers.push_back(verifyCase2); - case_verifiers.push_back(verifyCase3); + std::vector> case_verifiers; + case_verifiers.push_back(verify_case0); + case_verifiers.push_back(verify_case1); + case_verifiers.push_back(verify_case2); + case_verifiers.push_back(verify_case3); for (size_t i = 0; i < 2u /*tests.size()*/; ++i) { @@ -607,9 +824,9 @@ background_read_weight=2 LOG_INFO(log, "parsing [index=" << i << "] [content=" << test_case << "]"); ASSERT_TRUE(config->has("storage.io_rate_limit")); - + StorageIORateLimitConfig io_config; - verifyDefault(io_config); + verify_default(io_config); io_config.parse(config->getString("storage.io_rate_limit"), log); case_verifiers[i](io_config); } diff --git a/dbms/src/Storages/PathCapacityMetrics.cpp b/dbms/src/Storages/PathCapacityMetrics.cpp index 8454cf81399..5581c7b9fd3 100644 --- a/dbms/src/Storages/PathCapacityMetrics.cpp +++ b/dbms/src/Storages/PathCapacityMetrics.cpp @@ -75,7 +75,7 @@ void PathCapacityMetrics::addUsedSize(std::string_view file_path, size_t used_by return; } - // Now we expect size of path_infos not change, don't acquire hevay lock on `path_infos` now. + // Now we expect size of path_infos not change, don't acquire heavy lock on `path_infos` now. path_infos[path_idx].used_bytes += used_bytes; } @@ -88,7 +88,7 @@ void PathCapacityMetrics::freeUsedSize(std::string_view file_path, size_t used_b return; } - // Now we expect size of path_infos not change, don't acquire hevay lock on `path_infos` now. + // Now we expect size of path_infos not change, don't acquire heavy lock on `path_infos` now. path_infos[path_idx].used_bytes -= used_bytes; }