Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

Commit

Permalink
update by cr
Browse files Browse the repository at this point in the history
  • Loading branch information
@hycdong
hycdong committed May 27, 2021
1 parent 9ce71a1 commit fe94dcc
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 41 deletions.
23 changes: 7 additions & 16 deletions src/common/replication_common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,7 @@ void replication_options::initialize()
dsn_config_get_value_string("replication", "data_dirs", "", "replica directory list");
std::vector<std::string> config_data_dirs;
std::vector<std::string> config_data_dir_tags;
std::string error_msg = "";
bool flag = get_data_dir_and_tag(
dirs_str, app_dir, app_name, config_data_dirs, config_data_dir_tags, error_msg);
dassert_f(flag, error_msg);
get_data_dir_and_tag(dirs_str, app_dir, app_name, config_data_dirs, config_data_dir_tags);

// check if data_dir in black list, data_dirs doesn't contain dir in black list
std::string black_list_file =
Expand Down Expand Up @@ -498,13 +495,12 @@ void replica_helper::load_meta_servers(/*out*/ std::vector<dsn::rpc_address> &se
dassert(servers.size() > 0, "no meta server specified in config [%s].%s", section, key);
}

/*static*/ bool
/*static*/ void
replication_options::get_data_dir_and_tag(const std::string &config_dirs_str,
const std::string &default_dir,
const std::string &app_name,
/*out*/ std::vector<std::string> &data_dirs,
/*out*/ std::vector<std::string> &data_dir_tags,
/*out*/ std::string &err_msg)
/*out*/ std::vector<std::string> &data_dir_tags)
{
// - if {config_dirs_str} is empty (return true):
// - dir = {default_dir}
Expand All @@ -527,25 +523,21 @@ replication_options::get_data_dir_and_tag(const std::string &config_dirs_str,
std::vector<std::string> tag_and_dir;
utils::split_args(dir.c_str(), tag_and_dir, ':');
if (tag_and_dir.size() != 2) {
err_msg = fmt::format("invalid data_dir item({}) in config", dir);
return false;
dassert_f("invalid data_dir item({}) in config", dir);
}
if (tag_and_dir[0].empty() || tag_and_dir[1].empty()) {
err_msg = fmt::format("invalid data_dir item({}) in config", dir);
return false;
dassert_f("invalid data_dir item({}) in config", dir);
}
dir = utils::filesystem::path_combine(tag_and_dir[1], app_name);
for (unsigned i = 0; i < dir_tags.size(); ++i) {
if (dirs[i] == dir) {
err_msg = fmt::format("dir({}) and dir({}) conflict", dirs[i], dir);
return false;
dassert_f("dir({}) and dir({}) conflict", dirs[i], dir);
}
}
for (unsigned i = 0; i < dir_tags.size(); ++i) {
if (dir_tags[i] == tag_and_dir[0]) {
err_msg = fmt::format(
dassert_f(
"dir({}) and dir({}) have same tag({})", dirs[i], dir, tag_and_dir[0]);
return false;
}
}
dir_tags.push_back(tag_and_dir[0]);
Expand All @@ -558,7 +550,6 @@ replication_options::get_data_dir_and_tag(const std::string &config_dirs_str,
data_dirs.push_back(utils::filesystem::path_combine(dir, "reps"));
data_dir_tags.push_back(dir_tags[i]);
}
return true;
}

/*static*/ void
Expand Down
5 changes: 2 additions & 3 deletions src/common/replication_common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,11 @@ class replication_options
~replication_options();

void initialize();
static bool get_data_dir_and_tag(const std::string &config_dirs_str,
static void get_data_dir_and_tag(const std::string &config_dirs_str,
const std::string &default_dir,
const std::string &app_name,
/*out*/ std::vector<std::string> &data_dirs,
/*out*/ std::vector<std::string> &data_dir_tags,
/*out*/ std::string &err_msg);
/*out*/ std::vector<std::string> &data_dir_tags);
static void get_data_dirs_in_black_list(const std::string &fname,
/*out*/ std::vector<std::string> &dirs);
static bool check_if_in_black_list(const std::vector<std::string> &black_list_dir,
Expand Down
28 changes: 6 additions & 22 deletions src/common/test/replication_common_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,34 +31,18 @@ TEST(replication_common, get_data_dir_test)
{
std::vector<std::string> data_dirs;
std::vector<std::string> data_dir_tags;
std::string err_msg = "";

// Test cases:
// - default dir: ""
// - invalid dir:
// - "wrong_dir"
// - "tag:dir:wrong"
// - "tag:"
// - "tag1:disk,tag2,disk"
// - "tag:disk1,tag:disk2"
// - valid: "tag1:disk1,tag2:disk2"
struct get_data_dir_test
{
std::string data_dir_str;
bool expected_val;
} tests[] = {{"", true},
{"wrong_dir", false},
{"tag:dir:wrong", false},
{"tag:", false},
{"tag1:disk,tag2,disk", false},
{"tag:disk1,tag:disk2", false},
{"tag1:disk1,tag2:disk2", true}};
int32_t expected_length;
} tests[] = {{"", 1}, {"tag1:disk1", 1}, {"tag1:disk1, ", 1}, {"tag1:disk1,tag2:disk2", 2}};
for (const auto &test : tests) {
data_dirs.clear();
data_dir_tags.clear();
ASSERT_EQ(replication_options::get_data_dir_and_tag(
test.data_dir_str, "test_dir", "replica", data_dirs, data_dir_tags, err_msg),
test.expected_val);
replication_options::get_data_dir_and_tag(
test.data_dir_str, "test_dir", "replica", data_dirs, data_dir_tags);
ASSERT_EQ(data_dirs.size(), data_dir_tags.size());
ASSERT_EQ(data_dirs.size(), test.expected_length);
}
}

Expand Down

0 comments on commit fe94dcc

Please sign in to comment.