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

Reply via email to