gavinchou commented on code in PR #32228:
URL: https://github.com/apache/doris/pull/32228#discussion_r1527172066


##########
cloud/test/meta_service_test.cpp:
##########
@@ -5088,6 +5082,113 @@ TEST(MetaServiceTest, AddHdfsInfoTest) {
     SyncPoint::get_instance()->clear_all_call_backs();
 }
 
+TEST(MetaServiceTest, DropHdfsInfoTest) {
+    auto meta_service = get_meta_service();
+
+    auto sp = cloud::SyncPoint::get_instance();
+    sp->enable_processing();
+    sp->set_call_back("encrypt_ak_sk:get_encryption_key_ret",
+                      [](void* p) { *reinterpret_cast<int*>(p) = 0; });
+    sp->set_call_back("encrypt_ak_sk:get_encryption_key", [](void* p) {
+        *reinterpret_cast<std::string*>(p) = 
"selectdbselectdbselectdbselectdb";
+    });
+    sp->set_call_back("encrypt_ak_sk:get_encryption_key_id",
+                      [](void* p) { *reinterpret_cast<int*>(p) = 1; });
+
+    std::unique_ptr<Transaction> txn;
+    ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK);
+    std::string key;
+    std::string val;
+    InstanceKeyInfo key_info {"test_instance"};
+    instance_key(key_info, &key);
+
+    ObjectStoreInfoPB obj_info;
+    obj_info.set_id("1");
+    obj_info.set_ak("ak");
+    obj_info.set_sk("sk");
+    StorageVaultPB vault;
+    vault.set_name("test_hdfs_vault");
+    vault.set_id("2");
+    InstanceInfoPB instance;
+    instance.add_obj_info()->CopyFrom(obj_info);
+    instance.add_storage_vault_names(vault.name());
+    instance.add_resource_ids(vault.id());
+    val = instance.SerializeAsString();
+    txn->put(key, val);
+    txn->put(storage_vault_key({instance.instance_id(), "2"}), 
vault.SerializeAsString());
+    ASSERT_EQ(txn->commit(), TxnErrorCode::TXN_OK);
+
+    auto get_test_instance = [&](InstanceInfoPB& i) {
+        std::string key;
+        std::string val;
+        std::unique_ptr<Transaction> txn;
+        ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), 
TxnErrorCode::TXN_OK);
+        InstanceKeyInfo key_info {"test_instance"};
+        instance_key(key_info, &key);
+        ASSERT_EQ(txn->get(key, &val), TxnErrorCode::TXN_OK);
+        i.ParseFromString(val);
+    };
+
+    // update failed because has no storage vault set
+    {
+        AlterObjStoreInfoRequest req;
+        req.set_cloud_unique_id("test_cloud_unique_id");
+        req.set_op(AlterObjStoreInfoRequest::DROP_HDFS_INFO);
+
+        brpc::Controller cntl;
+        AlterObjStoreInfoResponse res;
+        meta_service->alter_obj_store_info(
+                reinterpret_cast<::google::protobuf::RpcController*>(&cntl), 
&req, &res, nullptr);
+        ASSERT_EQ(res.status().code(), MetaServiceCode::INVALID_ARGUMENT) << 
res.status().msg();
+    }
+
+    // update failed because vault name does not exist
+    {
+        AlterObjStoreInfoRequest req;
+        req.set_cloud_unique_id("test_cloud_unique_id");
+        req.set_op(AlterObjStoreInfoRequest::DROP_HDFS_INFO);
+        StorageVaultPB hdfs;
+        hdfs.set_name("test_hdfs_vault_not_found");
+        HdfsVaultInfo params;
+
+        hdfs.mutable_hdfs_info()->CopyFrom(params);
+        req.mutable_hdfs()->CopyFrom(hdfs);
+
+        brpc::Controller cntl;
+        AlterObjStoreInfoResponse res;
+        meta_service->alter_obj_store_info(
+                reinterpret_cast<::google::protobuf::RpcController*>(&cntl), 
&req, &res, nullptr);
+        ASSERT_EQ(res.status().code(), 
MetaServiceCode::STORAGE_VAULT_NOT_FOUND)
+                << res.status().msg();
+    }
+
+    // update successfully
+    {
+        AlterObjStoreInfoRequest req;
+        req.set_cloud_unique_id("test_cloud_unique_id");
+        req.set_op(AlterObjStoreInfoRequest::DROP_HDFS_INFO);
+        StorageVaultPB hdfs;
+        hdfs.set_name("test_hdfs_vault");
+        HdfsVaultInfo params;
+
+        hdfs.mutable_hdfs_info()->CopyFrom(params);
+        req.mutable_hdfs()->CopyFrom(hdfs);
+
+        brpc::Controller cntl;
+        AlterObjStoreInfoResponse res;
+        meta_service->alter_obj_store_info(
+                reinterpret_cast<::google::protobuf::RpcController*>(&cntl), 
&req, &res, nullptr);
+        ASSERT_EQ(res.status().code(), MetaServiceCode::OK) << 
res.status().msg();
+        InstanceInfoPB instance;
+        get_test_instance(instance);
+        ASSERT_EQ(instance.resource_ids().size(), 0);
+        ASSERT_EQ(instance.storage_vault_names().size(), 0);

Review Comment:
   read the InstanceInfoPB again and read  the removed key to ensure all things 
go right



##########
cloud/src/meta-service/meta_service_resource.cpp:
##########
@@ -322,16 +328,32 @@ static int add_hdfs_storage_valut(InstanceInfoPB& 
instance, Transaction* txn,
     hdfs_param.set_id(vault_id);
     std::string val = hdfs_param.SerializeAsString();
     txn->put(key, val);
-    LOG_INFO("try to put storage vault_id={}, vault_name={}, err={}", 
vault_id, hdfs_param.name());
+    LOG_INFO("try to put storage vault_id={}, vault_name={}", vault_id, 
hdfs_param.name());
     instance.mutable_resource_ids()->Add(std::move(vault_id));
     *instance.mutable_storage_vault_names()->Add() = hdfs_param.name();
     return 0;
 }
 
-// TODO(ByteYue): Implement drop storage vault.
-[[maybe_unused]] static int remove_hdfs_storage_valut(std::string_view 
vault_key, Transaction* txn,
-                                                      MetaServiceCode& code, 
std::string& msg) {
+static int remove_hdfs_storage_valut(InstanceInfoPB& instance, Transaction* 
txn,

Review Comment:
   typo remove_hdfs_storage_valut



-- 
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