github-actions[bot] commented on code in PR #65330:
URL: https://github.com/apache/doris/pull/65330#discussion_r3570369115
##########
cloud/src/recycler/recycler.cpp:
##########
@@ -897,41 +873,79 @@ int InstanceRecycler::recycle_deleted_instance() {
return 0;
Review Comment:
This early return now skips more than the destructive final metadata/data
deletion. Before this change, deleted instances still ran tmp-rowset,
versioned-rowset, and operation-log cleanup before stopping on active cluster
snapshots. At least `recycle_operation_logs()` is already snapshot-aware: it
loads active snapshots and recycles only logs not referenced by them, and the
existing tests cover a mix of protected and recyclable logs. With the new
ordering, a single active snapshot prevents those safe cleanup passes for the
whole snapshot lifetime. Please keep the snapshot gate around the final
destructive phase, or otherwise allow the snapshot-aware/uncommitted cleanup
steps to continue while snapshots exist, with a deleted-instance test covering
that case.
##########
cloud/src/recycler/recycler.cpp:
##########
@@ -756,6 +756,11 @@ int InstanceRecycler::init_storage_vault_accessors() {
}
int InstanceRecycler::init() {
+ if (instance_info_.status() == InstanceInfoPB::DELETED &&
+ instance_info_.recycled_status() ==
InstanceDeletedStatus::INSTANCE_META_AND_DATA_DELETED) {
Review Comment:
Persisting `INSTANCE_DATA_DELETED` still cannot resume reliably here because
every production recycle pass constructs a fresh `InstanceRecycler` and must
pass `init()` before `do_recycle()`. This shortcut only bypasses accessors for
the terminal `INSTANCE_META_AND_DATA_DELETED` state, while
`recycle_deleted_instance()` treats `INSTANCE_DATA_DELETED` as the
metadata-only phase and no longer uses `accessor_map_`. That is exactly the
state the new HTTP recovery path tells operators to set when object data is
already gone or inaccessible, and it is also the state left behind if the
metadata-delete transaction succeeds but the later terminal-status update fails
after removing the `storage_vault_key` range. A fresh worker with
`resource_ids()` can then fail in `init_storage_vault_accessors()` before it
reaches the metadata cleanup. Please let deleted instances at least at
`INSTANCE_DATA_DELETED` skip object/vault accessor initialization, or split the
metadata-only resume path, and add a
fresh-worker test for that persisted intermediate state.
##########
cloud/src/common/http_helper.cpp:
##########
@@ -561,6 +567,33 @@ HttpResponse process_alter_instance(MetaServiceImpl*
service, brpc::Controller*
return http_json_reply(resp.status());
}
+HttpResponse process_set_instance_recycled_status(RecyclerServiceImpl* service,
+ brpc::Controller* ctrl) {
+ auto& uri = ctrl->http_request().uri();
+ std::string instance_id(http_query(uri, "instance_id"));
+ std::string recycled_status_str(http_query(uri, "recycled_status"));
+ if (instance_id.empty()) {
+ return http_json_reply(MetaServiceCode::INVALID_ARGUMENT, "instance_id
is empty");
+ }
+ if (recycled_status_str.empty()) {
+ return http_json_reply(MetaServiceCode::INVALID_ARGUMENT,
"recycled_status is empty");
+ }
+
+ InstanceDeletedStatus recycled_status;
+ if (!InstanceDeletedStatus_Parse(recycled_status_str, &recycled_status) ||
+ (recycled_status != InstanceDeletedStatus::INSTANCE_RECYCLE_PENDING &&
+ recycled_status != InstanceDeletedStatus::INSTANCE_DATA_DELETED &&
+ recycled_status !=
InstanceDeletedStatus::INSTANCE_META_AND_DATA_DELETED)) {
Review Comment:
Allowing the manual endpoint to accept `INSTANCE_META_AND_DATA_DELETED`
makes it possible to persist the terminal state without proving either the
object data or metadata ranges were cleaned. The service only checks that the
instance is `DELETED` before writing the requested status, and once this
terminal value is stored, both `init()` and `recycle_deleted_instance()` return
immediately on later scanner passes. A premature manual call therefore leaves
any remaining data/metadata orphaned indefinitely. Please restrict this
recovery endpoint to the intended non-terminal transition, or add monotonic
transition and cleanup-precondition checks before accepting the terminal state.
--
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]
