config: cap rewrite.read-size at the maximum data extent size

rewrite.read-size sets the buffer for source-side reads in the
materialize copy loop and the block-map hashing pass.  It was validated
only for being a positive multiple of the sectorsize, so nothing stopped
an operator from asking for a buffer larger than any transfer that could
usefully be made with it.

btrfs will not create a data extent larger than 128 MiB, so a copy
buffer bigger than that cannot produce one larger extent: the write is
split into 128 MiB extents regardless, and the surplus only holds dirty
memory, once per worker thread.  Preallocated extents can reach 256 MiB
(BLOCK_SIZE_MAX_EXTENT, for the fallocate limit introduced by kernel
commit 24542bf7ea5e4fdfdb5157ff544c093fa4dcb536), but a prealloc extent
cannot be replaced by a single write either, so that larger number is
not a useful buffer size.

Add BLOCK_SIZE_MAX_DATA_EXTENT for the 128 MiB figure, reject anything
above it at config load next to the existing alignment check, state the
maximum in the built-in config comment, and explain in docs/config-file.md
why the useful ceiling is the smaller of the two extent limits.

The cap also keeps the copy loop's writes under BLOCK_SIZE_MAX_TRANSFER,
which pwrite_or_die() enforces.

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

diff --git a/docs/config-file.md b/docs/config-file.md
index 6a707a30014a895d5368c8e4f71a0027291d5d20..43bd194690c96f0835695af4ffa8394c7fd1fb8d 100644 (file)
--- a/docs/config-file.md
+++ b/docs/config-file.md
@@ -397,6 +397,14 @@ removed.
     the partial block when its bytes are pwritten to the temp file,
     which is wasteful enough to be worth rejecting at config load
     rather than diagnosing in production.
+  * Maximum `128M`.  btrfs will not create a data extent larger than
+    128 MiB, so a write bigger than that cannot produce a single
+    larger extent — it is split into 128 MiB extents regardless, and
+    the extra buffer only holds more dirty memory, once per worker
+    thread.  Preallocated extents can reach 256 MiB, but a prealloc
+    extent cannot be replaced by one write either, so 256 MiB is not
+    a useful size here.  Values above the maximum are rejected at
+    config load.
 
 * **`restart-max`**
   Maximum hash-conflict restarts per extent.  Hash-conflict restarts

diff --git a/src/bees-config-v2.cc b/src/bees-config-v2.cc
index 891612008c45c49d96a26a7b36b93fc09f8514d4..7c8521c91f6491b8e8466adbe1a10d211d4e32e8 100644 (file)
--- a/src/bees-config-v2.cc
+++ b/src/bees-config-v2.cc
@@ -255,7 +255,8 @@ static const char bees_config_v2[] = R"--v2-config--(
 
         # Buffer size for source-side reads in the materialize copy
         # loop and the block-map hashing pass.  Must be a positive
-        # multiple of the filesystem sectorsize.
+        # multiple of the filesystem sectorsize.  Maximum 128M, the
+        # largest extent a single write can replace.
         # Size value (e.g. 128K, 1M, 4M).
         # See docs/config-file.md for sizing tradeoffs.
         read-size = 1M

diff --git a/src/bees-config.cc b/src/bees-config.cc
index 6a453f730d45757d63061cab1160cd3056973bfe..7b7a148d7c6fcc89a1224a046491be831726f53c 100644 (file)
--- a/src/bees-config.cc
+++ b/src/bees-config.cc
@@ -510,6 +510,18 @@ BeesConfig::load_rewrite_policy()
                                << " is not a positive multiple of filesystem sectorsize "
                                << BeesContext::s_sectorsize);
                }
+               // Cap the buffer at the largest extent a single write can
+               // replace.  Beyond that the copy loop cannot produce a bigger
+               // extent, so the extra bytes buy nothing and cost RSS in every
+               // worker thread at once.  The cap is also what keeps the copy
+               // loop's pwrite below BLOCK_SIZE_MAX_TRANSFER, which
+               // pwrite_or_die() enforces.
+               if (rv.m_read_size > ranged_cast<uint64_t>(BLOCK_SIZE_MAX_DATA_EXTENT)) {
+                       THROW_ERROR(invalid_argument,
+                               "rewrite.read-size: " << pretty(rv.m_read_size)
+                               << " exceeds the maximum " << pretty(BLOCK_SIZE_MAX_DATA_EXTENT)
+                               << ", the largest extent a single write can replace");
+               }
                BEESLOGINFO("rewrite.read-size = " << pretty(rv.m_read_size) << " [rewrite.read-size]");
        }
        m_rewrite_policy = rv;

diff --git a/src/bees.h b/src/bees.h
index 49756f39714319ee9ce189980e3a1b55f196a85d..e9db2009ee67af66e91ff07c096ca511c2f79d12 100644 (file)
--- a/src/bees.h
+++ b/src/bees.h
@@ -71,6 +71,14 @@ const off_t BLOCK_SIZE_MAX_COMPRESSED_EXTENT = 128 * 1024;
 /// where preallocated extents may be 256 MiB instead of the normal 128 MiB limit.
 const off_t BLOCK_SIZE_MAX_EXTENT = 256 * 1024 * 1024;
 
+/// Largest data extent btrfs will create, and so the largest write that can
+/// replace an extent in one operation (128 MiB).  Buffering more than this
+/// for a copy cannot produce a larger extent, it only holds more dirty
+/// memory.  Note this is smaller than @ref BLOCK_SIZE_MAX_EXTENT: a
+/// preallocated extent may be 256 MiB, but it cannot be replaced by a single
+/// write, so 256 MiB is not a useful copy-buffer size either.
+const off_t BLOCK_SIZE_MAX_DATA_EXTENT = 128 * 1024 * 1024;
+
 /// @}
 
 /// @name Block alignment masks