w41ter commented on code in PR #36786:
URL: https://github.com/apache/doris/pull/36786#discussion_r1666158135
##########
cloud/src/meta-service/meta_service_partition.cpp:
##########
@@ -614,4 +621,93 @@ void
MetaServiceImpl::drop_partition(::google::protobuf::RpcController* controll
}
}
+void check_create_table(std::string instance_id, std::shared_ptr<TxnKv> txn_kv,
+ const CheckKVRequest* request, CheckKVResponse*
response,
+ MetaServiceCode* code, std::string* msg,
+ check_create_table_type get_check_info) {
+ std::unique_ptr<Transaction> txn;
+ TxnErrorCode err = txn_kv->create_txn(&txn);
+ if (err != TxnErrorCode::TXN_OK) {
+ *code = cast_as<ErrCategory::READ>(err);
+ *msg = "failed to create txn";
+ return;
+ }
+ auto& [keys, hint, key_func] = get_check_info(request);
+ if (keys.empty()) {
+ *code = MetaServiceCode::INVALID_ARGUMENT;
+ *msg = "empty keys";
+ return;
+ }
+
+ for (auto id : keys) {
+ auto key = key_func(instance_id, id);
+ err = check_recycle_key_exist(txn.get(), key);
+ if (err == TxnErrorCode::TXN_KEY_NOT_FOUND) {
+ continue;
+ } else if (err == TxnErrorCode::TXN_OK) {
+ // find not match, prepare commit
+ *code = MetaServiceCode::ALREADY_EXISTED;
+ *msg = "prepare and commit rpc not match, recycle key remained";
+ return;
+ } else {
+ // err != TXN_OK, fdb read err
+ *code = cast_as<ErrCategory::READ>(err);
+ *msg = "ms read key error";
Review Comment:
```suggestion
*msg = fmt::format("ms read key error: {}", err);
```
##########
cloud/src/meta-service/meta_service_partition.cpp:
##########
@@ -614,4 +621,93 @@ void
MetaServiceImpl::drop_partition(::google::protobuf::RpcController* controll
}
}
+void check_create_table(std::string instance_id, std::shared_ptr<TxnKv> txn_kv,
+ const CheckKVRequest* request, CheckKVResponse*
response,
+ MetaServiceCode* code, std::string* msg,
+ check_create_table_type get_check_info) {
+ std::unique_ptr<Transaction> txn;
+ TxnErrorCode err = txn_kv->create_txn(&txn);
+ if (err != TxnErrorCode::TXN_OK) {
+ *code = cast_as<ErrCategory::READ>(err);
+ *msg = "failed to create txn";
+ return;
+ }
+ auto& [keys, hint, key_func] = get_check_info(request);
+ if (keys.empty()) {
+ *code = MetaServiceCode::INVALID_ARGUMENT;
+ *msg = "empty keys";
+ return;
+ }
+
+ for (auto id : keys) {
+ auto key = key_func(instance_id, id);
+ err = check_recycle_key_exist(txn.get(), key);
+ if (err == TxnErrorCode::TXN_KEY_NOT_FOUND) {
+ continue;
+ } else if (err == TxnErrorCode::TXN_OK) {
+ // find not match, prepare commit
+ *code = MetaServiceCode::ALREADY_EXISTED;
+ *msg = "prepare and commit rpc not match, recycle key remained";
+ return;
+ } else {
+ // err != TXN_OK, fdb read err
+ *code = cast_as<ErrCategory::READ>(err);
+ *msg = "ms read key error";
+ return;
+ }
+ }
+ LOG_INFO("check {} success request={}", hint, request->ShortDebugString());
+ return;
Review Comment:
```suggestion
```
##########
cloud/src/meta-service/meta_service_partition.cpp:
##########
@@ -614,4 +621,93 @@ void
MetaServiceImpl::drop_partition(::google::protobuf::RpcController* controll
}
}
+void check_create_table(std::string instance_id, std::shared_ptr<TxnKv> txn_kv,
+ const CheckKVRequest* request, CheckKVResponse*
response,
+ MetaServiceCode* code, std::string* msg,
+ check_create_table_type get_check_info) {
+ std::unique_ptr<Transaction> txn;
+ TxnErrorCode err = txn_kv->create_txn(&txn);
+ if (err != TxnErrorCode::TXN_OK) {
+ *code = cast_as<ErrCategory::READ>(err);
+ *msg = "failed to create txn";
+ return;
+ }
+ auto& [keys, hint, key_func] = get_check_info(request);
+ if (keys.empty()) {
+ *code = MetaServiceCode::INVALID_ARGUMENT;
+ *msg = "empty keys";
+ return;
+ }
+
+ for (auto id : keys) {
+ auto key = key_func(instance_id, id);
+ err = check_recycle_key_exist(txn.get(), key);
+ if (err == TxnErrorCode::TXN_KEY_NOT_FOUND) {
+ continue;
+ } else if (err == TxnErrorCode::TXN_OK) {
+ // find not match, prepare commit
+ *code = MetaServiceCode::ALREADY_EXISTED;
+ *msg = "prepare and commit rpc not match, recycle key remained";
+ return;
+ } else {
+ // err != TXN_OK, fdb read err
+ *code = cast_as<ErrCategory::READ>(err);
+ *msg = "ms read key error";
+ return;
+ }
+ }
+ LOG_INFO("check {} success request={}", hint, request->ShortDebugString());
Review Comment:
If there are many requests for a check, the request will be huge and
unsuitable for printing directly to the log.
##########
cloud/src/meta-service/meta_service_partition.cpp:
##########
@@ -614,4 +621,93 @@ void
MetaServiceImpl::drop_partition(::google::protobuf::RpcController* controll
}
}
+void check_create_table(std::string instance_id, std::shared_ptr<TxnKv> txn_kv,
+ const CheckKVRequest* request, CheckKVResponse*
response,
+ MetaServiceCode* code, std::string* msg,
+ check_create_table_type get_check_info) {
+ std::unique_ptr<Transaction> txn;
+ TxnErrorCode err = txn_kv->create_txn(&txn);
+ if (err != TxnErrorCode::TXN_OK) {
+ *code = cast_as<ErrCategory::READ>(err);
+ *msg = "failed to create txn";
+ return;
+ }
+ auto& [keys, hint, key_func] = get_check_info(request);
+ if (keys.empty()) {
+ *code = MetaServiceCode::INVALID_ARGUMENT;
+ *msg = "empty keys";
+ return;
+ }
+
+ for (auto id : keys) {
+ auto key = key_func(instance_id, id);
+ err = check_recycle_key_exist(txn.get(), key);
+ if (err == TxnErrorCode::TXN_KEY_NOT_FOUND) {
+ continue;
+ } else if (err == TxnErrorCode::TXN_OK) {
+ // find not match, prepare commit
+ *code = MetaServiceCode::ALREADY_EXISTED;
+ *msg = "prepare and commit rpc not match, recycle key remained";
+ return;
+ } else {
+ // err != TXN_OK, fdb read err
+ *code = cast_as<ErrCategory::READ>(err);
+ *msg = "ms read key error";
+ return;
+ }
+ }
+ LOG_INFO("check {} success request={}", hint, request->ShortDebugString());
+ return;
+}
+
+void MetaServiceImpl::check_kv(::google::protobuf::RpcController* controller,
+ const CheckKVRequest* request, CheckKVResponse*
response,
+ ::google::protobuf::Closure* done) {
+ RPC_PREPROCESS(check_kv);
+ instance_id = get_instance_id(resource_mgr_, request->cloud_unique_id());
+ if (instance_id.empty()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ msg = "empty instance_id";
+ return;
+ }
+ if (!request->has_op()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ msg = "op not given";
+ return;
+ }
+ if (!request->has_check_keys()) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
+ msg = "empty check keys";
+ return;
+ }
+ RPC_RATE_LIMIT(check_kv);
+ switch (request->op()) {
+ case CheckKVRequest::CREATE_INDEX_AFTER_FE_COMMIT: {
+ check_create_table(instance_id, txn_kv_, request, response, &code,
&msg,
+ [](const CheckKVRequest* request) {
+ return std::make_tuple(
+ request->check_keys().index_ids(),
"index",
+ [](std::string instance_id, int64_t id)
{
+ return
recycle_index_key({std::move(instance_id), id});
+ });
+ });
+ break;
+ }
+ case CheckKVRequest::CREATE_PARTITION_AFTER_FE_COMMIT: {
+ check_create_table(
+ instance_id, txn_kv_, request, response, &code, &msg,
+ [](const CheckKVRequest* request) {
+ return std::make_tuple(
+ request->check_keys().partition_ids(), "partition",
+ [](std::string instance_id, int64_t id) {
+ return
recycle_partition_key({std::move(instance_id), id});
+ });
+ });
+ break;
+ }
+ default:
+ DCHECK(false);
Review Comment:
It is more appropriate to return an invalid argument directly here.
--
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
