acelyc111 commented on code in PR #1935:
URL:
https://github.com/apache/incubator-pegasus/pull/1935#discussion_r1516055149
##########
src/replica/replica_stub.cpp:
##########
@@ -1904,57 +1904,73 @@ replica *replica_stub::new_replica(gpid gpid,
return rep;
}
-replica *replica_stub::load_replica(dir_node *dn, const char *dir)
+bool replica_stub::validate_replica_dir(const std::string &dir,
+ app_info &ai,
+ gpid &pid,
+ std::string &hint_message)
{
- FAIL_POINT_INJECT_F("mock_replica_load",
- [&](absl::string_view) -> replica * { return nullptr;
});
+ if (!utils::filesystem::directory_exists(dir)) {
+ hint_message = fmt::format("replica dir '{}' not exist", dir);
+ return false;
+ }
char splitters[] = {'\\', '/', 0};
- std::string name = utils::get_last_component(std::string(dir), splitters);
+ const auto name = utils::get_last_component(dir, splitters);
if (name.empty()) {
- LOG_ERROR("invalid replica dir {}", dir);
- return nullptr;
+ hint_message = fmt::format("invalid replica dir '{}'", dir);
+ return false;
}
- char app_type[128];
+ char app_type[128] = {0};
int32_t app_id, pidx;
if (3 != sscanf(name.c_str(), "%d.%d.%s", &app_id, &pidx, app_type)) {
- LOG_ERROR("invalid replica dir {}", dir);
- return nullptr;
+ hint_message = fmt::format("invalid replica dir '{}'", dir);
+ return false;
}
- gpid pid(app_id, pidx);
- if (!utils::filesystem::directory_exists(dir)) {
- LOG_ERROR("replica dir {} not exist", dir);
- return nullptr;
+ pid = gpid(app_id, pidx);
+ replica_app_info rai(&ai);
+ const auto ai_path = utils::filesystem::path_combine(dir,
replica::kAppInfo);
+ auto err = rai.load(ai_path);
+ if (ERR_OK != err) {
+ hint_message = fmt::format("load app-info from '{}' failed, err = {}",
ai_path, err);
+ return false;
}
- dsn::app_info info;
- replica_app_info info2(&info);
- std::string path = utils::filesystem::path_combine(dir,
replica_app_info::kAppInfo);
- auto err = info2.load(path);
- if (ERR_OK != err) {
- LOG_ERROR("load app-info from {} failed, err = {}", path, err);
- return nullptr;
+ if (ai.app_type != app_type) {
+ hint_message = fmt::format("unmatched app type '{}' for '{}'",
ai.app_type, ai_path);
+ return false;
}
- if (info.app_type != app_type) {
- LOG_ERROR("unmatched app type {} for {}", info.app_type, path);
- return nullptr;
+ if (ai.partition_count < pidx) {
Review Comment:
When online partition split aborted, the new partitions are garbages.
Suppose the parent table has 8 partitions (i.e., ai.partition_count is 8), the
replica directories with pidx of 8 ~ 15 are garbages if partition split
aborted, which means the directory with pidx 8 is garbage too.
This is an existed issue, we can fix it in another bugfix patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
