From: Matan Breizman Date: Tue, 14 Jul 2026 14:15:53 +0000 (+0000) Subject: crimson/seastore: cleanup collection-lock leftovers X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=32e28b9a70313455fd2c5108ef147ca9f2ba6ec9;p=ceph.git crimson/seastore: cleanup collection-lock leftovers Signed-off-by: Matan Breizman --- diff --git a/src/crimson/os/seastore/ordering_handle.h b/src/crimson/os/seastore/ordering_handle.h index e4b375416e86..957b39f62438 100644 --- a/src/crimson/os/seastore/ordering_handle.h +++ b/src/crimson/os/seastore/ordering_handle.h @@ -123,45 +123,12 @@ struct OrderingHandle { // we can easily optimize this dynalloc out as all concretes are // supposed to have exactly the same size. std::unique_ptr op; - seastar::shared_mutex *collection_ordering_lock = nullptr; - - seastar::lowres_clock::time_point lock_acquire_time{}; - seastar::lowres_clock::duration lock_hold_time{0}; // in the future we might add further constructors / template to type // erasure while extracting the location of tracking events. OrderingHandle(std::unique_ptr op) : op(std::move(op)) {} OrderingHandle(OrderingHandle &&other) - : op(std::move(other.op)), - collection_ordering_lock(other.collection_ordering_lock), - lock_acquire_time(other.lock_acquire_time), - lock_hold_time(other.lock_hold_time) { - other.collection_ordering_lock = nullptr; - } - - seastar::future<> take_collection_lock(seastar::shared_mutex &mutex) { - ceph_assert(!collection_ordering_lock); - collection_ordering_lock = &mutex; - return collection_ordering_lock->lock(); - } - - void set_lock_acquire_time(seastar::lowres_clock::time_point tp) { - lock_acquire_time = tp; - } - - seastar::lowres_clock::duration get_lock_hold_time() const { - return lock_hold_time; - } - - void maybe_release_collection_lock() { - if (collection_ordering_lock) { - if (lock_acquire_time != seastar::lowres_clock::time_point{}) { - lock_hold_time = seastar::lowres_clock::now() - lock_acquire_time; - } - collection_ordering_lock->unlock(); - collection_ordering_lock = nullptr; - } - } + : op(std::move(other.op)) {} template seastar::future<> enter(T &t) { @@ -175,10 +142,6 @@ struct OrderingHandle { seastar::future<> complete() { return op->complete(); } - - ~OrderingHandle() { - maybe_release_collection_lock(); - } }; inline OrderingHandle get_dummy_ordering_handle() { diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc index 1102bfbe768a..8baa7951c5ef 100644 --- a/src/crimson/os/seastore/seastore.cc +++ b/src/crimson/os/seastore/seastore.cc @@ -215,8 +215,6 @@ void SeaStore::Shard::register_metrics(store_index_t store_index) ); std::pair labels_by_stage[] = { - {txn_stage_t::COLLOCK_WAIT, sm::label_instance("stage", "collock_wait")}, - {txn_stage_t::COLLOCK_HOLD, sm::label_instance("stage", "collock_hold")}, {txn_stage_t::THROTTLER_WAIT, sm::label_instance("stage", "throttler_wait")}, {txn_stage_t::BUILD, sm::label_instance("stage", "build")}, {txn_stage_t::BUILD_GET_ONODE, sm::label_instance("stage", "build_get_onode")}, @@ -921,14 +919,13 @@ seastar::future<> SeaStore::report_stats() calc_conflicts(io_total.read_num, io_total.repeat_read_num), calc_conflicts(io_total.get_bg_num(), io_total.get_repeat_bg_num())); INFO("trans outstanding: {},{},{},{} " - "per-shard: {:.2f}({:.2f},{:.2f},{:.2f},{:.2f},{:.2f}),{:.2f},{:.2f},{:.2f}", + "per-shard: {:.2f}({:.2f},{:.2f},{:.2f},{:.2f}),{:.2f},{:.2f},{:.2f}", io_total.pending_io_num, io_total.pending_read_num, io_total.pending_bg_num, io_total.pending_flush_num, (double)io_total.pending_io_num/seastar::smp::count, (double)io_total.starting_io_num/seastar::smp::count, - (double)io_total.waiting_collock_io_num/seastar::smp::count, (double)io_total.waiting_throttler_io_num/seastar::smp::count, (double)io_total.processing_inlock_io_num/seastar::smp::count, (double)io_total.processing_postlock_io_num/seastar::smp::count, @@ -940,7 +937,6 @@ seastar::future<> SeaStore::report_stats() for (const auto &s : shard_io_stats) { oss_pending << s.pending_io_num << "(" << s.starting_io_num - << "," << s.waiting_collock_io_num << "," << s.waiting_throttler_io_num << "," << s.processing_inlock_io_num << "," << s.processing_postlock_io_num @@ -1889,8 +1885,6 @@ seastar::future<> SeaStore::Shard::run_one_batch( const std::array< std::pair, STAGE_MAX> stage_samples = {{ - {txn_stage_t::COLLOCK_WAIT, seastar::lowres_clock::duration::zero()}, - {txn_stage_t::COLLOCK_HOLD, seastar::lowres_clock::duration::zero()}, {txn_stage_t::THROTTLER_WAIT, throttler_wait}, {txn_stage_t::BUILD, ctx.build_time}, {txn_stage_t::BUILD_GET_ONODE, ctx.get_onode_time}, @@ -3051,7 +3045,7 @@ shard_stats_t SeaStore::Shard::get_io_stats( }; INFO("iops={:.2f},{:.2f},{:.2f}({:.2f},{:.2f},{:.2f},{:.2f}),{:.2f} " "conflicts={:.2f},{:.2f},{:.2f}({:.2f},{:.2f},{:.2f},{:.2f}) " - "outstanding={}({},{},{},{},{}),{},{},{}", + "outstanding={}({},{},{},{}),{},{},{}", // iops ret.io_num/seconds, ret.read_num/seconds, @@ -3072,7 +3066,6 @@ shard_stats_t SeaStore::Shard::get_io_stats( // outstanding ret.pending_io_num, ret.starting_io_num, - ret.waiting_collock_io_num, ret.waiting_throttler_io_num, ret.processing_inlock_io_num, ret.processing_postlock_io_num, diff --git a/src/crimson/os/seastore/seastore.h b/src/crimson/os/seastore/seastore.h index 7ad4a20d1dd5..605e444b8e72 100644 --- a/src/crimson/os/seastore/seastore.h +++ b/src/crimson/os/seastore/seastore.h @@ -49,9 +49,7 @@ enum class op_type_t : uint8_t { }; enum class txn_stage_t : uint8_t { - COLLOCK_WAIT = 0, // waiting on the collection ordering_lock - COLLOCK_HOLD, // collection ordering_lock held (acquire -> release at prepare_record) - THROTTLER_WAIT, // waiting for a throttler slot + THROTTLER_WAIT = 0, // waiting for a throttler slot BUILD, // building the transaction (_do_transaction_step loop) BUILD_GET_ONODE, // onode_manager get/get_or_create calls within BUILD SUBMIT_TOTAL, // the whole submit_transaction (pipeline + journal write) @@ -71,8 +69,6 @@ public: SeastoreCollection(T&&... args) : FuturizedCollection(std::forward(args)...) {} - seastar::shared_mutex ordering_lock; - struct batch_entry_t { ceph::os::Transaction txn; seastar::promise<> pr; diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h index 082baeacea34..58b23da38147 100644 --- a/src/crimson/os/seastore/seastore_types.h +++ b/src/crimson/os/seastore/seastore_types.h @@ -3293,7 +3293,6 @@ struct shard_stats_t { uint64_t repeat_io_num = 0; uint64_t pending_io_num = 0; uint64_t starting_io_num = 0; - uint64_t waiting_collock_io_num = 0; uint64_t waiting_throttler_io_num = 0; uint64_t processing_inlock_io_num = 0; uint64_t processing_postlock_io_num = 0; @@ -3336,7 +3335,6 @@ struct shard_stats_t { repeat_io_num += o.repeat_io_num; pending_io_num += o.pending_io_num; starting_io_num += o.starting_io_num; - waiting_collock_io_num += o.waiting_collock_io_num; waiting_throttler_io_num += o.waiting_throttler_io_num; processing_inlock_io_num += o.processing_inlock_io_num; processing_postlock_io_num += o.processing_postlock_io_num; diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index a8d28e276f3c..b4bfd50a2a57 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -703,7 +703,6 @@ TransactionManager::do_submit_transaction( tref.get_phase_durations().prepare_record += std::chrono::steady_clock::now() - prepare_record_start; - tref.get_handle().maybe_release_collection_lock(); if (tref.get_src() == Transaction::src_t::MUTATE) { --(shard_stats.processing_inlock_io_num); ++(shard_stats.processing_postlock_io_num); @@ -749,7 +748,6 @@ seastar::future<> TransactionManager::flush(OrderingHandle &handle) }).then([this, &handle] { return handle.enter(write_pipeline.prepare); }).then([this, &handle] { - handle.maybe_release_collection_lock(); return journal->flush(handle); }).then([FNAME, &handle] { SUBDEBUG(seastore_t, "H{} completed", (void*)&handle);