crimson/os/seastore: don't let a later rewrite reset force_rewrite_conflict

rewrite_logical_extent() sets the flag with a plain assignment:

    t.force_rewrite_conflict = (extents.size() > 1);

The flag marks remaps that produced multiple extents, and with them lba
insertions the no-conflict publish path cannot handle. But the trimmer
and the cleaner rewrite a batch of extents in one transaction (the
do_for_each loops in async_cleaner.cc), so each extent overwrites what
the previous one set: extent A remaps into three pieces and sets the
flag, extent B remaps into one and clears it again.
should_use_no_conflict_publish() then routes a transaction with
structural btree changes through the publish-to-prior path the comment
above this line rules out, and that path rebases concurrently built
transactions incompletely, leaving them with stale mappings.

Make the flag sticky with |=. It is only cleared on transaction reset,
which is the intended lifetime.

Introduced by 02e511c5d5d; the batch callers predate it, so the reset has been
reachable from the start.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 10244545a2fcfe2695c525d78f938ac3763a4318..c36d1b7dbc93da2a86c5af0be39b8a7d5e4a9af4 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -785,8 +785,9 @@ TransactionManager::rewrite_logical_extent(
     // the LBA update likely involves insertions/splits (structural
     // btree changes), which the no-conflict publish-to-prior path
     // does not currently cover safely. Fall back to optimistic
-    // conflict handling.
-    t.force_rewrite_conflict = (extents.size() > 1);
+    // conflict handling. a batch transaction rewrites many extents,
+    // and any structural rewrite taints them all.
+    t.force_rewrite_conflict |= (extents.size() > 1);
     for (auto &_nextent : extents) {
       auto nextent = _nextent->template cast<LogicalChildNode>();
       bool first_extent = (off == 0);