liaoxin01 commented on code in PR #65330:
URL: https://github.com/apache/doris/pull/65330#discussion_r3571975841


##########
cloud/src/recycler/recycler.cpp:
##########
@@ -897,41 +873,79 @@ int InstanceRecycler::recycle_deleted_instance() {
         return 0;
     }
 
-    bool snapshot_enabled = instance_info().has_snapshot_switch_status() &&
-                            instance_info().snapshot_switch_status() !=
-                                    
SnapshotSwitchStatus::SNAPSHOT_SWITCH_DISABLED;
-    if (snapshot_enabled) {
-        bool has_unrecycled_rowsets = false;
-        if (recycle_ref_rowsets(&has_unrecycled_rowsets) != 0) {
-            LOG_WARNING("failed to recycle ref rowsets").tag("instance_id", 
instance_id_);
+    if (instance_info_.recycled_status() < 
InstanceDeletedStatus::INSTANCE_DATA_DELETED) {
+        auto recycle_tmp_rowsets_with_mark_delete_enabled = [&]() -> int {
+            int res = recycle_tmp_rowsets();
+            if (res == 0 && config::enable_mark_delete_rowset_before_recycle) {
+                // If mark_delete_rowset_before_recycle is enabled, we will 
mark delete rowsets before recycling them,
+                // so we need to recycle tmp rowsets again to make sure all 
rowsets in recycle space are marked for
+                // deletion, otherwise we may meet some corner cases that some 
rowsets are not marked for deletion
+                // and cannot be recycled.
+                res = recycle_tmp_rowsets();
+            }
+            return res;
+        };
+        if (recycle_tmp_rowsets_with_mark_delete_enabled() != 0) {
+            LOG_WARNING("failed to recycle tmp rowsets").tag("instance_id", 
instance_id_);
             ret = -1;
             return -1;
-        } else if (has_unrecycled_rowsets) {
-            LOG_INFO("instance has referenced rowsets, skip recycling")
-                    .tag("instance_id", instance_id_);
-            return ret;
         }
-    } else { // delete all remote data if snapshot is disabled
-        for (auto& [_, accessor] : accessor_map_) {
-            if (stopped()) {
+
+        if (recycle_versioned_rowsets() != 0) {
+            LOG_WARNING("failed to recycle versioned 
rowsets").tag("instance_id", instance_id_);
+            ret = -1;
+            return -1;
+        }
+
+        if (recycle_operation_logs() != 0) {
+            LOG_WARNING("failed to recycle operation logs").tag("instance_id", 
instance_id_);
+            ret = -1;
+            return -1;
+        }
+
+        bool snapshot_enabled = instance_info().has_snapshot_switch_status() &&
+                                instance_info().snapshot_switch_status() !=
+                                        
SnapshotSwitchStatus::SNAPSHOT_SWITCH_DISABLED;
+        if (snapshot_enabled) {
+            bool has_unrecycled_rowsets = false;
+            if (recycle_ref_rowsets(&has_unrecycled_rowsets) != 0) {
+                LOG_WARNING("failed to recycle ref 
rowsets").tag("instance_id", instance_id_);
+                ret = -1;
+                return -1;
+            } else if (has_unrecycled_rowsets) {
+                LOG_INFO("instance has referenced rowsets, skip recycling")
+                        .tag("instance_id", instance_id_);
                 return ret;
             }
+        } else { // delete all remote data if snapshot is disabled
+            for (auto& [_, accessor] : accessor_map_) {
+                if (stopped()) {
+                    return ret;
+                }
 
-            LOG(INFO) << "begin to delete all objects in " << accessor->uri();
-            int del_ret = accessor->delete_all();
-            if (del_ret == 0) {
-                LOG(INFO) << "successfully delete all objects in " << 
accessor->uri();
-            } else if (del_ret != 1) { // no need to log, because S3Accessor 
has logged this error
-                // If `del_ret == 1`, it can be considered that the object 
data has been recycled by cloud platform,
-                // so the recycling has been successful.
-                ret = -1;
+                LOG(INFO) << "begin to delete all objects in " << 
accessor->uri();
+                int del_ret = accessor->delete_all();
+                if (del_ret == 0) {
+                    LOG(INFO) << "successfully delete all objects in " << 
accessor->uri();
+                } else if (del_ret !=

Review Comment:
   no need to do this change.



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

Reply via email to