From f72befe05e15fb5085557b376cd1f15f90d2cac6 Mon Sep 17 00:00:00 2001 From: plat1ko Date: Fri, 29 Mar 2024 17:47:51 +0800 Subject: [PATCH] [fix](path-gc) Fix pending rowset guard check failure when ordered data compaction failed (#33029) --- be/src/olap/compaction.cpp | 12 ++++++-- be/src/olap/compaction.h | 1 + be/src/olap/rowset/pending_rowset_helper.cpp | 29 ++++++++++++++++++++ be/src/olap/rowset/pending_rowset_helper.h | 22 +++++---------- 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/be/src/olap/compaction.cpp b/be/src/olap/compaction.cpp index 9bedbce11a..ae6bded1b7 100644 --- a/be/src/olap/compaction.cpp +++ b/be/src/olap/compaction.cpp @@ -268,6 +268,12 @@ bool Compaction::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) { @@ -296,9 +302,11 @@ bool Compaction::handle_ordered_data_compaction() { // just handle nonoverlappint rowsets auto st = do_compact_ordered_rowsets(); if (!st.ok()) { - return false; + LOG(WARNING) << "failed to compact ordered rowsets: " << st; + _pending_rs_guard.drop(); } - return true; + + return st.ok(); } Status Compaction::do_compaction_impl(int64_t permits) { diff --git a/be/src/olap/compaction.h b/be/src/olap/compaction.h index a8e3b1a5c2..5b1580f209 100644 --- a/be/src/olap/compaction.h +++ b/be/src/olap/compaction.h @@ -91,6 +91,7 @@ protected: bool should_vertical_compaction(); int64_t get_avg_segment_rows(); + // Return true if do ordered data compaction successfully bool handle_ordered_data_compaction(); Status do_compact_ordered_rowsets(); bool is_rowset_tidy(std::string& pre_max_key, const RowsetSharedPtr& rhs); diff --git a/be/src/olap/rowset/pending_rowset_helper.cpp b/be/src/olap/rowset/pending_rowset_helper.cpp index 3e976344f6..0d858675e8 100644 --- a/be/src/olap/rowset/pending_rowset_helper.cpp +++ b/be/src/olap/rowset/pending_rowset_helper.cpp @@ -28,6 +28,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 index 53d1f4f16c..360424636b 100644 --- a/be/src/olap/rowset/pending_rowset_helper.h +++ b/be/src/olap/rowset/pending_rowset_helper.h @@ -35,21 +35,13 @@ public: 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;