diff --git a/docs/internals/packs.rst b/docs/internals/packs.rst index 381cf2eab8..f0a6d29edc 100644 --- a/docs/internals/packs.rst +++ b/docs/internals/packs.rst @@ -188,7 +188,9 @@ Writing packs (``PackWriter``). When the buffered blobs reach the pack limit, the buffer is stored as one pack. By default the limit is a size of 50 MB (``DEFAULT_PACK_MAX_SIZE``). ``BORG_PACK_MAX_SIZE`` sets the size limit and ``BORG_PACK_MAX_COUNT`` a blob count -limit; with only ``BORG_PACK_MAX_COUNT`` set, packs are bound by count only, see +limit. ``BORG_PACK_MAX_SIZE`` must be below ``MAX_PACK_SIZE_LIMIT``, which is +2126512128 bytes (2 GiB - 20 MiB); with only ``BORG_PACK_MAX_COUNT`` set, packs are +still capped at ``MAX_PACK_SIZE_LIMIT - 1`` (2126512127) bytes, see :ref:`env_vars`. The blob that reaches the limit is part of that pack, so a pack can be larger than the size limit by less than one blob. diff --git a/src/borg/archiver/help_cmd.py b/src/borg/archiver/help_cmd.py index 475e539122..9245f43605 100644 --- a/src/borg/archiver/help_cmd.py +++ b/src/borg/archiver/help_cmd.py @@ -745,14 +745,18 @@ class HelpMixIn: When set to a numeric value, limit the pack cache to that many bytes. Only has an effect if BORG_STORE_CACHE is set. BORG_PACK_MAX_SIZE - When set to a numeric value, cap packs (the repository objects that batch up many + When set to a positive integer, cap packs (the repository objects that batch up many chunks, see the internals documentation about pack files) at that many bytes instead of the default of 50000000. + The value must be below 2126512128 (2 GiB - 20 MiB), which keeps packs + clear of OS bugs with files of 2 GiB or more. A non-integer, a non-positive + value, or a value that reaches that limit is rejected. Smaller packs mean more (but smaller) repository objects and more fine-grained uploads; bigger packs mean fewer objects and fewer stores. BORG_PACK_MAX_COUNT - When set to a numeric value, cap packs at that many objects per pack. - If BORG_PACK_MAX_SIZE is not also set, packs are then bound by count only. + When set to a positive integer, cap packs at that many objects per pack. + A non-integer or non-positive value is rejected. + If BORG_PACK_MAX_SIZE is not set, packs still stay below 2 GiB. BORG_PACK_ASYNC When set to ``no``, disable the background thread that stores a finished pack while the next one is being assembled, and store packs synchronously instead. diff --git a/src/borg/constants.py b/src/borg/constants.py index 0cc22219c6..ffa3d5112d 100644 --- a/src/borg/constants.py +++ b/src/borg/constants.py @@ -87,6 +87,11 @@ # MAX_OBJECT_SIZE = MAX_DATA_SIZE + len(PUT header) MAX_OBJECT_SIZE = MAX_DATA_SIZE + 41 # see assertion at end of repository module +# BORG_PACK_MAX_SIZE must stay below this to keep packs below 2 GiB and avoid OS bugs. +# PackWriter.add includes the object that crosses the cap, so the +# largest accepted size is one less than this limit. +MAX_PACK_SIZE_LIMIT = 2**31 - MAX_OBJECT_SIZE + # Clock skew is the difference between the clocks of the machines writing to a repository (seconds). # A check result timestamp up to this far in the future still counts as recent; further ahead than # this, the pack is re-verified. diff --git a/src/borg/repository.py b/src/borg/repository.py index a58469d498..ccfbf6e59e 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -984,6 +984,41 @@ def __init__( # True if packs are cached locally (BORG_STORE_CACHE): store.load() of a pack may return the cached copy. self.uses_pack_store_cache = cache_url is not None + # pack-sizing overrides: BORG_PACK_MAX_COUNT sets the max object count per pack, + # BORG_PACK_MAX_SIZE the max pack size in bytes. Default: size-bound only. + # They are validated here, so a bad value fails before the store is created, opened or locked. + # An empty value counts as unset, like BORG_PACK_CACHE_SIZE. A non-integer or non-positive + # value is rejected. The size must also stay below + # MAX_PACK_SIZE_LIMIT: add() keeps the object that crosses the cap, so this keeps packs + # below 2 GiB, clear of OS bugs with files of 2 GiB or more. Count-only mode still passes + # that ceiling (minus one) as max_size. pack_max_size remembers the configured size, or the + # default when the user did not set one, so the safety ceiling does not become the compact target. + max_count_env = os.environ.get("BORG_PACK_MAX_COUNT") or None + max_size_env = os.environ.get("BORG_PACK_MAX_SIZE") or None + if max_count_env is None: + max_count = None + else: + try: + max_count = int(max_count_env) + except ValueError: + raise Error(f"BORG_PACK_MAX_COUNT must be an integer, but is: {max_count_env!r}") from None + if max_count <= 0: + raise Error(f"BORG_PACK_MAX_COUNT must be positive, but is: {max_count}") + self._pack_max_count = max_count + if max_size_env is None: + self._pack_max_size = (MAX_PACK_SIZE_LIMIT - 1) if max_count is not None else DEFAULT_PACK_MAX_SIZE + self._configured_pack_max_size = DEFAULT_PACK_MAX_SIZE + else: + try: + max_size = int(max_size_env) + except ValueError: + raise Error(f"BORG_PACK_MAX_SIZE must be an integer, but is: {max_size_env!r}") from None + if max_size <= 0: + raise Error(f"BORG_PACK_MAX_SIZE must be positive, but is: {max_size}") + if max_size >= MAX_PACK_SIZE_LIMIT: + raise Error(f"BORG_PACK_MAX_SIZE must be below {MAX_PACK_SIZE_LIMIT}, but is: {max_size}") + self._pack_max_size = self._configured_pack_max_size = max_size + propagate_rsh() # borgstore shall use the same remote shell command as borg try: @@ -1324,26 +1359,21 @@ def open(self, *, exclusive, lock_wait=None, lock=True): else: self.acquire_lock() self._chunks = None - # pack-sizing overrides: BORG_PACK_MAX_COUNT sets the max object count per pack, - # BORG_PACK_MAX_SIZE the max pack size in bytes. Default: size-bound only. - max_count_env = os.environ.get("BORG_PACK_MAX_COUNT") - max_size_env = os.environ.get("BORG_PACK_MAX_SIZE") - max_count = int(max_count_env) if max_count_env is not None else None - if max_size_env is not None: - max_size = int(max_size_env) - else: - max_size = None if max_count is not None else DEFAULT_PACK_MAX_SIZE # BORG_PACK_ASYNC=no disables the background store-thread (debugging aid, see PackWriter). async_store = os.environ.get("BORG_PACK_ASYNC", "yes") != "no" self._pack_writer = PackWriter( - self.store, repository=self, max_count=max_count, max_size=max_size, async_store=async_store + self.store, + repository=self, + max_count=self._pack_max_count, + max_size=self._pack_max_size, + async_store=async_store, ) self.opened = True @property def pack_max_size(self): - """The configured byte cap for a pack (BORG_PACK_MAX_SIZE, or the default if count-bound).""" - return self._pack_writer.max_size or DEFAULT_PACK_MAX_SIZE + """The configured byte cap for a pack (BORG_PACK_MAX_SIZE, or the default if it was not set).""" + return self._configured_pack_max_size @property def chunks(self): diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index f5bf051792..9bd1706051 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -16,7 +16,7 @@ from .. import repository as repository_module from ..cache import chunkindex_is_invalid, delete_chunkindex_from_repo, write_chunkindex_invalid from ..compress import CNONE -from ..constants import MAX_CLOCK_SKEW, ROBJ_FILE_STREAM +from ..constants import DEFAULT_PACK_MAX_SIZE, MAX_CLOCK_SKEW, MAX_PACK_SIZE_LIMIT, ROBJ_FILE_STREAM from ..crypto.key import AESOCBKey, AuthenticatedKey, Blake3AuthenticatedKey, CHPOKey from ..helpers import Error, IntegrityError, Location, bin_to_hex, hex_to_bin from ..hashindex import ChunkIndex, ChunkIndexEntry @@ -1454,6 +1454,89 @@ def __getattr__(self, name): return getattr(self._inner, name) +@pytest.mark.parametrize("value", ["0", "-1", "abc", str(MAX_PACK_SIZE_LIMIT)]) +def test_borg_pack_max_size_rejected(tmp_path, monkeypatch, value): + with Repository(os.fspath(tmp_path / "repo"), exclusive=True, create=True) as repository: + pass + monkeypatch.setenv("BORG_PACK_MAX_SIZE", value) + with pytest.raises(Error, match="BORG_PACK_MAX_SIZE"): + with reopen(repository): + pass + + +@pytest.mark.parametrize("name", ["BORG_PACK_MAX_SIZE", "BORG_PACK_MAX_COUNT"]) +def test_borg_pack_limits_rejected_before_create(tmp_path, monkeypatch, name): + monkeypatch.setenv(name, "0") + with pytest.raises(Error, match=name): + with Repository(os.fspath(tmp_path / "repo"), exclusive=True, create=True): + pass + assert not (tmp_path / "repo").exists() + + +def test_borg_pack_max_size_just_below_limit(tmp_path, monkeypatch): + with Repository(os.fspath(tmp_path / "repo"), exclusive=True, create=True) as repository: + pass + accepted = MAX_PACK_SIZE_LIMIT - 1 + monkeypatch.setenv("BORG_PACK_MAX_SIZE", str(accepted)) + with reopen(repository) as repository: + assert repository._pack_writer.max_size == accepted + assert repository.pack_max_size == accepted + + +@pytest.mark.parametrize("value", ["0", "-1", "nope"]) +def test_borg_pack_max_count_rejected(tmp_path, monkeypatch, value): + with Repository(os.fspath(tmp_path / "repo"), exclusive=True, create=True) as repository: + pass + monkeypatch.setenv("BORG_PACK_MAX_COUNT", value) + with pytest.raises(Error, match="BORG_PACK_MAX_COUNT"): + with reopen(repository): + pass + + +def test_borg_pack_max_count_only_keeps_size_ceiling(tmp_path, monkeypatch): + with Repository(os.fspath(tmp_path / "repo"), exclusive=True, create=True) as repository: + pass + monkeypatch.delenv("BORG_PACK_MAX_SIZE", raising=False) + monkeypatch.setenv("BORG_PACK_MAX_COUNT", "4") + with reopen(repository) as repository: + assert repository._pack_writer.max_count == 4 + assert repository._pack_writer.max_size == MAX_PACK_SIZE_LIMIT - 1 + assert repository.pack_max_size == DEFAULT_PACK_MAX_SIZE + + +def test_borg_pack_limits_default_when_unset(tmp_path, monkeypatch): + with Repository(os.fspath(tmp_path / "repo"), exclusive=True, create=True) as repository: + pass + monkeypatch.delenv("BORG_PACK_MAX_COUNT", raising=False) + monkeypatch.delenv("BORG_PACK_MAX_SIZE", raising=False) + with reopen(repository) as repository: + assert repository._pack_writer.max_count is None + assert repository._pack_writer.max_size == DEFAULT_PACK_MAX_SIZE + assert repository.pack_max_size == DEFAULT_PACK_MAX_SIZE + + +def test_borg_pack_limits_empty_means_unset(tmp_path, monkeypatch): + with Repository(os.fspath(tmp_path / "repo"), exclusive=True, create=True) as repository: + pass + monkeypatch.setenv("BORG_PACK_MAX_COUNT", "") + monkeypatch.setenv("BORG_PACK_MAX_SIZE", "") + with reopen(repository) as repository: + assert repository._pack_writer.max_count is None + assert repository._pack_writer.max_size == DEFAULT_PACK_MAX_SIZE + assert repository.pack_max_size == DEFAULT_PACK_MAX_SIZE + + +def test_borg_pack_count_and_size_both_passed_through(tmp_path, monkeypatch): + with Repository(os.fspath(tmp_path / "repo"), exclusive=True, create=True) as repository: + pass + monkeypatch.setenv("BORG_PACK_MAX_COUNT", "5") + monkeypatch.setenv("BORG_PACK_MAX_SIZE", "100000") + with reopen(repository) as repository: + assert repository._pack_writer.max_count == 5 + assert repository._pack_writer.max_size == 100000 + assert repository.pack_max_size == 100000 + + def test_pack_writer_returns_none_when_not_full(): pw = PackWriter(MockStore(), max_count=2, chunks=ChunkIndex()) assert pw.add(b"a" * 32, b"data") is None