gavinchou commented on code in PR #37606:
URL: https://github.com/apache/doris/pull/37606#discussion_r1679361743
##########
cloud/src/common/bvars.cpp:
##########
@@ -50,6 +50,7 @@ BvarLatencyRecorderWithTag g_bvar_ms_drop_partition("ms",
"drop_partition");
BvarLatencyRecorderWithTag g_bvar_ms_get_tablet_stats("ms",
"get_tablet_stats");
BvarLatencyRecorderWithTag g_bvar_ms_get_obj_store_info("ms",
"get_obj_store_info");
BvarLatencyRecorderWithTag g_bvar_ms_alter_obj_store_info("ms",
"alter_obj_store_info");
+BvarLatencyRecorderWithTag g_bvar_ms_alter_storage_vault("ms",
"alter_obj_storage_vault");
Review Comment:
just call it `alter_storage_vault`
##########
cloud/src/meta-service/meta_service_resource.cpp:
##########
@@ -71,6 +71,7 @@ static int encrypt_ak_sk_helper(const std::string plain_ak,
const std::string pl
MetaServiceCode& code, std::string& msg) {
std::string key;
int64_t key_id;
+ LOG_INFO("enter encrypt_ak_sk_helper");
Review Comment:
Print something useful like plain_ak.
##########
cloud/src/meta-service/meta_service_resource.cpp:
##########
@@ -865,6 +951,201 @@ void
MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController* cont
}
}
+void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController*
controller,
+ const AlterObjStoreInfoRequest*
request,
+ AlterObjStoreInfoResponse* response,
+ ::google::protobuf::Closure* done) {
+ std::string ak, sk, bucket, prefix, endpoint, external_endpoint, region;
+ EncryptionInfoPB encryption_info;
+ AkSkPair cipher_ak_sk_pair;
+ RPC_PREPROCESS(alter_obj_store_info);
+ switch (request->op()) {
+ case AlterObjStoreInfoRequest::ADD_OBJ_INFO:
+ case AlterObjStoreInfoRequest::LEGACY_UPDATE_AK_SK:
+ case AlterObjStoreInfoRequest::UPDATE_AK_SK: {
+ auto tmp_desc =
+ ObjectStorageDesc {ak, sk, bucket, prefix, endpoint,
external_endpoint, region};
+ if (0 != extract_object_storage_info(request, code, msg, tmp_desc,
encryption_info,
+ cipher_ak_sk_pair)) {
+ return;
+ }
+ } break;
+ case AlterObjStoreInfoRequest::ADD_BUILT_IN_VAULT: {
Review Comment:
is this necessary for the lagacy `object_store` backends?
##########
cloud/src/meta-service/meta_service_resource.cpp:
##########
@@ -865,6 +951,201 @@ void
MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController* cont
}
}
+void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController*
controller,
+ const AlterObjStoreInfoRequest*
request,
+ AlterObjStoreInfoResponse* response,
+ ::google::protobuf::Closure* done) {
+ std::string ak, sk, bucket, prefix, endpoint, external_endpoint, region;
+ EncryptionInfoPB encryption_info;
+ AkSkPair cipher_ak_sk_pair;
+ RPC_PREPROCESS(alter_obj_store_info);
+ switch (request->op()) {
+ case AlterObjStoreInfoRequest::ADD_OBJ_INFO:
+ case AlterObjStoreInfoRequest::LEGACY_UPDATE_AK_SK:
+ case AlterObjStoreInfoRequest::UPDATE_AK_SK: {
+ auto tmp_desc =
+ ObjectStorageDesc {ak, sk, bucket, prefix, endpoint,
external_endpoint, region};
+ if (0 != extract_object_storage_info(request, code, msg, tmp_desc,
encryption_info,
+ cipher_ak_sk_pair)) {
+ return;
+ }
+ } break;
+ case AlterObjStoreInfoRequest::ADD_BUILT_IN_VAULT: {
+ // It should at least has one hdfs info or obj info inside storage
vault
+ if ((!request->has_vault())) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ msg = "hdfs info is not found " + proto_to_json(*request);
Review Comment:
typo hdfs?
##########
cloud/src/meta-service/meta_service_resource.cpp:
##########
@@ -509,56 +510,193 @@ static void set_default_vault_log_helper(const
InstanceInfoPB& instance,
LOG(INFO) << vault_msg;
}
-void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController*
controller,
- const AlterObjStoreInfoRequest*
request,
- AlterObjStoreInfoResponse* response,
- ::google::protobuf::Closure* done) {
+static int alter_s3_storage_vault(InstanceInfoPB& instance,
std::unique_ptr<Transaction> txn,
+ const StorageVaultPB& vault,
MetaServiceCode& code,
+ std::string& msg) {
+ if (!vault.has_obj_info()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ std::stringstream ss;
+ ss << "Only s3 vault can be altered";
+ msg = ss.str();
+ return -1;
+ }
+ const auto& obj_info = vault.obj_info();
+ if (obj_info.has_bucket() || obj_info.has_endpoint() ||
obj_info.has_prefix() ||
+ obj_info.has_provider()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ std::stringstream ss;
+ ss << "Only ak, sk can be altered";
+ msg = ss.str();
+ return -1;
+ }
+ const auto& name = vault.name();
+ auto name_itr = std::find_if(instance.storage_vault_names().begin(),
+ instance.storage_vault_names().end(),
+ [&](const auto& vault_name) { return name ==
vault_name; });
+ if (name_itr == instance.storage_vault_names().end()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ std::stringstream ss;
+ ss << "invalid storage vault name, name =" << name;
Review Comment:
change to "invalid storage vault name, not found, name="
##########
cloud/src/meta-service/meta_service_http.cpp:
##########
@@ -208,7 +208,26 @@ static HttpResponse
process_get_obj_store_info(MetaServiceImpl* service, brpc::C
static HttpResponse process_alter_obj_store_info(MetaServiceImpl* service,
brpc::Controller* ctrl) {
static std::unordered_map<std::string_view,
AlterObjStoreInfoRequest::Operation> operations {
{"add_obj_info", AlterObjStoreInfoRequest::ADD_OBJ_INFO},
- {"legacy_update_ak_sk",
AlterObjStoreInfoRequest::LEGACY_UPDATE_AK_SK},
+ {"legacy_update_ak_sk",
AlterObjStoreInfoRequest::LEGACY_UPDATE_AK_SK}};
+
+ auto& path = ctrl->http_request().unresolved_path();
+ auto it = operations.find(remove_version_prefix(path));
+ if (it == operations.end()) {
+ std::string msg = "not supportted alter obj store info operation: " +
path;
+ return http_json_reply(MetaServiceCode::INVALID_ARGUMENT, msg);
+ }
+
+ AlterObjStoreInfoRequest req;
+ PARSE_MESSAGE_OR_RETURN(ctrl, req);
+ req.set_op(it->second);
+
+ AlterObjStoreInfoResponse resp;
+ service->alter_obj_store_info(ctrl, &req, &resp, nullptr);
+ return http_json_reply(resp.status());
+}
+
+static HttpResponse process_alter_storage_vault(MetaServiceImpl* service,
brpc::Controller* ctrl) {
Review Comment:
the log
"not supportted alter obj store info operation"
is not correct it should be "storage vault"
##########
cloud/src/meta-service/meta_service_resource.cpp:
##########
@@ -509,56 +510,193 @@ static void set_default_vault_log_helper(const
InstanceInfoPB& instance,
LOG(INFO) << vault_msg;
}
-void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController*
controller,
- const AlterObjStoreInfoRequest*
request,
- AlterObjStoreInfoResponse* response,
- ::google::protobuf::Closure* done) {
+static int alter_s3_storage_vault(InstanceInfoPB& instance,
std::unique_ptr<Transaction> txn,
+ const StorageVaultPB& vault,
MetaServiceCode& code,
+ std::string& msg) {
+ if (!vault.has_obj_info()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ std::stringstream ss;
+ ss << "Only s3 vault can be altered";
+ msg = ss.str();
+ return -1;
+ }
+ const auto& obj_info = vault.obj_info();
+ if (obj_info.has_bucket() || obj_info.has_endpoint() ||
obj_info.has_prefix() ||
+ obj_info.has_provider()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ std::stringstream ss;
+ ss << "Only ak, sk can be altered";
+ msg = ss.str();
+ return -1;
+ }
+ const auto& name = vault.name();
+ auto name_itr = std::find_if(instance.storage_vault_names().begin(),
+ instance.storage_vault_names().end(),
+ [&](const auto& vault_name) { return name ==
vault_name; });
+ if (name_itr == instance.storage_vault_names().end()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ std::stringstream ss;
+ ss << "invalid storage vault name, name =" << name;
+ msg = ss.str();
+ return -1;
+ }
+ auto pos = name_itr - instance.storage_vault_names().begin();
+ auto id_itr = instance.resource_ids().begin() + pos;
+ auto vault_key = storage_vault_key({instance.instance_id(), *id_itr});
+ std::string val;
+
+ auto err = txn->get(vault_key, &val);
+ LOG(INFO) << "get instance_key=" << hex(vault_key);
+
+ if (err != TxnErrorCode::TXN_OK) {
Review Comment:
print the obj info before and after change.
mask the sk by replacing the middle chars.
e.g
```
sk = abcdefgh
print abc***fgh
```
--
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
