Skip to content

Commit

Permalink
Fix conflict
Browse files Browse the repository at this point in the history 
Signed-off-by: JaySon-Huang <[email protected]>
  • Loading branch information
@JaySon-Huang
JaySon-Huang committed Apr 14, 2022
1 parent 43d9885 commit af10523
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 121 deletions.
28 changes: 3 additions & 25 deletions dbms/src/Server/StorageConfigParser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,25 +86,18 @@ void TiFlashStorageConfig::parseStoragePath(const String & storage, Poco::Logger
if (main_data_paths.empty())
{
String error_msg = "The configuration \"storage.main.dir\" is empty. Please check your configuration file.";
LOG_FMT_ERROR(log, "{}", error_msg);
LOG_ERROR(log, error_msg);
throw Exception(error_msg, ErrorCodes::INVALID_CONFIG_PARAMETER);
}
if (!main_capacity_quota.empty() && main_capacity_quota.size() != main_data_paths.size())
{
<<<<<<< HEAD
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.";
LOG_ERROR(log, error_msg);
=======
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_FMT_ERROR(log, "{}", error_msg);
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
LOG_ERROR(log, error_msg);
throw Exception(error_msg, ErrorCodes::INVALID_CONFIG_PARAMETER);
}
for (size_t i = 0; i < main_data_paths.size(); ++i)
Expand Down Expand Up @@ -133,20 +126,13 @@ void TiFlashStorageConfig::parseStoragePath(const String & storage, Poco::Logger
}
if (!latest_capacity_quota.empty() && latest_capacity_quota.size() != latest_data_paths.size())
{
<<<<<<< HEAD
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.";
LOG_ERROR(log, error_msg);
=======
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_FMT_ERROR(log, "{}", error_msg);
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
LOG_ERROR(log, error_msg);
throw Exception(error_msg, ErrorCodes::INVALID_CONFIG_PARAMETER);
}
for (size_t i = 0; i < latest_data_paths.size(); ++i)
Expand Down Expand Up @@ -317,17 +303,9 @@ std::tuple<size_t, TiFlashStorageConfig> TiFlashStorageConfig::parseSettings(Poc
kvstore_paths.emplace_back(getNormalizedPath(deprecated_kvstore_path));
for (size_t i = 0; i < kvstore_paths.size(); ++i)
{
<<<<<<< HEAD
LOG_WARNING(log,
"Raft data candidate path: "
<< kvstore_paths[i] << ". The path is overwritten by deprecated configuration for backward compatibility.");
=======
LOG_FMT_WARNING(
log,
"Raft data candidate path: {}. "
"The path is overwritten by deprecated configuration for backward compatibility.",
kvstore_path);
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
}
}
}
Expand Down
104 changes: 28 additions & 76 deletions dbms/src/Server/tests/gtest_storage_config.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,17 @@ 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() {}

protected:
Poco::Logger * log;
};

<<<<<<< HEAD
TEST_F(StorageConfig_test, MultiSSDSettings)
=======
TEST_F(StorageConfigTest, SimpleSinglePath)
try
{
Expand Down Expand Up @@ -91,7 +88,8 @@ dir=["/data0/tiflash"]
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);
PathCapacityMetrics path_capacity(global_capacity_quota, storage.main_data_paths, storage.main_capacity_quota,
storage.latest_data_paths, storage.latest_capacity_quota);
}
}
CATCH
Expand Down Expand Up @@ -150,13 +148,13 @@ dir=["/data222/kvstore"]
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);
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)
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
try
{
Strings tests = {
Expand Down Expand Up @@ -201,7 +199,8 @@ dir=["/data0/tiflash"]
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);
PathCapacityMetrics path_capacity(global_capacity_quota, storage.main_data_paths, storage.main_capacity_quota,
storage.latest_data_paths, storage.latest_capacity_quota);
}
}
CATCH
Expand Down Expand Up @@ -229,7 +228,7 @@ dir=["/data0/tiflash", "/data1/tiflash", "/data2/tiflash"]
const auto & test_case = tests[i];
auto config = loadConfigFromString(test_case);

LOG_FMT_INFO(log, "parsing [index={}] [content={}]", i, test_case);
LOG_INFO(log, "parsing [index=" << i << "] [content=" << test_case << "]");

size_t global_capacity_quota = 0;
TiFlashStorageConfig storage;
Expand Down Expand Up @@ -310,7 +309,7 @@ dir=["/ssd0/tiflash"]
}
CATCH

TEST_F(StorageConfig_test, ParseMaybeBrokenCases)
TEST_F(StorageConfigTest, ParseMaybeBrokenCases)
try
{
Strings tests = {
Expand Down Expand Up @@ -349,15 +348,12 @@ path = "/data0/tiflash,/data1/tiflash"
[storage.main]
dir = [ "/data0/tiflash", "/data1/tiflash" ]
capacity = [ 10737418240 ]
<<<<<<< HEAD
# [storage.latest]
# dir = [ ]
# capacity = [ 10737418240, 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"
Expand Down Expand Up @@ -409,7 +405,6 @@ dir = [["/data0/tiflash", "/data1/tiflash"], ["/data2/tiflash", ]]
[storage.main]
dir = [1,2,3]
)",
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
};

