From 657ea886a65bde73c64e387a743f91a06f5f2b95 Mon Sep 17 00:00:00 2001 From: bobhan1 Date: Thu, 27 Jul 2023 12:08:30 +0800 Subject: [PATCH 01/10] update --- be/src/olap/calc_delete_bitmap_executor.cpp | 6 +++ be/src/olap/tablet.cpp | 48 +++++++++++++++++++++ be/src/olap/tablet.h | 6 +++ be/src/olap/tablet_meta.h | 2 + 4 files changed, 62 insertions(+) diff --git a/be/src/olap/calc_delete_bitmap_executor.cpp b/be/src/olap/calc_delete_bitmap_executor.cpp index 284c03c985d45d..e4c905b32796d7 100644 --- a/be/src/olap/calc_delete_bitmap_executor.cpp +++ b/be/src/olap/calc_delete_bitmap_executor.cpp @@ -56,6 +56,12 @@ Status CalcDeleteBitmapToken::submit(TabletSharedPtr tablet, RowsetSharedPtr cur _status = st; } } + + RowsetIdUnorderedSet rowsetids; + for (const auto& rowset : target_rowsets) { + rowsetids.emplace(rowset->rowset_id()); + } + tablet->add_sentinel_mark_to_delete_bitmap(bitmap, rowsetids); }); } diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index 9bbabb48c699c5..45b94d7d306b5b 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -3026,6 +3026,11 @@ Status Tablet::calc_delete_bitmap(RowsetSharedPtr rowset, RETURN_IF_ERROR(calc_segment_delete_bitmap(rowset, segments[i], specified_rowsets, seg_delete_bitmap, end_version, rowset_writer)); + RowsetIdUnorderedSet rowsetids; + for (const auto& rowset : specified_rowsets) { + rowsetids.emplace(rowset->rowset_id()); + } + add_sentinel_mark_to_delete_bitmap(seg_delete_bitmap, rowsetids); } } @@ -3318,6 +3323,10 @@ Status Tablet::update_delete_bitmap(const RowsetSharedPtr& rowset, << ", cur max_version: " << cur_version << ", transaction_id: " << txn_id << ", cost: " << watch.get_elapse_time_us() << "(us), total rows: " << total_rows; + // check if all the rowset has ROWSET_SENTINEL_MARK + RETURN_IF_ERROR(check_delete_bitmap_correctness(delete_bitmap, cur_version - 1)); + remove_sentinel_mark_from_delete_bitmap(delete_bitmap); + // update version without write lock, compaction and publish_txn // will update delete bitmap, handle compaction with _rowset_update_lock // and publish_txn runs sequential so no need to lock here @@ -3651,4 +3660,43 @@ Status Tablet::calc_delete_bitmap_between_segments( return Status::OK(); } +void Tablet::add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, + const RowsetIdUnorderedSet& rowsetids) { + for (const auto& rowsetid : rowsetids) { + delete_bitmap->add({rowsetid, 0, 0}, DeleteBitmap::ROWSET_SENTINEL_MARK); + } +} + +void Tablet::remove_sentinel_mark_from_delete_bitmap(DeleteBitmapPtr delete_bitmap) { + for (auto& [key, bitmap] : delete_bitmap->delete_bitmap) { + bitmap.remove(DeleteBitmap::ROWSET_SENTINEL_MARK); + } +} + +Status Tablet::check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, + int64_t max_version) const { + std::map result; + for (const auto& rowsetid : all_rs_id(max_version)) { + result.emplace(rowsetid, false); + } + for (const auto& [key, bitmap] : delete_bitmap->delete_bitmap) { + auto it = result.find(std::get<0>(key)); + if (it == result.end()) { + LOG(WARNING) << "can't find rowsetid when checking delete bitmap correctness"; + return Status::InternalError("check delete bitmap correctness failed"); + } + if (bitmap.contains(DeleteBitmap::ROWSET_SENTINEL_MARK)) { + it->second = true; + } + } + for (const auto& [rowsetid, value] : result) { + if (!value) { + LOG(WARNING) << "check delete bitmap correctness failed, can't find setinel mark in " + "rowset with RowsetId: " + << rowsetid; + return Status::InternalError("check delete bitmap correctness failed"); + } + } + return Status::OK(); +} } // namespace doris diff --git a/be/src/olap/tablet.h b/be/src/olap/tablet.h index 0b0c20719cf349..b7f9b1465137e7 100644 --- a/be/src/olap/tablet.h +++ b/be/src/olap/tablet.h @@ -550,6 +550,12 @@ class Tablet : public BaseTablet { void set_binlog_config(BinlogConfig binlog_config); + void add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, + const RowsetIdUnorderedSet& rowsetids); + void remove_sentinel_mark_from_delete_bitmap(DeleteBitmapPtr delete_bitmap); + Status check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, + int64_t max_version) const; + private: Status _init_once_action(); void _print_missed_versions(const std::vector& missed_versions) const; diff --git a/be/src/olap/tablet_meta.h b/be/src/olap/tablet_meta.h index c9ddff24171703..f8cb795130a94a 100644 --- a/be/src/olap/tablet_meta.h +++ b/be/src/olap/tablet_meta.h @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -335,6 +336,7 @@ class DeleteBitmap { using Version = uint64_t; using BitmapKey = std::tuple; std::map delete_bitmap; // Ordered map + constexpr static uint32_t ROWSET_SENTINEL_MARK = std::numeric_limits::max() - 1; /** * From 3d6da257a30f8c4a766203c7f3c5e8fdc7229717 Mon Sep 17 00:00:00 2001 From: bobhan1 Date: Thu, 27 Jul 2023 19:36:49 +0800 Subject: [PATCH 02/10] don't do correctness check when the rowset has 0 written rows --- be/src/olap/calc_delete_bitmap_executor.cpp | 6 ---- be/src/olap/delta_writer.cpp | 5 ++- be/src/olap/tablet.cpp | 39 +++++++++++++-------- be/src/olap/tablet.h | 6 ++-- be/src/olap/tablet_meta.h | 2 +- be/src/olap/txn_manager.cpp | 13 +++---- be/src/olap/txn_manager.h | 8 +++-- 7 files changed, 44 insertions(+), 35 deletions(-) diff --git a/be/src/olap/calc_delete_bitmap_executor.cpp b/be/src/olap/calc_delete_bitmap_executor.cpp index e4c905b32796d7..284c03c985d45d 100644 --- a/be/src/olap/calc_delete_bitmap_executor.cpp +++ b/be/src/olap/calc_delete_bitmap_executor.cpp @@ -56,12 +56,6 @@ Status CalcDeleteBitmapToken::submit(TabletSharedPtr tablet, RowsetSharedPtr cur _status = st; } } - - RowsetIdUnorderedSet rowsetids; - for (const auto& rowset : target_rowsets) { - rowsetids.emplace(rowset->rowset_id()); - } - tablet->add_sentinel_mark_to_delete_bitmap(bitmap, rowsetids); }); } diff --git a/be/src/olap/delta_writer.cpp b/be/src/olap/delta_writer.cpp index 947999a07630e3..00db6263f29886 100644 --- a/be/src/olap/delta_writer.cpp +++ b/be/src/olap/delta_writer.cpp @@ -321,8 +321,11 @@ Status DeltaWriter::commit_txn(const PSlaveTabletNodes& slave_tablet_nodes, const bool write_single_replica) { std::lock_guard l(_lock); SCOPED_TIMER(_close_wait_timer); + // only do correctness check if the rowset has at least one row written + bool do_correctness_check = (_rowset_writer->num_rows() != 0); Status res = _storage_engine->txn_manager()->commit_txn(_req.partition_id, _tablet, _req.txn_id, - _req.load_id, _cur_rowset, false); + _req.load_id, _cur_rowset, false, + do_correctness_check); if (!res && !res.is()) { LOG(WARNING) << "Failed to commit txn: " << _req.txn_id diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index 45b94d7d306b5b..9fa44d5638bef3 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -2977,6 +2977,15 @@ Status Tablet::calc_segment_delete_bitmap(RowsetSharedPtr rowset, } DCHECK_EQ(total, row_id) << "segment total rows: " << total << " row_id:" << row_id; + RowsetIdUnorderedSet rowsetids; + for (const auto& rowset : specified_rowsets) { + rowsetids.emplace(rowset->rowset_id()); + LOG(INFO) << "[tabletID:" << tablet_id() << "]" + << "[add_sentinel_mark_to_delete_bitmap][end_version:" << end_version << "]" + << "add:" << rowset->rowset_id(); + } + add_sentinel_mark_to_delete_bitmap(delete_bitmap, rowsetids); + if (pos > 0) { RETURN_IF_ERROR(generate_new_block_for_partial_update( rowset_schema, read_plan_ori, read_plan_update, rsid_to_rowset, &block)); @@ -3026,11 +3035,6 @@ Status Tablet::calc_delete_bitmap(RowsetSharedPtr rowset, RETURN_IF_ERROR(calc_segment_delete_bitmap(rowset, segments[i], specified_rowsets, seg_delete_bitmap, end_version, rowset_writer)); - RowsetIdUnorderedSet rowsetids; - for (const auto& rowset : specified_rowsets) { - rowsetids.emplace(rowset->rowset_id()); - } - add_sentinel_mark_to_delete_bitmap(seg_delete_bitmap, rowsetids); } } @@ -3275,7 +3279,7 @@ Status Tablet::commit_phase_update_delete_bitmap( Status Tablet::update_delete_bitmap(const RowsetSharedPtr& rowset, const RowsetIdUnorderedSet& pre_rowset_ids, DeleteBitmapPtr delete_bitmap, int64_t txn_id, - RowsetWriter* rowset_writer) { + RowsetWriter* rowset_writer, bool do_correctness_check) { SCOPED_BVAR_LATENCY(g_tablet_update_delete_bitmap_latency); RowsetIdUnorderedSet cur_rowset_ids; RowsetIdUnorderedSet rowset_ids_to_add; @@ -3323,9 +3327,11 @@ Status Tablet::update_delete_bitmap(const RowsetSharedPtr& rowset, << ", cur max_version: " << cur_version << ", transaction_id: " << txn_id << ", cost: " << watch.get_elapse_time_us() << "(us), total rows: " << total_rows; - // check if all the rowset has ROWSET_SENTINEL_MARK - RETURN_IF_ERROR(check_delete_bitmap_correctness(delete_bitmap, cur_version - 1)); - remove_sentinel_mark_from_delete_bitmap(delete_bitmap); + if (do_correctness_check) { + // check if all the rowset has ROWSET_SENTINEL_MARK + RETURN_IF_ERROR(check_delete_bitmap_correctness(delete_bitmap, cur_version - 1)); + remove_sentinel_mark_from_delete_bitmap(delete_bitmap); + } // update version without write lock, compaction and publish_txn // will update delete bitmap, handle compaction with _rowset_update_lock @@ -3673,10 +3679,10 @@ void Tablet::remove_sentinel_mark_from_delete_bitmap(DeleteBitmapPtr delete_bitm } } -Status Tablet::check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, - int64_t max_version) const { +Status Tablet::check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, int64_t max_version) { std::map result; - for (const auto& rowsetid : all_rs_id(max_version)) { + RowsetIdUnorderedSet rowsets = all_rs_id(max_version); + for (const auto& rowsetid : rowsets) { result.emplace(rowsetid, false); } for (const auto& [key, bitmap] : delete_bitmap->delete_bitmap) { @@ -3691,12 +3697,15 @@ Status Tablet::check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, } for (const auto& [rowsetid, value] : result) { if (!value) { - LOG(WARNING) << "check delete bitmap correctness failed, can't find setinel mark in " - "rowset with RowsetId: " - << rowsetid; + LOG(WARNING) << "check delete bitmap correctness failed, can't find sentinel mark in " + "rowset with RowsetId:" + << rowsetid << "version:" << get_rowset(rowsetid)->version().to_string() + << "max_version:" << max_version; return Status::InternalError("check delete bitmap correctness failed"); } } + LOG(INFO) << "[check_delete_bitmap_correctness][max_version:" << max_version << "]" + << "rowset size:" << rowsets.size(); return Status::OK(); } } // namespace doris diff --git a/be/src/olap/tablet.h b/be/src/olap/tablet.h index b7f9b1465137e7..30b5179cc30cdc 100644 --- a/be/src/olap/tablet.h +++ b/be/src/olap/tablet.h @@ -486,7 +486,8 @@ class Tablet : public BaseTablet { Status update_delete_bitmap(const RowsetSharedPtr& rowset, const RowsetIdUnorderedSet& pre_rowset_ids, DeleteBitmapPtr delete_bitmap, int64_t txn_id, - RowsetWriter* rowset_writer = nullptr); + RowsetWriter* rowset_writer = nullptr, + bool do_correctness_check = false); void calc_compaction_output_rowset_delete_bitmap( const std::vector& input_rowsets, const RowIdConversion& rowid_conversion, uint64_t start_version, uint64_t end_version, @@ -553,8 +554,7 @@ class Tablet : public BaseTablet { void add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, const RowsetIdUnorderedSet& rowsetids); void remove_sentinel_mark_from_delete_bitmap(DeleteBitmapPtr delete_bitmap); - Status check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, - int64_t max_version) const; + Status check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, int64_t max_version); private: Status _init_once_action(); diff --git a/be/src/olap/tablet_meta.h b/be/src/olap/tablet_meta.h index f8cb795130a94a..2f3d6dbb4aef14 100644 --- a/be/src/olap/tablet_meta.h +++ b/be/src/olap/tablet_meta.h @@ -336,7 +336,7 @@ class DeleteBitmap { using Version = uint64_t; using BitmapKey = std::tuple; std::map delete_bitmap; // Ordered map - constexpr static uint32_t ROWSET_SENTINEL_MARK = std::numeric_limits::max() - 1; + constexpr static inline uint32_t ROWSET_SENTINEL_MARK = std::numeric_limits::max() - 1; /** * diff --git a/be/src/olap/txn_manager.cpp b/be/src/olap/txn_manager.cpp index e3dc722b0bac40..481b25f2f390f4 100644 --- a/be/src/olap/txn_manager.cpp +++ b/be/src/olap/txn_manager.cpp @@ -152,10 +152,11 @@ Status TxnManager::prepare_txn(TPartitionId partition_id, TTransactionId transac Status TxnManager::commit_txn(TPartitionId partition_id, const TabletSharedPtr& tablet, TTransactionId transaction_id, const PUniqueId& load_id, - const RowsetSharedPtr& rowset_ptr, bool is_recovery) { + const RowsetSharedPtr& rowset_ptr, bool is_recovery, + bool do_correctness_check) { return commit_txn(tablet->data_dir()->get_meta(), partition_id, transaction_id, tablet->tablet_id(), tablet->schema_hash(), tablet->tablet_uid(), load_id, - rowset_ptr, is_recovery); + rowset_ptr, is_recovery, do_correctness_check); } Status TxnManager::publish_txn(TPartitionId partition_id, const TabletSharedPtr& tablet, @@ -217,7 +218,7 @@ Status TxnManager::commit_txn(OlapMeta* meta, TPartitionId partition_id, TTransactionId transaction_id, TTabletId tablet_id, SchemaHash schema_hash, TabletUid tablet_uid, const PUniqueId& load_id, const RowsetSharedPtr& rowset_ptr, - bool is_recovery) { + bool is_recovery, bool do_correctness_check) { if (partition_id < 1 || transaction_id < 1 || tablet_id < 1) { LOG(FATAL) << "invalid commit req " << " partition_id=" << partition_id << " transaction_id=" << transaction_id @@ -366,9 +367,9 @@ Status TxnManager::publish_txn(OlapMeta* meta, TPartitionId partition_id, tablet->create_transient_rowset_writer(rowset, &rowset_writer); int64_t t2 = MonotonicMicros(); - RETURN_IF_ERROR(tablet->update_delete_bitmap(rowset, tablet_txn_info.rowset_ids, - tablet_txn_info.delete_bitmap, transaction_id, - rowset_writer.get())); + RETURN_IF_ERROR(tablet->update_delete_bitmap( + rowset, tablet_txn_info.rowset_ids, tablet_txn_info.delete_bitmap, transaction_id, + rowset_writer.get(), tablet_txn_info.do_correctness_check)); int64_t t3 = MonotonicMicros(); stats->calc_delete_bitmap_time_us = t3 - t2; if (rowset->tablet_schema()->is_partial_update()) { diff --git a/be/src/olap/txn_manager.h b/be/src/olap/txn_manager.h index d024e71e16c845..cd3b7766cdbfbd 100644 --- a/be/src/olap/txn_manager.h +++ b/be/src/olap/txn_manager.h @@ -59,6 +59,7 @@ struct TabletTxnInfo { RowsetIdUnorderedSet rowset_ids; int64_t creation_time; bool ingest {false}; + bool do_correctness_check {false}; TabletTxnInfo(PUniqueId load_id, RowsetSharedPtr rowset) : load_id(load_id), rowset(rowset), creation_time(UnixSeconds()) {} @@ -123,7 +124,8 @@ class TxnManager { Status commit_txn(TPartitionId partition_id, const TabletSharedPtr& tablet, TTransactionId transaction_id, const PUniqueId& load_id, - const RowsetSharedPtr& rowset_ptr, bool is_recovery); + const RowsetSharedPtr& rowset_ptr, bool is_recovery, + bool do_correctness_check = false); Status publish_txn(TPartitionId partition_id, const TabletSharedPtr& tablet, TTransactionId transaction_id, const Version& version, @@ -138,8 +140,8 @@ class TxnManager { Status commit_txn(OlapMeta* meta, TPartitionId partition_id, TTransactionId transaction_id, TTabletId tablet_id, SchemaHash schema_hash, TabletUid tablet_uid, - const PUniqueId& load_id, const RowsetSharedPtr& rowset_ptr, - bool is_recovery); + const PUniqueId& load_id, const RowsetSharedPtr& rowset_ptr, bool is_recovery, + bool do_correctness_check = false); // remove a txn from txn manager // not persist rowset meta because From e58851586df691c5f3690955dc581d4dd4d484e0 Mon Sep 17 00:00:00 2001 From: bobhan1 Date: Fri, 28 Jul 2023 10:15:15 +0800 Subject: [PATCH 03/10] update --- be/src/olap/tablet.cpp | 4 +++- be/src/olap/tablet_meta.h | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index 9fa44d5638bef3..988c9e5e1dee03 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -3669,7 +3669,9 @@ Status Tablet::calc_delete_bitmap_between_segments( void Tablet::add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, const RowsetIdUnorderedSet& rowsetids) { for (const auto& rowsetid : rowsetids) { - delete_bitmap->add({rowsetid, 0, 0}, DeleteBitmap::ROWSET_SENTINEL_MARK); + delete_bitmap->add( + {rowsetid, DeleteBitmap::INVALID_SEGMENT_ID, DeleteBitmap::INVALID_VERSION}, + DeleteBitmap::ROWSET_SENTINEL_MARK); } } diff --git a/be/src/olap/tablet_meta.h b/be/src/olap/tablet_meta.h index 2f3d6dbb4aef14..b5d577b23ab618 100644 --- a/be/src/olap/tablet_meta.h +++ b/be/src/olap/tablet_meta.h @@ -336,7 +336,10 @@ class DeleteBitmap { using Version = uint64_t; using BitmapKey = std::tuple; std::map delete_bitmap; // Ordered map - constexpr static inline uint32_t ROWSET_SENTINEL_MARK = std::numeric_limits::max() - 1; + constexpr static inline uint32_t INVALID_SEGMENT_ID = std::numeric_limits::max(); + constexpr static inline uint64_t INVALID_VERSION = std::numeric_limits::max(); + constexpr static inline uint32_t ROWSET_SENTINEL_MARK = + std::numeric_limits::max() - 1; /** * From eb89da9a0d70bb9a9ef22fb8493c497ecd9ea160 Mon Sep 17 00:00:00 2001 From: bobhan1 Date: Fri, 28 Jul 2023 10:31:54 +0800 Subject: [PATCH 04/10] only print warn msg if check correctness failed due to 2PC --- be/src/olap/tablet.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index 988c9e5e1dee03..f604ff82bbf7d8 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -3691,7 +3691,8 @@ Status Tablet::check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, in auto it = result.find(std::get<0>(key)); if (it == result.end()) { LOG(WARNING) << "can't find rowsetid when checking delete bitmap correctness"; - return Status::InternalError("check delete bitmap correctness failed"); + DCHECK(false); + return Status::OK(); } if (bitmap.contains(DeleteBitmap::ROWSET_SENTINEL_MARK)) { it->second = true; @@ -3703,11 +3704,10 @@ Status Tablet::check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, in "rowset with RowsetId:" << rowsetid << "version:" << get_rowset(rowsetid)->version().to_string() << "max_version:" << max_version; - return Status::InternalError("check delete bitmap correctness failed"); + DCHECK(false); + return Status::OK(); } } - LOG(INFO) << "[check_delete_bitmap_correctness][max_version:" << max_version << "]" - << "rowset size:" << rowsets.size(); return Status::OK(); } } // namespace doris From 49f89e3b0d1aec84e7c8dd5adeef6bee5c735ccb Mon Sep 17 00:00:00 2001 From: bobhan1 Date: Fri, 28 Jul 2023 17:01:29 +0800 Subject: [PATCH 05/10] update --- be/src/common/config.cpp | 2 + be/src/common/config.h | 2 + be/src/olap/delta_writer.cpp | 5 +-- be/src/olap/tablet.cpp | 71 ++++++++++++++++++------------------ be/src/olap/tablet.h | 14 +++---- be/src/olap/tablet_meta.h | 3 +- be/src/olap/txn_manager.cpp | 13 +++---- be/src/olap/txn_manager.h | 8 ++-- 8 files changed, 57 insertions(+), 61 deletions(-) diff --git a/be/src/common/config.cpp b/be/src/common/config.cpp index ad7ed6614ce199..b94afa3a2c2c2b 100644 --- a/be/src/common/config.cpp +++ b/be/src/common/config.cpp @@ -1053,6 +1053,8 @@ DEFINE_Bool(enable_hdfs_hedged_read, "false"); DEFINE_Int32(hdfs_hedged_read_thread_num, "128"); DEFINE_Int32(hdfs_hedged_read_threshold_time, "500"); +DEFINE_mBool(enable_merge_on_write_correctness_check, "true"); + #ifdef BE_TEST // test s3 DEFINE_String(test_s3_resource, "resource"); diff --git a/be/src/common/config.h b/be/src/common/config.h index 416eae2880e7bd..632cfc5e9cada9 100644 --- a/be/src/common/config.h +++ b/be/src/common/config.h @@ -1109,6 +1109,8 @@ DECLARE_Int32(hdfs_hedged_read_thread_num); // Maybe overwritten by the value specified when creating catalog DECLARE_Int32(hdfs_hedged_read_threshold_time); +DECLARE_mBool(enable_merge_on_write_correctness_check); + #ifdef BE_TEST // test s3 DECLARE_String(test_s3_resource); diff --git a/be/src/olap/delta_writer.cpp b/be/src/olap/delta_writer.cpp index 00db6263f29886..947999a07630e3 100644 --- a/be/src/olap/delta_writer.cpp +++ b/be/src/olap/delta_writer.cpp @@ -321,11 +321,8 @@ Status DeltaWriter::commit_txn(const PSlaveTabletNodes& slave_tablet_nodes, const bool write_single_replica) { std::lock_guard l(_lock); SCOPED_TIMER(_close_wait_timer); - // only do correctness check if the rowset has at least one row written - bool do_correctness_check = (_rowset_writer->num_rows() != 0); Status res = _storage_engine->txn_manager()->commit_txn(_req.partition_id, _tablet, _req.txn_id, - _req.load_id, _cur_rowset, false, - do_correctness_check); + _req.load_id, _cur_rowset, false); if (!res && !res.is()) { LOG(WARNING) << "Failed to commit txn: " << _req.txn_id diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index f604ff82bbf7d8..ab4f25f6260af4 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -2981,10 +2981,12 @@ Status Tablet::calc_segment_delete_bitmap(RowsetSharedPtr rowset, for (const auto& rowset : specified_rowsets) { rowsetids.emplace(rowset->rowset_id()); LOG(INFO) << "[tabletID:" << tablet_id() << "]" - << "[add_sentinel_mark_to_delete_bitmap][end_version:" << end_version << "]" + << "[_add_sentinel_mark_to_delete_bitmap][end_version:" << end_version << "]" << "add:" << rowset->rowset_id(); } - add_sentinel_mark_to_delete_bitmap(delete_bitmap, rowsetids); + if (config::enable_merge_on_write_correctness_check) { + _add_sentinel_mark_to_delete_bitmap(delete_bitmap, rowsetids); + } if (pos > 0) { RETURN_IF_ERROR(generate_new_block_for_partial_update( @@ -3230,6 +3232,12 @@ Status Tablet::update_delete_bitmap_without_lock(const RowsetSharedPtr& rowset) << ", rowset_ids: " << cur_rowset_ids.size() << ", cur max_version: " << cur_version << ", transaction_id: " << -1 << ", cost: " << watch.get_elapse_time_us() << "(us), total rows: " << total_rows; + if (config::enable_merge_on_write_correctness_check) { + // check if all the rowset has ROWSET_SENTINEL_MARK + RETURN_IF_ERROR( + _check_delete_bitmap_correctness(delete_bitmap, cur_version - 1, cur_rowset_ids)); + _remove_sentinel_mark_from_delete_bitmap(delete_bitmap); + } for (auto iter = delete_bitmap->delete_bitmap.begin(); iter != delete_bitmap->delete_bitmap.end(); ++iter) { _tablet_meta->delete_bitmap().merge( @@ -3279,7 +3287,7 @@ Status Tablet::commit_phase_update_delete_bitmap( Status Tablet::update_delete_bitmap(const RowsetSharedPtr& rowset, const RowsetIdUnorderedSet& pre_rowset_ids, DeleteBitmapPtr delete_bitmap, int64_t txn_id, - RowsetWriter* rowset_writer, bool do_correctness_check) { + RowsetWriter* rowset_writer) { SCOPED_BVAR_LATENCY(g_tablet_update_delete_bitmap_latency); RowsetIdUnorderedSet cur_rowset_ids; RowsetIdUnorderedSet rowset_ids_to_add; @@ -3327,10 +3335,12 @@ Status Tablet::update_delete_bitmap(const RowsetSharedPtr& rowset, << ", cur max_version: " << cur_version << ", transaction_id: " << txn_id << ", cost: " << watch.get_elapse_time_us() << "(us), total rows: " << total_rows; - if (do_correctness_check) { + if (config::enable_merge_on_write_correctness_check && rowset->num_rows() != 0) { + // only do correctness check if the rowset has at least one row written // check if all the rowset has ROWSET_SENTINEL_MARK - RETURN_IF_ERROR(check_delete_bitmap_correctness(delete_bitmap, cur_version - 1)); - remove_sentinel_mark_from_delete_bitmap(delete_bitmap); + RETURN_IF_ERROR( + _check_delete_bitmap_correctness(delete_bitmap, cur_version - 1, cur_rowset_ids)); + _remove_sentinel_mark_from_delete_bitmap(delete_bitmap); } // update version without write lock, compaction and publish_txn @@ -3666,45 +3676,34 @@ Status Tablet::calc_delete_bitmap_between_segments( return Status::OK(); } -void Tablet::add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, - const RowsetIdUnorderedSet& rowsetids) { +void Tablet::_add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, + const RowsetIdUnorderedSet& rowsetids) { for (const auto& rowsetid : rowsetids) { - delete_bitmap->add( - {rowsetid, DeleteBitmap::INVALID_SEGMENT_ID, DeleteBitmap::INVALID_VERSION}, - DeleteBitmap::ROWSET_SENTINEL_MARK); + delete_bitmap->add({rowsetid, DeleteBitmap::INVALID_SEGMENT_ID, 0}, + DeleteBitmap::ROWSET_SENTINEL_MARK); } } -void Tablet::remove_sentinel_mark_from_delete_bitmap(DeleteBitmapPtr delete_bitmap) { - for (auto& [key, bitmap] : delete_bitmap->delete_bitmap) { - bitmap.remove(DeleteBitmap::ROWSET_SENTINEL_MARK); +void Tablet::_remove_sentinel_mark_from_delete_bitmap(DeleteBitmapPtr delete_bitmap) { + for (auto it = delete_bitmap->delete_bitmap.begin(), end = delete_bitmap->delete_bitmap.end(); + it != end;) { + if (std::get<1>(it->first) == DeleteBitmap::INVALID_SEGMENT_ID) { + it = delete_bitmap->delete_bitmap.erase(it); + } else { + ++it; + } } } -Status Tablet::check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, int64_t max_version) { - std::map result; - RowsetIdUnorderedSet rowsets = all_rs_id(max_version); - for (const auto& rowsetid : rowsets) { - result.emplace(rowsetid, false); - } - for (const auto& [key, bitmap] : delete_bitmap->delete_bitmap) { - auto it = result.find(std::get<0>(key)); - if (it == result.end()) { - LOG(WARNING) << "can't find rowsetid when checking delete bitmap correctness"; - DCHECK(false); - return Status::OK(); - } - if (bitmap.contains(DeleteBitmap::ROWSET_SENTINEL_MARK)) { - it->second = true; - } - } - for (const auto& [rowsetid, value] : result) { - if (!value) { +Status Tablet::_check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, int64_t max_version, + const RowsetIdUnorderedSet& rowset_ids) const { + for (const auto& rowsetid : rowset_ids) { + if (!delete_bitmap->delete_bitmap.contains( + {rowsetid, DeleteBitmap::INVALID_SEGMENT_ID, 0})) { LOG(WARNING) << "check delete bitmap correctness failed, can't find sentinel mark in " "rowset with RowsetId:" - << rowsetid << "version:" << get_rowset(rowsetid)->version().to_string() - << "max_version:" << max_version; - DCHECK(false); + << rowsetid << "max_version:" << max_version; + DCHECK(false) << "check delete bitmap correctness failed!"; return Status::OK(); } } diff --git a/be/src/olap/tablet.h b/be/src/olap/tablet.h index 30b5179cc30cdc..a056cee4ef90ea 100644 --- a/be/src/olap/tablet.h +++ b/be/src/olap/tablet.h @@ -486,8 +486,7 @@ class Tablet : public BaseTablet { Status update_delete_bitmap(const RowsetSharedPtr& rowset, const RowsetIdUnorderedSet& pre_rowset_ids, DeleteBitmapPtr delete_bitmap, int64_t txn_id, - RowsetWriter* rowset_writer = nullptr, - bool do_correctness_check = false); + RowsetWriter* rowset_writer = nullptr); void calc_compaction_output_rowset_delete_bitmap( const std::vector& input_rowsets, const RowIdConversion& rowid_conversion, uint64_t start_version, uint64_t end_version, @@ -551,11 +550,6 @@ class Tablet : public BaseTablet { void set_binlog_config(BinlogConfig binlog_config); - void add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, - const RowsetIdUnorderedSet& rowsetids); - void remove_sentinel_mark_from_delete_bitmap(DeleteBitmapPtr delete_bitmap); - Status check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, int64_t max_version); - private: Status _init_once_action(); void _print_missed_versions(const std::vector& missed_versions) const; @@ -603,6 +597,12 @@ class Tablet : public BaseTablet { // end cooldown functions //////////////////////////////////////////////////////////////////////////// + void _add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, + const RowsetIdUnorderedSet& rowsetids); + void _remove_sentinel_mark_from_delete_bitmap(DeleteBitmapPtr delete_bitmap); + Status _check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, int64_t max_version, + const RowsetIdUnorderedSet& rowset_ids) const; + public: static const int64_t K_INVALID_CUMULATIVE_POINT = -1; diff --git a/be/src/olap/tablet_meta.h b/be/src/olap/tablet_meta.h index b5d577b23ab618..c0dcdc249a15fe 100644 --- a/be/src/olap/tablet_meta.h +++ b/be/src/olap/tablet_meta.h @@ -336,8 +336,7 @@ class DeleteBitmap { using Version = uint64_t; using BitmapKey = std::tuple; std::map delete_bitmap; // Ordered map - constexpr static inline uint32_t INVALID_SEGMENT_ID = std::numeric_limits::max(); - constexpr static inline uint64_t INVALID_VERSION = std::numeric_limits::max(); + constexpr static inline uint32_t INVALID_SEGMENT_ID = std::numeric_limits::max() - 1; constexpr static inline uint32_t ROWSET_SENTINEL_MARK = std::numeric_limits::max() - 1; diff --git a/be/src/olap/txn_manager.cpp b/be/src/olap/txn_manager.cpp index 481b25f2f390f4..e3dc722b0bac40 100644 --- a/be/src/olap/txn_manager.cpp +++ b/be/src/olap/txn_manager.cpp @@ -152,11 +152,10 @@ Status TxnManager::prepare_txn(TPartitionId partition_id, TTransactionId transac Status TxnManager::commit_txn(TPartitionId partition_id, const TabletSharedPtr& tablet, TTransactionId transaction_id, const PUniqueId& load_id, - const RowsetSharedPtr& rowset_ptr, bool is_recovery, - bool do_correctness_check) { + const RowsetSharedPtr& rowset_ptr, bool is_recovery) { return commit_txn(tablet->data_dir()->get_meta(), partition_id, transaction_id, tablet->tablet_id(), tablet->schema_hash(), tablet->tablet_uid(), load_id, - rowset_ptr, is_recovery, do_correctness_check); + rowset_ptr, is_recovery); } Status TxnManager::publish_txn(TPartitionId partition_id, const TabletSharedPtr& tablet, @@ -218,7 +217,7 @@ Status TxnManager::commit_txn(OlapMeta* meta, TPartitionId partition_id, TTransactionId transaction_id, TTabletId tablet_id, SchemaHash schema_hash, TabletUid tablet_uid, const PUniqueId& load_id, const RowsetSharedPtr& rowset_ptr, - bool is_recovery, bool do_correctness_check) { + bool is_recovery) { if (partition_id < 1 || transaction_id < 1 || tablet_id < 1) { LOG(FATAL) << "invalid commit req " << " partition_id=" << partition_id << " transaction_id=" << transaction_id @@ -367,9 +366,9 @@ Status TxnManager::publish_txn(OlapMeta* meta, TPartitionId partition_id, tablet->create_transient_rowset_writer(rowset, &rowset_writer); int64_t t2 = MonotonicMicros(); - RETURN_IF_ERROR(tablet->update_delete_bitmap( - rowset, tablet_txn_info.rowset_ids, tablet_txn_info.delete_bitmap, transaction_id, - rowset_writer.get(), tablet_txn_info.do_correctness_check)); + RETURN_IF_ERROR(tablet->update_delete_bitmap(rowset, tablet_txn_info.rowset_ids, + tablet_txn_info.delete_bitmap, transaction_id, + rowset_writer.get())); int64_t t3 = MonotonicMicros(); stats->calc_delete_bitmap_time_us = t3 - t2; if (rowset->tablet_schema()->is_partial_update()) { diff --git a/be/src/olap/txn_manager.h b/be/src/olap/txn_manager.h index cd3b7766cdbfbd..d024e71e16c845 100644 --- a/be/src/olap/txn_manager.h +++ b/be/src/olap/txn_manager.h @@ -59,7 +59,6 @@ struct TabletTxnInfo { RowsetIdUnorderedSet rowset_ids; int64_t creation_time; bool ingest {false}; - bool do_correctness_check {false}; TabletTxnInfo(PUniqueId load_id, RowsetSharedPtr rowset) : load_id(load_id), rowset(rowset), creation_time(UnixSeconds()) {} @@ -124,8 +123,7 @@ class TxnManager { Status commit_txn(TPartitionId partition_id, const TabletSharedPtr& tablet, TTransactionId transaction_id, const PUniqueId& load_id, - const RowsetSharedPtr& rowset_ptr, bool is_recovery, - bool do_correctness_check = false); + const RowsetSharedPtr& rowset_ptr, bool is_recovery); Status publish_txn(TPartitionId partition_id, const TabletSharedPtr& tablet, TTransactionId transaction_id, const Version& version, @@ -140,8 +138,8 @@ class TxnManager { Status commit_txn(OlapMeta* meta, TPartitionId partition_id, TTransactionId transaction_id, TTabletId tablet_id, SchemaHash schema_hash, TabletUid tablet_uid, - const PUniqueId& load_id, const RowsetSharedPtr& rowset_ptr, bool is_recovery, - bool do_correctness_check = false); + const PUniqueId& load_id, const RowsetSharedPtr& rowset_ptr, + bool is_recovery); // remove a txn from txn manager // not persist rowset meta because From 4137eea06fec6cae74331c0a235061776e718edb Mon Sep 17 00:00:00 2001 From: bobhan1 Date: Thu, 10 Aug 2023 09:53:25 +0800 Subject: [PATCH 06/10] update --- be/src/olap/tablet.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index ab4f25f6260af4..6087b46195d285 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -2977,14 +2977,14 @@ Status Tablet::calc_segment_delete_bitmap(RowsetSharedPtr rowset, } DCHECK_EQ(total, row_id) << "segment total rows: " << total << " row_id:" << row_id; - RowsetIdUnorderedSet rowsetids; - for (const auto& rowset : specified_rowsets) { - rowsetids.emplace(rowset->rowset_id()); - LOG(INFO) << "[tabletID:" << tablet_id() << "]" - << "[_add_sentinel_mark_to_delete_bitmap][end_version:" << end_version << "]" - << "add:" << rowset->rowset_id(); - } if (config::enable_merge_on_write_correctness_check) { + RowsetIdUnorderedSet rowsetids; + for (const auto& rowset : specified_rowsets) { + rowsetids.emplace(rowset->rowset_id()); + LOG(INFO) << "[tabletID:" << tablet_id() << "]" + << "[_add_sentinel_mark_to_delete_bitmap][end_version:" << end_version << "]" + << "add:" << rowset->rowset_id(); + } _add_sentinel_mark_to_delete_bitmap(delete_bitmap, rowsetids); } From 5d0d2618b1a21b91c6af23837333f5b7c4a2796d Mon Sep 17 00:00:00 2001 From: bobhan1 Date: Thu, 10 Aug 2023 15:29:44 +0800 Subject: [PATCH 07/10] update log --- be/src/olap/tablet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index 6087b46195d285..f0290de620bf88 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -3702,7 +3702,7 @@ Status Tablet::_check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, i {rowsetid, DeleteBitmap::INVALID_SEGMENT_ID, 0})) { LOG(WARNING) << "check delete bitmap correctness failed, can't find sentinel mark in " "rowset with RowsetId:" - << rowsetid << "max_version:" << max_version; + << rowsetid << ",max_version:" << max_version; DCHECK(false) << "check delete bitmap correctness failed!"; return Status::OK(); } From f5f56f63c7d0c4561336c71fa6b73942235429bf Mon Sep 17 00:00:00 2001 From: bobhan1 Date: Thu, 10 Aug 2023 16:40:42 +0800 Subject: [PATCH 08/10] add mark in flush phase when doing partial update --- be/src/olap/rowset/segment_v2/segment_writer.cpp | 5 +++++ be/src/olap/tablet.cpp | 6 +++--- be/src/olap/tablet.h | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/be/src/olap/rowset/segment_v2/segment_writer.cpp b/be/src/olap/rowset/segment_v2/segment_writer.cpp index 4f303efe016b3a..896a38493daa3d 100644 --- a/be/src/olap/rowset/segment_v2/segment_writer.cpp +++ b/be/src/olap/rowset/segment_v2/segment_writer.cpp @@ -451,6 +451,11 @@ Status SegmentWriter::append_block_with_partial_content(const vectorized::Block* } CHECK(use_default_or_null_flag.size() == num_rows); + if (config::enable_merge_on_write_correctness_check) { + _tablet->add_sentinel_mark_to_delete_bitmap(_mow_context->delete_bitmap, + _mow_context->rowset_ids); + } + // read and fill block auto mutable_full_columns = full_block.mutate_columns(); RETURN_IF_ERROR(fill_missing_columns(mutable_full_columns, use_default_or_null_flag, diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index f0290de620bf88..f14013c1e583b1 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -2982,10 +2982,10 @@ Status Tablet::calc_segment_delete_bitmap(RowsetSharedPtr rowset, for (const auto& rowset : specified_rowsets) { rowsetids.emplace(rowset->rowset_id()); LOG(INFO) << "[tabletID:" << tablet_id() << "]" - << "[_add_sentinel_mark_to_delete_bitmap][end_version:" << end_version << "]" + << "[add_sentinel_mark_to_delete_bitmap][end_version:" << end_version << "]" << "add:" << rowset->rowset_id(); } - _add_sentinel_mark_to_delete_bitmap(delete_bitmap, rowsetids); + add_sentinel_mark_to_delete_bitmap(delete_bitmap, rowsetids); } if (pos > 0) { @@ -3676,7 +3676,7 @@ Status Tablet::calc_delete_bitmap_between_segments( return Status::OK(); } -void Tablet::_add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, +void Tablet::add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, const RowsetIdUnorderedSet& rowsetids) { for (const auto& rowsetid : rowsetids) { delete_bitmap->add({rowsetid, DeleteBitmap::INVALID_SEGMENT_ID, 0}, diff --git a/be/src/olap/tablet.h b/be/src/olap/tablet.h index a056cee4ef90ea..d78dfe1ff5b4e1 100644 --- a/be/src/olap/tablet.h +++ b/be/src/olap/tablet.h @@ -549,6 +549,8 @@ class Tablet : public BaseTablet { int64_t binlog_max_bytes() const { return _tablet_meta->binlog_config().max_bytes(); } void set_binlog_config(BinlogConfig binlog_config); + void add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, + const RowsetIdUnorderedSet& rowsetids); private: Status _init_once_action(); @@ -597,8 +599,6 @@ class Tablet : public BaseTablet { // end cooldown functions //////////////////////////////////////////////////////////////////////////// - void _add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, - const RowsetIdUnorderedSet& rowsetids); void _remove_sentinel_mark_from_delete_bitmap(DeleteBitmapPtr delete_bitmap); Status _check_delete_bitmap_correctness(DeleteBitmapPtr delete_bitmap, int64_t max_version, const RowsetIdUnorderedSet& rowset_ids) const; From 1f2e34d69ee04faa7d08474d0a10a9168429683e Mon Sep 17 00:00:00 2001 From: bobhan1 Date: Thu, 10 Aug 2023 19:36:26 +0800 Subject: [PATCH 09/10] format --- be/src/olap/tablet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp index f14013c1e583b1..00d19639f0753a 100644 --- a/be/src/olap/tablet.cpp +++ b/be/src/olap/tablet.cpp @@ -3677,7 +3677,7 @@ Status Tablet::calc_delete_bitmap_between_segments( } void Tablet::add_sentinel_mark_to_delete_bitmap(DeleteBitmapPtr delete_bitmap, - const RowsetIdUnorderedSet& rowsetids) { + const RowsetIdUnorderedSet& rowsetids) { for (const auto& rowsetid : rowsetids) { delete_bitmap->add({rowsetid, DeleteBitmap::INVALID_SEGMENT_ID, 0}, DeleteBitmap::ROWSET_SENTINEL_MARK); From 4ae4b223f228ba079d69ccba46dbd71beb3a7a50 Mon Sep 17 00:00:00 2001 From: bobhan1 Date: Fri, 11 Aug 2023 09:54:51 +0800 Subject: [PATCH 10/10] format --- be/src/common/config.h | 1 - 1 file changed, 1 deletion(-) diff --git a/be/src/common/config.h b/be/src/common/config.h index e3749cc724b6e5..bd009f2b4ed87b 100644 --- a/be/src/common/config.h +++ b/be/src/common/config.h @@ -1116,7 +1116,6 @@ DECLARE_mBool(enable_merge_on_write_correctness_check); // The secure path with user files, used in the `local` table function. DECLARE_mString(user_files_secure_path); - #ifdef BE_TEST // test s3 DECLARE_String(test_s3_resource);