From c6ce2a709e0cfdb56829eed2c63d99d18d64084b Mon Sep 17 00:00:00 2001 From: srlch Date: Tue, 25 Feb 2025 17:06:29 +0800 Subject: [PATCH 1/2] [Enhancement] Wait apply finish out of tablet meta lock when set tablet state as SHUTDOWN ## Why I'm doing: In current impl, when set state as SHUTDOWN for primary key tablet, BE will wait until apply task has been finished inside the tablet meta lock leading many lock contention problem. But it is no need to hold the tablet meta lock when waiting the finishing of the background apply task. ## What I'm doing: Lock free for tablet meta lock to wait the apply task before setting the state as SHUTDOWN. Signed-off-by: srlch --- be/src/storage/tablet_manager.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/be/src/storage/tablet_manager.cpp b/be/src/storage/tablet_manager.cpp index 2c8cc5a4e8236..1b9c9f9869a49 100644 --- a/be/src/storage/tablet_manager.cpp +++ b/be/src/storage/tablet_manager.cpp @@ -407,6 +407,13 @@ Status TabletManager::drop_tablet(TTabletId tablet_id, TabletDropFlag flag) { DroppedTabletInfo drop_info{.tablet = dropped_tablet, .flag = flag}; + // meta lock free shutdown for primary key table. In current impl, TabletUpdates's context + // is protected by specified lock defined in TabletUpdates itself but not tablet meta lock + // It is safe call _stop_and_wait_apply_done out of tablet meta lock + if (dropped_tablet->updates() != nullptr) { + dropped_tablet->updates()->on_shutdown(); + } + if (flag == kDeleteFiles) { { // NOTE: Other threads may save the tablet meta back to storage again after we @@ -492,6 +499,13 @@ Status TabletManager::drop_tablets_on_error_root_path(const std::vectorupdates() != nullptr) { + dropped_tablet->updates()->on_shutdown(); + } + // make sure dropped tablet state is TABLET_SHUTDOWN IN MEMORY ONLY! // any persistent operation is useless because the disk has failed // and the IO operation should be always failed in this case. @@ -1520,6 +1534,13 @@ Status TabletManager::_drop_tablet_unlocked(TTabletId tablet_id, TabletDropFlag DroppedTabletInfo drop_info{.tablet = dropped_tablet, .flag = flag}; + // meta lock free shutdown for primary key table. In current impl, TabletUpdates's context + // is protected by specified lock defined in TabletUpdates itself but not tablet meta lock + // It is safe call _stop_and_wait_apply_done out of tablet meta lock + if (dropped_tablet->updates() != nullptr) { + dropped_tablet->updates()->on_shutdown(); + } + if (flag == kDeleteFiles) { { // NOTE: Other threads may save the tablet meta back to storage again after we From 3affb7fe4c01fef481e8cb93d0f303ed5d230c1d Mon Sep 17 00:00:00 2001 From: srlch Date: Thu, 27 Feb 2025 11:46:04 +0800 Subject: [PATCH 2/2] fix Signed-off-by: srlch --- be/src/storage/tablet.cpp | 2 +- be/src/storage/tablet_manager.cpp | 12 ++++++------ be/src/storage/tablet_updates.cpp | 8 ++++---- be/src/storage/tablet_updates.h | 3 ++- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/be/src/storage/tablet.cpp b/be/src/storage/tablet.cpp index 5fd44e6c5553f..e0a7e10093156 100644 --- a/be/src/storage/tablet.cpp +++ b/be/src/storage/tablet.cpp @@ -1622,7 +1622,7 @@ Status Tablet::rowset_commit(int64_t version, const RowsetSharedPtr& rowset, uin void Tablet::on_shutdown() { if (_updates) { - _updates->_stop_and_wait_apply_done(); + _updates->stop_and_wait_apply_done(); } } diff --git a/be/src/storage/tablet_manager.cpp b/be/src/storage/tablet_manager.cpp index 1b9c9f9869a49..bf44826630a36 100644 --- a/be/src/storage/tablet_manager.cpp +++ b/be/src/storage/tablet_manager.cpp @@ -409,9 +409,9 @@ Status TabletManager::drop_tablet(TTabletId tablet_id, TabletDropFlag flag) { // meta lock free shutdown for primary key table. In current impl, TabletUpdates's context // is protected by specified lock defined in TabletUpdates itself but not tablet meta lock - // It is safe call _stop_and_wait_apply_done out of tablet meta lock + // It is safe call stop_and_wait_apply_done out of tablet meta lock if (dropped_tablet->updates() != nullptr) { - dropped_tablet->updates()->on_shutdown(); + dropped_tablet->updates()->stop_and_wait_apply_done(); } if (flag == kDeleteFiles) { @@ -501,9 +501,9 @@ Status TabletManager::drop_tablets_on_error_root_path(const std::vectorupdates() != nullptr) { - dropped_tablet->updates()->on_shutdown(); + dropped_tablet->updates()->stop_and_wait_apply_done(); } // make sure dropped tablet state is TABLET_SHUTDOWN IN MEMORY ONLY! @@ -1536,9 +1536,9 @@ Status TabletManager::_drop_tablet_unlocked(TTabletId tablet_id, TabletDropFlag // meta lock free shutdown for primary key table. In current impl, TabletUpdates's context // is protected by specified lock defined in TabletUpdates itself but not tablet meta lock - // It is safe call _stop_and_wait_apply_done out of tablet meta lock + // It is safe call stop_and_wait_apply_done out of tablet meta lock if (dropped_tablet->updates() != nullptr) { - dropped_tablet->updates()->on_shutdown(); + dropped_tablet->updates()->stop_and_wait_apply_done(); } if (flag == kDeleteFiles) { diff --git a/be/src/storage/tablet_updates.cpp b/be/src/storage/tablet_updates.cpp index 9242c50325666..79dc19ec122db 100644 --- a/be/src/storage/tablet_updates.cpp +++ b/be/src/storage/tablet_updates.cpp @@ -84,7 +84,7 @@ std::string EditVersion::to_string() const { TabletUpdates::TabletUpdates(Tablet& tablet) : _tablet(tablet), _unused_rowsets(UINT64_MAX) {} TabletUpdates::~TabletUpdates() { - _stop_and_wait_apply_done(); + stop_and_wait_apply_done(); } template @@ -1028,7 +1028,7 @@ void TabletUpdates::_wait_apply_done() { } } -void TabletUpdates::_stop_and_wait_apply_done() { +void TabletUpdates::stop_and_wait_apply_done() { _apply_stopped = true; _wait_apply_done(); } @@ -4912,7 +4912,7 @@ Status TabletUpdates::load_snapshot(const SnapshotMeta& snapshot_meta, bool rest RETURN_IF_ERROR(check_rowset_files(rowset_meta_pb)); } // Stop apply thread. - _stop_and_wait_apply_done(); + stop_and_wait_apply_done(); DeferOp defer([&]() { if (!_error.load()) { @@ -5644,7 +5644,7 @@ Status TabletUpdates::recover() { } LOG(INFO) << "Tablet " << _tablet.tablet_id() << " begin do recover: " << _error_msg; // Stop apply thread. - _stop_and_wait_apply_done(); + stop_and_wait_apply_done(); DeferOp defer([&]() { if (!_error.load()) { diff --git a/be/src/storage/tablet_updates.h b/be/src/storage/tablet_updates.h index 2b588b6f08b83..2160b8bf2bcf1 100644 --- a/be/src/storage/tablet_updates.h +++ b/be/src/storage/tablet_updates.h @@ -386,6 +386,8 @@ class TabletUpdates { void rewrite_rs_meta(bool is_fatal); + void stop_and_wait_apply_done(); + private: friend class Tablet; friend class PrimaryIndex; @@ -440,7 +442,6 @@ class TabletUpdates { EditVersion* commit_version); void _wait_apply_done(); - void _stop_and_wait_apply_done(); Status _do_compaction(std::unique_ptr* pinfo);