github-actions[bot] commented on code in PR #65330:
URL: https://github.com/apache/doris/pull/65330#discussion_r3642598354
##########
cloud/src/meta-service/meta_service_snapshot.cpp:
##########
@@ -210,4 +210,36 @@ void
MetaServiceImpl::compact_snapshot(::google::protobuf::RpcController* contro
std::tie(code, msg) = snapshot_manager_->compact_snapshot(instance_id);
}
+std::pair<MetaServiceCode, std::string>
MetaServiceImpl::check_instance_recycle_completed(
+ const std::string& instance_id, bool& finished) {
+ std::unique_ptr<Transaction> txn;
+ TxnErrorCode err = txn_kv_->create_txn(&txn);
+ if (err != TxnErrorCode::TXN_OK) {
+ std::string msg = fmt::format("failed to create txn, err={}", err);
+ LOG(WARNING) << msg << " instance_id=" << instance_id;
+ return {MetaServiceCode::KV_TXN_CREATE_ERR, std::move(msg)};
+ }
+
+ std::string key = instance_key({instance_id});
+ std::string value;
+ err = txn->get(key, &value);
+ if (err != TxnErrorCode::TXN_OK) {
+ std::string msg =
+ fmt::format("failed to get instance, instance_id={}, err={}",
instance_id, err);
+ LOG(WARNING) << msg;
+ return {MetaServiceCode::KV_TXN_GET_ERR, std::move(msg)};
+ }
+
+ InstanceInfoPB instance;
+ if (!instance.ParseFromString(value)) {
+ std::string msg = fmt::format("malformed instance info, key={}",
hex(key));
+ LOG(WARNING) << msg;
+ return {MetaServiceCode::PROTOBUF_PARSE_ERR, std::move(msg)};
+ }
+
+ finished = instance.recycled_state() ==
Review Comment:
[P1] Report completion for the whole dropped chain. Dropping rollback tail B
marks B and every predecessor deleted in one transaction, but cleanup is
deliberately tail-to-head: B can legitimately reach `CLEANUP_COMPLETED` while A
is still `DATA_CLEANUP_COMPLETED` (or stuck). This single-PB check then returns
`finished=true` even though the requested DROP still has predecessor metadata
to remove. Please check the entire original-to-tail chain in a consistent read,
or publish a chain-level completion marker only after the last predecessor
finishes, and add a paused-predecessor test.
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -121,11 +121,19 @@ std::string get_instance_id(const
std::shared_ptr<ResourceManager>& rc_mgr,
}
// check instance_id valid by get fdb
- if (config::enable_check_instance_id &&
!rc_mgr->is_instance_id_registered(id)) {
- LOG(WARNING) << "use degraded format cloud_unique_id, but check
instance failed, "
- "cloud_unique_id="
- << cloud_unique_id;
- return "";
+ if (config::enable_check_instance_id) {
Review Comment:
[P1] Validate the deleted status on cache hits too. This FDB check runs only
after `get_node()` misses. In a multi-MS deployment, another node can still
have the `cloud_unique_id` mapping while cluster/DROP refresh notifications and
the global watcher are pending; `begin_txn()` on that node takes the cache-hit
branch and creates metadata under the already-`DELETED` instance. Please
validate persisted status for both branches (or synchronously fence deletion in
the cache), with a two-manager paused-refresh test.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
