roots: hold the extent crawler below live temp file transids

A temp file's writes create ordinary btrfs extents, so an extent scan
picks them up like anything else once a temp file outlives a
transaction boundary and reaches the scan queue.  Skipping their refs
during block map construction stops bees hashing recycled data, but it
does not address the earlier problem: those extents are still being
built.  Refs are added to them as the dedupes consuming the temp file
complete, so an extent scanned mid-construction has an incomplete ref
list.  It fails as a dedupe dst, and offers an incomplete src ref map
to whatever does match it.

loop.transid-age-min can paper over this by making every extent wait a
fixed number of transids before it is eligible, but that is a blunt
instrument: it delays every extent on the filesystem to avoid a
condition that applies to a handful, and the operator has to guess a
number that is large enough.

Connect the two directly instead.  A temp file samples transid_max() at
its first write after a reset -- the earliest transaction its extents
can belong to -- and publishes it to the context, keyed by BeesFileId
like the blacklist it already registers in.  effective_transid_max()
holds every crawler strictly below the earliest such transid.  The
entry is dropped in reset(), which the pool's checkin handler calls as
soon as the borrowing planner is done, so the bound applies only while
a temp file actually holds data and costs nothing otherwise.

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

diff --git a/src/bees-context.cc b/src/bees-context.cc
index 505da12aee02287b14701082c36683eadbcd5c58..62686099cb13f92237869952854e9216a974ef21 100644 (file)
--- a/src/bees-context.cc
+++ b/src/bees-context.cc
@@ -1753,6 +1753,37 @@ BeesContext::blacklist_insert(const BeesFileId &fid)
        m_blacklist.insert(fid);
 }
 
+void
+BeesContext::temp_transid_insert(const BeesFileId &fid, const uint64_t transid)
+{
+       unique_lock<mutex> lock(m_temp_transid_mutex);
+       // First write since reset wins: later writes join the same or a later
+       // transaction, so the earliest sample bounds every extent this temp
+       // file can still be creating.
+       m_temp_transid.emplace(fid, transid);
+}
+
+void
+BeesContext::temp_transid_erase(const BeesFileId &fid)
+{
+       unique_lock<mutex> lock(m_temp_transid_mutex);
+       m_temp_transid.erase(fid);
+}
+
+uint64_t
+BeesContext::temp_transid_min() const
+{
+       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) {
+                       rv = transid;
+               }
+       }
+       return rv;
+}
+
 void
 BeesContext::blacklist_erase(const BeesFileId &fid)
 {

diff --git a/src/bees-roots.cc b/src/bees-roots.cc
index 5e92173f71e3360eb47e5293f2f1da496c935cf4..adb3ae137d2395a06cfe4911ec1ed4a61e132fd0 100644 (file)
--- a/src/bees-roots.cc
+++ b/src/bees-roots.cc
@@ -1831,7 +1831,28 @@ uint64_t
 BeesRoots::effective_transid_max()
 {
        const auto raw = transid_max();
-       return raw > m_min_transid_age ? raw - m_min_transid_age : 0;
+       auto rv = raw > m_min_transid_age ? raw - m_min_transid_age : 0;
+
+       // Hold the crawler below any transid in which a live temp file is
+       // still creating extents.  Those extents are ordinary btrfs extents
+       // and the scan would pick them up like any other, but they are only
+       // half-built: refs are still being added as the dedupes that consume
+       // the temp file complete.  An extent scanned mid-construction has an
+       // incomplete ref list, so it fails as a dedupe dst and contributes an
+       // incomplete src ref map.  loop.transid-age-min can paper over this
+       // by making every extent wait a fixed number of transids; this is the
+       // exact bound, and costs nothing once the temp files are reset.
+       const auto temp_floor = m_ctx->temp_transid_min();
+       if (temp_floor) {
+               // Strictly below: the crawl window must not include the transid
+               // the temp file's extents can belong to.
+               const auto limit = temp_floor - 1;
+               if (limit < rv) {
+                       rv = limit;
+                       BEESCOUNT(crawl_temp_transid_hold);
+               }
+       }
+       return rv;
 }
 
 bool

diff --git a/src/bees-tempfile.cc b/src/bees-tempfile.cc
index 3117b82793b97f57626e23144eaaade08e5b0b09..ca130c656c49a9cb4b7c551f689092433482f0bc 100644 (file)
--- a/src/bees-tempfile.cc
+++ b/src/bees-tempfile.cc
@@ -72,11 +72,31 @@ BeesTempFile::enter_mode(const Mode want)
        THROW_CHECK2(runtime_error, static_cast<int>(m_mode), static_cast<int>(want), m_mode == want);
 }
 
