extent-layer: report temp file refs instead of quietly skipping them

Two corrections to the temp file handling.

A ref into a temp file reaching block_map_fetch() is not a routine
condition to be counted and dropped.  The crawler is held below live
temp file transids precisely so this cannot happen, so an occurrence
means that bound failed and bees is looking at an extent it should
never have reached.  Log it with the full extent dump.  Skipping is
still the right action -- the data is not ours to hash -- but it is now
impossible to miss, and the dump identifies the extent that got
through.  Left as a warning rather than a throw only because a real
extent legitimately carries a temp file ref between materialization
and the reset that drops it; if this proves quiet in practice it should
become an exception.

temp_transid_min() must never return zero.  An empty registry does not
mean there is nothing to avoid: the scan is running now, and a planner
can materialize a temp file before the next transid bump, creating
extents in the current transaction.  Returning zero lifted the bound
entirely and allowed the crawler into the transaction that created
them, which is the case the bound exists to prevent.  Floor it at the
current transid, so with nothing registered the crawler still stays
strictly below the running transaction, and a registered temp file only
lowers it further.

Assisted-by: Claude-Code:claude-opus-5

diff --git a/src/bees-context.cc b/src/bees-context.cc
index 62686099cb13f92237869952854e9216a974ef21..7b8123cb46c58c18480c0fcac3d2e0c7357cdafb 100644 (file)
--- a/src/bees-context.cc
+++ b/src/bees-context.cc
@@ -1771,13 +1771,21 @@ BeesContext::temp_transid_erase(const BeesFileId &fid)
 }
 
 uint64_t
-BeesContext::temp_transid_min() const
+BeesContext::temp_transid_min()
 {
+       // Never zero.  An empty map does not mean "no temp extents to avoid":
+       // the scan is running now, and a planner can materialize a temp file
+       // before the next transid bump, creating extents in the current
+       // transaction.  Returning 0 would lift the crawler's bound and let it
+       // scan those extents in the transaction that created them -- exactly
+       // the case this exists to prevent.  With nothing registered the floor
+       // is the current transid, which holds the crawler strictly below the
+       // running transaction; a registered temp file only lowers it further.
+       uint64_t rv = roots()->transid_max();
        unique_lock<mutex> lock(m_temp_transid_mutex);
-       uint64_t rv = 0;
        for (const auto &[fid, transid] : m_temp_transid) {
                (void)fid;
-               if (!rv || transid < rv) {
+               if (transid < rv) {
                        rv = transid;
                }
        }

diff --git a/src/bees-extent-layer.cc b/src/bees-extent-layer.cc
index 90c10dcbaa877fa039efaebc0efd26ca7d3c282e..aed8fc41a7e2c2a4c7b99af491d527665b42c4aa 100644 (file)
--- a/src/bees-extent-layer.cc
+++ b/src/bees-extent-layer.cc
@@ -656,7 +656,19 @@ BeesBtrfsExtentLayer::block_map_fetch(BeesBtrfsExtentNode &node,
                // blacklist, so skip those refs instead of hashing them: they
                // contribute no data coverage and pass 2 never preads them.
                if (m_ctx.is_blacklisted(BeesFileId(ref.root(), ref.inum()))) {
+                       // Reaching here means the transid floor did not keep the
+                       // crawler off this extent, so say so loudly with the whole
+                       // extent rather than incrementing a counter nobody reads.
+                       // Skipping is the safe action -- the data is not ours to
+                       // hash -- but it is not a normal outcome.
                        BEESCOUNT(block_map_ref_tempfile);
+                       BEESLOGWARN("block_map_fetch: ref into temp file root "
+                               << ref.root() << " inum " << ref.inum()
+                               << " offset " << to_hex(ref.offset())
+                               << " on extent " << to_hex(node.m_bytenr)
+                               << "; skipping, but the crawler should not have "
+                               "reached this extent:\n"
+                               << BeesExtentTreeDump(BeesExtent(node.shared_from_this())));
                        continue;
                }
 

diff --git a/src/bees.h b/src/bees.h
index 7a0a59efe3bc29b82aec7e1d251fb3fca09aaef0..dff60a8344ed85078b6ca9b78011f839490cc314 100644 (file)
--- a/src/bees.h
+++ b/src/bees.h
@@ -1802,7 +1802,7 @@ public:
        /// Forget @p fid's transid; called when the temp file is reset.
        void temp_transid_erase(const BeesFileId &fid);
        /// Earliest transid among live temp files, or 0 when none hold data.
-       uint64_t temp_transid_min() const;
+       uint64_t temp_transid_min();
 
        /// Return the per-inode Exclusion mutex for @p inode (creates if absent).
        shared_ptr<Exclusion> get_inode_mutex(uint64_t inode);