From b4194fe967b413386b256e2bad2c331a19ba57b1 Mon Sep 17 00:00:00 2001 From: Matan Breizman Date: Wed, 3 Jun 2026 09:39:52 +0000 Subject: [PATCH 1/3] crimson/os/seastore: make retry helper policy-free with_repeat_trans_intr() was previously mixing retry mechanics with transaction-specific behavior such as reset and preserve handle. instead, introduce prepare_attempt hook to run before each retry attempt. Keep the older variant overload with no prepare (internal users) Also, fix func/args forwarding to avoid re-moving. Signed-off-by: Matan Breizman --- .../os/seastore/transaction_interruptor.h | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/crimson/os/seastore/transaction_interruptor.h b/src/crimson/os/seastore/transaction_interruptor.h index c79cce9932b..b2dde2b88b2 100644 --- a/src/crimson/os/seastore/transaction_interruptor.h +++ b/src/crimson/os/seastore/transaction_interruptor.h @@ -68,25 +68,35 @@ using base_iertr = trans_iertr; template auto with_trans_intr(Transaction &t, F &&f, Args&&... args) { return trans_intr::with_interruption_to_error( - std::move(f), + std::forward(f), TransactionConflictCondition(t), t, std::forward(args)...); } -template -auto with_repeat_trans_intr(Transaction &t, F &&f, Args&&... args) { - return repeat_eagain([&t, f=std::move(f), ... args = std::forward(args) ] { - // Try again with a fresh transaction - t.reset_preserve_handle(); +template +auto with_repeat_trans_intr(Prepare &&prepare_attempt, Transaction &t, F &&f, Args&&... args) { + return repeat_eagain( + [&t, + prepare_attempt = std::forward(prepare_attempt), + f = std::forward(f), + ... args = std::forward(args) ] { + prepare_attempt(); return trans_intr::with_interruption_to_error( - std::move(f), + f, TransactionConflictCondition(t), t, - std::forward(args)...); + args...); }); +} - +template +auto with_repeat_trans_intr(Transaction &t, F &&f, Args&&... args) { + return with_repeat_trans_intr( + [] {}, + t, + std::forward(f), + std::forward(args)...); } template From eca68d7fd460329db5bf57836f8c56363891cfb1 Mon Sep 17 00:00:00 2001 From: Matan Breizman Date: Wed, 3 Jun 2026 09:43:19 +0000 Subject: [PATCH 2/3] crimson/os/seastore: move reset_preserve_handle to retry prepare step do_transaction_no_callbacks() relied on with_repeat_trans_intr() implicitly resetting the transaction before every retry attempt. That behavior is policy, not retry mechanics, so it should live with the caller. Signed-off-by: Matan Breizman --- src/crimson/os/seastore/seastore.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc index b7db30592dd..bc3744b15d0 100644 --- a/src/crimson/os/seastore/seastore.cc +++ b/src/crimson/os/seastore/seastore.cc @@ -1652,6 +1652,10 @@ seastar::future<> SeaStore::Shard::do_transaction_no_callbacks( ++(shard_stats.processing_inlock_io_num); co_await with_repeat_trans_intr( + [&, this] { + // Preserve handle and rewind the external iterator before each attempt. + ctx.reset_preserve_handle(*transaction_manager); + }, *ctx.transaction, seastar::coroutine::lambda([&ctx, this, FNAME](auto &t) -> tm_ret { @@ -1674,7 +1678,6 @@ seastar::future<> SeaStore::Shard::do_transaction_no_callbacks( ctx.iter.colls.size(), ctx.iter.objects.size()); - ctx.reset_preserve_handle(*transaction_manager); std::vector onodes(ctx.iter.objects.size()); // Get the total number of operations from the transaction From deb815c4b990faef82126798b9ea80ea71246bcf Mon Sep 17 00:00:00 2001 From: Matan Breizman Date: Wed, 3 Jun 2026 10:08:57 +0000 Subject: [PATCH 3/3] crimson/os/seastore/cache: avoid double increments reseting a txn doesnt really create a new one semantically. avoid incrementing "created" on reset, otherwise we end up with inflated numbers where MUTATE txn created count is twice as higher than committed. Note, "resets" are already tracked as invalidated. Signed-off-by: Matan Breizman --- src/crimson/os/seastore/cache.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index e6ebb2a7267..ce1d38607eb 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -141,7 +141,6 @@ public: LOG_PREFIX(Cache::reset_transaction_preserve_handle); if (t.did_reset()) { SUBDEBUGT(seastore_t, "reset", t); - ++(get_by_src(stats.trans_created_by_src, t.get_src())); } t.reset_preserve_handle(); }