gavinchou commented on code in PR #43253: URL: https://github.com/apache/doris/pull/43253#discussion_r1830463552
########## cloud/src/resource-manager/resource_manager.h: ########## @@ -114,6 +114,25 @@ class ResourceManager { bool check_cluster_params_valid(const ClusterPB& cluster, std::string* err, bool check_master_num); + /** + * Check cloud_unique_id is degraded format, and get instance_id from cloud_unique_id + * degraded format : "${version}:${instance_id}:${unique_id}" + * @param degraded cloud_unique_id + * + * @return a <is_degraded_format, instance_id> pair, if is_degraded_format == true , instance_id, if is_degraded_format == false, instance_id="" + */ + std::pair<bool, std::string> get_instance_id_by_degrade_unique_id( Review Comment: naming "get_instance_id_by_cloud_unique_id()" and make it a static member function ########## cloud/src/meta-service/meta_service.cpp: ########## @@ -88,46 +88,52 @@ std::string get_instance_id(const std::shared_ptr<ResourceManager>& rc_mgr, std::vector<NodeInfo> nodes; std::string err = rc_mgr->get_node(cloud_unique_id, &nodes); { TEST_SYNC_POINT_CALLBACK("get_instance_id_err", &err); } + std::string instance_id; if (!err.empty()) { // cache can't find cloud_unique_id, so degraded by parse cloud_unique_id // cloud_unique_id encode: ${version}:${instance_id}:${unique_id} // check it split by ':' c - auto vec = split(cloud_unique_id, ':'); - std::stringstream ss; - for (int i = 0; i < vec.size(); ++i) { - ss << "idx " << i << "= [" << vec[i] << "] "; - } - LOG(INFO) << "degraded to get instance_id, cloud_unique_id: " << cloud_unique_id - << "after split: " << ss.str(); - if (vec.size() != 3) { - LOG(WARNING) << "cloud unique id is not degraded format, failed to check instance " - "info, cloud_unique_id=" - << cloud_unique_id << " , err=" << err; + auto [valid, instance_id] = rc_mgr->get_instance_id_by_degrade_unique_id(cloud_unique_id); Review Comment: dangrous shadow variable ########## cloud/src/resource-manager/resource_manager.h: ########## @@ -114,6 +114,25 @@ class ResourceManager { bool check_cluster_params_valid(const ClusterPB& cluster, std::string* err, bool check_master_num); + /** + * Check cloud_unique_id is degraded format, and get instance_id from cloud_unique_id + * degraded format : "${version}:${instance_id}:${unique_id}" + * @param degraded cloud_unique_id + * + * @return a <is_degraded_format, instance_id> pair, if is_degraded_format == true , instance_id, if is_degraded_format == false, instance_id="" + */ + std::pair<bool, std::string> get_instance_id_by_degrade_unique_id( + const std::string& cloud_unique_id); + + /** + * check instance_id is a valid instance, check by get fdb kv + * + * @param instance_id + * + * @return true, instance_id in fdb kv + */ + bool check_degrade_instance_valid(const std::string& instance_id); Review Comment: naming "is_instance_id_valid()" or "is_instance_id_registered()" ########## cloud/src/resource-manager/resource_manager.cpp: ########## @@ -199,6 +211,28 @@ bool ResourceManager::check_cluster_params_valid(const ClusterPB& cluster, std:: return no_err; } +std::pair<bool, std::string> ResourceManager::get_instance_id_by_degrade_unique_id( + const std::string& cloud_unique_id) { + auto v = split(cloud_unique_id, ':'); + if (v.size() != 3) return {false, ""}; + // degraded format check it + int version = std::atoi(v[0].c_str()); + if (version != 1) return {false, ""}; + return {true, v[1]}; +} + +bool ResourceManager::check_degrade_instance_valid(const std::string& instance_id) { + // check kv + auto [c0, m0] = get_instance(nullptr, instance_id, nullptr); + { TEST_SYNC_POINT_CALLBACK("check_degrade_instance_valid", &c0); } + if (c0 != TxnErrorCode::TXN_OK) { + LOG(WARNING) << "check instance instance_id=" << instance_id + << " failed, code=" << format_as(c0) << ", info=" + m0; + return false; + } + return true; Review Comment: ```suggestion if (c0 != TxnErrorCode::TXN_OK) { LOG(WARNING) << "failed to check instance instance_id=" << instance_id << " expected= " << ... << " given=" << ... << " failed, code=" << format_as(c0) << ", info=" + m0; } return c0 == TxnErrorCode::TXN_OK; ``` -- 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: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org