+void
+BeesTempFile::note_data_transid()
+{
+       if (m_data_transid) {
+               return;
+       }
+       // Extents this temp file is about to create belong to the running
+       // transaction or a later one, so the transid sampled at the first
+       // write bounds them from below.  The extent crawler must not advance
+       // into that range: those extents are still acquiring refs, and an
+       // extent scanned mid-construction has an incomplete ref list -- it
+       // fails as a dedupe dst and offers an incomplete src ref map.
+       m_data_transid = m_ctx->roots()->transid_max();
+       m_ctx->temp_transid_insert(BeesFileId(m_fd), m_data_transid);
+}
+
 void
 BeesTempFile::reset()
 {
        // A reset begins a new mutation cycle: either family may be used next.
        m_mode = Mode::none;
+       // The old contents are gone, so this file no longer holds back the
+       // extent crawler.  See note_data_transid().
+       m_ctx->temp_transid_erase(BeesFileId(m_fd));
+       m_data_transid = 0;
        // Always leave first block empty to avoid creating a file with an inline extent
        resize(reserved());
 }
@@ -294,6 +314,8 @@ BeesTempFile::make_copy(const BeesFileRange &src)
 
        THROW_CHECK1(invalid_argument, src, src.size() > 0);
 
+       note_data_transid();
+
        // FIEMAP used to give us garbage data, e.g. distinct adjacent
        // extents merged into a single entry in the FIEMAP output.
        // FIEMAP didn't stop giving us garbage data, we just stopped
@@ -351,6 +373,7 @@ BeesTempFile::append(const void *const buf, const size_t len)
        BEESNOTE("appending " << len << " bytes to " << name_fd(m_fd) << " at " << to_hex(m_end_offset));
        BEESTRACE("appending " << len << " bytes to " << name_fd(m_fd) << " at " << to_hex(m_end_offset));
 
+       note_data_transid();
        pwrite_or_die(m_fd, buf, len, m_end_offset);
        m_end_offset += len;
        BEESCOUNT(tmp_append);

diff --git a/src/bees-tempfile.h b/src/bees-tempfile.h
index 65bd4a6bb43240fea2c542c6b0a792d5d18fb7df..1d02fddc8c73e821d3a35ec27a1cbe965ec5076c 100644 (file)
--- a/src/bees-tempfile.h
+++ b/src/bees-tempfile.h
@@ -83,6 +83,14 @@ class BeesTempFile {
        /// Extend or truncate the file to @p new_end_offset bytes via ftruncate().
        void resize(off_t new_end_offset);
 
+       /// Transid sampled at the first write since the last reset, or 0
+       /// when this file holds no data.  Published to the context so the
+       /// extent crawler can stay below it.
+       uint64_t                m_data_transid = 0;
+
+       /// Sample and publish m_data_transid if not already set.
+       void note_data_transid();
+
        /**
         * Mutation discipline used since the last reset().  A TempFile may be
         * driven by exactly one family of methods between resets: @c modern

diff --git a/src/bees.h b/src/bees.h
index e9db2009ee67af66e91ff07c096ca511c2f79d12..7a0a59efe3bc29b82aec7e1d251fb3fca09aaef0 100644 (file)
--- a/src/bees.h
+++ b/src/bees.h
@@ -1587,6 +1587,10 @@ private:
 
        mutable mutex                                   m_blacklist_mutex;
        set<BeesFileId>                                 m_blacklist;  ///< Files excluded from deduplication.
+       mutable mutex                                   m_temp_transid_mutex;
+       /// Earliest transid at which each live temp file materialized data.
+       /// Empty when no temp file currently holds data.
+       map<BeesFileId, uint64_t>                       m_temp_transid;
 
        /// Timer recording total daemon uptime (used in status output).
        Timer                                           m_total_timer;
@@ -1791,6 +1795,15 @@ public:
        /// Return true if @p fid is on the blacklist.
        bool is_blacklisted(const BeesFileId &fid) const;
 
+       /// Record that temp file @p fid materialized data at @p transid.
+       /// Idempotent: the first transid recorded since the last reset wins,
+       /// because that is the earliest transaction its extents can belong to.
+       void temp_transid_insert(const BeesFileId &fid, uint64_t transid);
+       /// 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;
+
        /// Return the per-inode Exclusion mutex for @p inode (creates if absent).
        shared_ptr<Exclusion> get_inode_mutex(uint64_t inode);