for (size_t i = 0; i < tests.size(); ++i)
Expand Down Expand Up @@ -493,23 +488,13 @@ capacity=[ 1024 ]
ASSERT_NE(idx, PathCapacityMetrics::INVALID_INDEX);
switch (i)
{
<<<<<<< HEAD
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;
=======
case 0:
case 1:
EXPECT_EQ(path_capacity.path_infos[idx].capacity_bytes, 0);
break;
case 2:
EXPECT_EQ(path_capacity.path_infos[idx].capacity_bytes, 2048);
break;
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
}
idx = path_capacity.locatePath("/data1/tiflash/");
ASSERT_NE(idx, PathCapacityMetrics::INVALID_INDEX);
Expand Down Expand Up @@ -629,7 +614,7 @@ dt_enable_rough_set_filter = false
}
CATCH

TEST_F(StorageConfig_test, CompatibilityWithIORateLimitConfig)
TEST_F(StorageConfigTest, CompatibilityWithIORateLimitConfig)
try
{
Strings tests = {
Expand Down Expand Up @@ -736,12 +721,7 @@ background_read_weight=2

Poco::Logger * log = &Poco::Logger::get("StorageIORateLimitConfig_test");

<<<<<<< HEAD
auto verifyDefault = [](const StorageIORateLimitConfig& io_config)
{
=======
auto verify_default = [](const StorageIORateLimitConfig & io_config) {
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
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);
Expand All @@ -753,18 +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);
};

<<<<<<< HEAD
auto verifyCase0 = [](const StorageIORateLimitConfig& io_config)
{
=======
auto verify_case0 = [](const StorageIORateLimitConfig & io_config) {
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
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);
Expand All @@ -776,18 +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);
};

<<<<<<< HEAD
auto verifyCase1 = [](const StorageIORateLimitConfig& io_config)
{
=======
auto verify_case1 = [](const StorageIORateLimitConfig & io_config) {
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
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);
Expand All @@ -801,16 +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);
};

<<<<<<< HEAD
auto verifyCase2 = [](const StorageIORateLimitConfig& io_config)
{
=======
auto verify_case2 = [](const StorageIORateLimitConfig & io_config) {
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
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);
Expand All @@ -822,18 +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);
};

<<<<<<< HEAD
auto verifyCase3 = [](const StorageIORateLimitConfig& io_config)
{
=======
auto verify_case3 = [](const StorageIORateLimitConfig & io_config) {
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
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);
Expand All @@ -845,25 +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);
};

<<<<<<< HEAD
std::vector<std::function<void(const StorageIORateLimitConfig&)>> case_verifiers;
case_verifiers.push_back(verifyCase0);
case_verifiers.push_back(verifyCase1);
case_verifiers.push_back(verifyCase2);
case_verifiers.push_back(verifyCase3);
=======
std::vector<std::function<void(const StorageIORateLimitConfig &)>> 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);
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))

for (size_t i = 0; i < 2u /*tests.size()*/; ++i)
{
Expand All @@ -872,7 +824,7 @@ background_read_weight=2

LOG_INFO(log, "parsing [index=" << i << "] [content=" << test_case << "]");
ASSERT_TRUE(config->has("storage.io_rate_limit"));

StorageIORateLimitConfig io_config;
verify_default(io_config);
io_config.parse(config->getString("storage.io_rate_limit"), log);
Expand Down
20 changes: 0 additions & 20 deletions dbms/src/Storages/PathCapacityMetrics.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,7 @@ FsStats PathCapacityMetrics::getFsStats() const

// Now we assume the size of `path_infos` will not change, don't acquire heavy lock on `path_infos`.
FsStats total_stat{};
<<<<<<< HEAD
for (size_t i = 0; i < path_infos.size(); ++i)
=======

// Build the disk stats map
// which use to measure single disk capacity and available size
auto disk_stats_map = getDiskStats();

for (auto & fs_it : disk_stats_map)
>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
{
FsStats path_stat = path_infos[i].getStats(log);
if (!path_stat.ok)
Expand All @@ -120,17 +111,6 @@ FsStats PathCapacityMetrics::getFsStats() const
return total_stat;
}

<<<<<<< HEAD
=======
const uint64_t disk_capacity_size = vfs_info.f_blocks * vfs_info.f_frsize;
if (disk_stat.capacity_size == 0 || disk_capacity_size < disk_stat.capacity_size)
disk_stat.capacity_size = disk_capacity_size;

// Calculate single disk info
const uint64_t disk_free_bytes = vfs_info.f_bavail * vfs_info.f_frsize;
disk_stat.avail_size = std::min(disk_free_bytes, disk_stat.avail_size);

>>>>>>> e50c06c46d (Fix invalid storage dir configurations lead to unexpected behavior (#4105))
// sum of all path's capacity and used_size
total_stat.capacity_size += path_stat.capacity_size;
total_stat.used_size += path_stat.used_size;
Expand Down

0 comments on commit af10523

Please sign in to comment.