From ed77b31ba0718b3c2f7acf87c01f9ffaa576daec Mon Sep 17 00:00:00 2001
From: morningman <chenmingyu@baidu.com>
Date: Thu, 17 Dec 2020 10:45:26 +0800
Subject: [PATCH] [Bug] Fix tablet shared ptr circular reference causing the
 tablet not to be cleared

Regardless of whether the tablet is submitted for compaction or not,
we need to call 'reset_compaction' to clean up the base_compaction or cumulative_compaction objects
in the tablet, because these two objects store the tablet's own shared_ptr.
If it is not cleaned up, the reference count of the tablet will always be greater than 1,
thus cannot be collected by the garbage collector. (TabletManager::start_trash_sweep)

This bug is introduced from #4891
---
 be/src/olap/olap_server.cpp | 18 ++++++++++++++----
 be/src/olap/tablet.cpp      |  8 ++++++++
 be/src/olap/tablet.h        |  1 +
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/be/src/olap/olap_server.cpp b/be/src/olap/olap_server.cpp
index b3322aac931e30..8f87c25af70621 100644
--- a/be/src/olap/olap_server.cpp
+++ b/be/src/olap/olap_server.cpp
@@ -344,12 +344,15 @@ void StorageEngine::_compaction_tasks_producer_callback() {
                                                        [=] { return _wakeup_producer_flag == 1; });
                 continue;
             }
+
+            /// Regardless of whether the tablet is submitted for compaction or not,
+            /// we need to call 'reset_compaction' to clean up the base_compaction or cumulative_compaction objects
+            /// in the tablet, because these two objects store the tablet's own shared_ptr.
+            /// If it is not cleaned up, the reference count of the tablet will always be greater than 1,
+            /// thus cannot be collected by the garbage collector. (TabletManager::start_trash_sweep)
             for (const auto& tablet : tablets_compaction) {
                 int64_t permits = tablet->prepare_compaction_and_calculate_permits(compaction_type, tablet);
-                if (permits == 0) {
-                    continue;
-                }
-                if (_permit_limiter.request(permits)) {
+                if (permits > 0 && _permit_limiter.request(permits)) {
                     {
                         // Push to _tablet_submitted_compaction before submitting task
                         std::unique_lock<std::mutex> lock(_tablet_submitted_compaction_mutex);
@@ -371,10 +374,17 @@ void StorageEngine::_compaction_tasks_producer_callback() {
                           _wakeup_producer_flag = 1;
                           _compaction_producer_sleep_cv.notify_one();
                       }
+                      // reset compaction
+                      tablet->reset_compaction(compaction_type); 
                     });
                     if (!st.ok()) {
                         _permit_limiter.release(permits);
+                        // reset compaction
+                        tablet->reset_compaction(compaction_type); 
                     }
+                } else {
+                    // reset compaction
+                    tablet->reset_compaction(compaction_type);
                 }
             }
         } else {
diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp
index a0428653c27f5b..1b5320b7f9cd1f 100644
--- a/be/src/olap/tablet.cpp
+++ b/be/src/olap/tablet.cpp
@@ -1426,4 +1426,12 @@ void Tablet::execute_compaction(CompactionType compaction_type) {
     }
 }
 
+void Tablet::reset_compaction(CompactionType compaction_type) {
+    if (compaction_type == CompactionType::CUMULATIVE_COMPACTION) {
+        _cumulative_compaction.reset();
+    } else {
+        _base_compaction.reset();
+    }
+}
+
 }  // namespace doris
diff --git a/be/src/olap/tablet.h b/be/src/olap/tablet.h
index 83d37dd8aaf237..c34d90967e973f 100644
--- a/be/src/olap/tablet.h
+++ b/be/src/olap/tablet.h
@@ -246,6 +246,7 @@ class Tablet : public BaseTablet {
 
     int64_t prepare_compaction_and_calculate_permits(CompactionType compaction_type, TabletSharedPtr tablet);
     void execute_compaction(CompactionType compaction_type);
+    void reset_compaction(CompactionType compaction_type);
 
     void set_clone_occurred(bool clone_occurred) { _is_clone_occurred = clone_occurred; }
     bool get_clone_occurred() { return _is_clone_occurred; }