[fix](path-gc) Fix pending rowset guard check failure when ordered da…

…ta compaction failed (#33026)
apache · Mar 29, 2024 · cb6287e · cb6287e
1 parent 232e818
commit cb6287e
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 15 deletions.
diff --git a/be/src/olap/compaction.cpp b/be/src/olap/compaction.cpp
@@ -319,6 +319,12 @@ bool CompactionMixin::handle_ordered_data_compaction() {
         _tablet->enable_unique_key_merge_on_write()) {
         return false;
     }
+
+    if (_tablet->tablet_meta()->tablet_schema()->skip_write_index_on_load()) {
+        // Expected to create index through normal compaction
+        return false;
+    }
+
     // check delete version: if compaction type is base compaction and
     // has a delete version, use original compaction
     if (compaction_type() == ReaderType::READER_BASE_COMPACTION) {
@@ -346,6 +352,11 @@ bool CompactionMixin::handle_ordered_data_compaction() {
     // most rowset of current compaction is nonoverlapping
     // just handle nonoverlappint rowsets
     auto st = do_compact_ordered_rowsets();
+    if (!st.ok()) {
+        LOG(WARNING) << "failed to compact ordered rowsets: " << st;
+        _pending_rs_guard.drop();
+    }
+
     return st.ok();
 }
 

diff --git a/be/src/olap/compaction.h b/be/src/olap/compaction.h
@@ -147,6 +147,7 @@ class CompactionMixin : public Compaction {
 
     void construct_skip_inverted_index(RowsetWriterContext& ctx);
 
+    // Return true if do ordered data compaction successfully
     bool handle_ordered_data_compaction();
 
     Status do_compact_ordered_rowsets();

diff --git a/be/src/olap/rowset/pending_rowset_helper.cpp b/be/src/olap/rowset/pending_rowset_helper.cpp
@@ -17,6 +17,8 @@
 
 #include "olap/rowset/pending_rowset_helper.h"
 
+#include "olap/olap_common.h"
+
 namespace doris {
 
 PendingRowsetGuard::~PendingRowsetGuard() {
@@ -28,6 +30,35 @@ PendingRowsetGuard::~PendingRowsetGuard() {
 PendingRowsetGuard::PendingRowsetGuard(const RowsetId& rowset_id, PendingRowsetSet* set)
         : _rowset_id(rowset_id), _pending_rowset_set(set) {}
 
+PendingRowsetGuard::PendingRowsetGuard(PendingRowsetGuard&& other) noexcept {
+    CHECK(!_pending_rowset_set ||
+          (_rowset_id == other._rowset_id && _pending_rowset_set == other._pending_rowset_set))
+            << _rowset_id << ' ' << other._rowset_id << ' ' << _pending_rowset_set << ' '
+            << other._pending_rowset_set;
+    _rowset_id = other._rowset_id;
+    _pending_rowset_set = other._pending_rowset_set;
+    other._pending_rowset_set = nullptr;
+}
+
+PendingRowsetGuard& PendingRowsetGuard::operator=(PendingRowsetGuard&& other) noexcept {
+    CHECK(!_pending_rowset_set ||
+          (_rowset_id == other._rowset_id && _pending_rowset_set == other._pending_rowset_set))
+            << _rowset_id << ' ' << other._rowset_id << ' ' << _pending_rowset_set << ' '
+            << other._pending_rowset_set;
+    _rowset_id = other._rowset_id;
+    _pending_rowset_set = other._pending_rowset_set;
+    other._pending_rowset_set = nullptr;
+    return *this;
+}
+
+void PendingRowsetGuard::drop() {
+    if (_pending_rowset_set) {
+        _pending_rowset_set->remove(_rowset_id);
+    }
+    _pending_rowset_set = nullptr;
+    _rowset_id = RowsetId {};
+}
+
 bool PendingRowsetSet::contains(const RowsetId& rowset_id) {
     std::lock_guard lock(_mtx);
     return _set.contains(rowset_id);

diff --git a/be/src/olap/rowset/pending_rowset_helper.h b/be/src/olap/rowset/pending_rowset_helper.h
@@ -35,21 +35,13 @@ class [[nodiscard]] PendingRowsetGuard {
     PendingRowsetGuard(const PendingRowsetGuard&) = delete;
     PendingRowsetGuard& operator=(const PendingRowsetGuard&) = delete;
 
-    PendingRowsetGuard(PendingRowsetGuard&& other) noexcept {
-        CHECK(!_pending_rowset_set ||
-              (_rowset_id == other._rowset_id && _pending_rowset_set == other._pending_rowset_set));
-        _rowset_id = other._rowset_id;
-        _pending_rowset_set = other._pending_rowset_set;
-        other._pending_rowset_set = nullptr;
-    }
-    PendingRowsetGuard& operator=(PendingRowsetGuard&& other) noexcept {
-        CHECK(!_pending_rowset_set ||
-              (_rowset_id == other._rowset_id && _pending_rowset_set == other._pending_rowset_set));
-        _rowset_id = other._rowset_id;
-        _pending_rowset_set = other._pending_rowset_set;
-        other._pending_rowset_set = nullptr;
-        return *this;
-    }
+    PendingRowsetGuard(PendingRowsetGuard&& other) noexcept;
+    PendingRowsetGuard& operator=(PendingRowsetGuard&& other) noexcept;
+
+    // Remove guarded rowset id from `PendingRowsetSet` if it's initialized and reset the guard to
+    // uninitialized state. This be used to manually release the pending rowset, ensure that the
+    // guarded rowset is indeed no longer in use.
+    void drop();
 
 private:
     friend class PendingRowsetSet;