mirror of
https://github.com/borgbackup/borg.git
synced 2026-10-12 02:11:36 +00:00
fix: validate BORG_PACK_MAX_SIZE and BORG_PACK_MAX_COUNT, fixes #10485
Repository.__init__ now parses and validates both variables, so a bad value fails before the store is created, opened or locked. Each must be a positive integer, and BORG_PACK_MAX_SIZE must be below the new MAX_PACK_SIZE_LIMIT of 2126512128 (2 GiB - 20 MiB). PackWriter.add keeps the object that crosses the cap, so packs stay below 2 GiB, clear of OS bugs with files of 2 GiB or more. A rejected value raises Error with the variable name and the value. With only BORG_PACK_MAX_COUNT set, the writer still gets MAX_PACK_SIZE_LIMIT - 1 as its byte ceiling. pack_max_size returns the user's size, or DEFAULT_PACK_MAX_SIZE when that variable is unset, so compaction keeps targeting that value. The environment-variable help and the packs internals page state the limit, and repository tests cover rejected values (also on repository creation), the size one below the limit, count-only mode, the unset defaults, and both variables set together. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_019wSaUMZrtm6XSzwEcxavkv
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
9bce0b55e2
commit
52aee23df1
@@ -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.
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -87,6 +87,11 @@ MIN_PACK_SIZE = DEFAULT_PACK_MAX_SIZE // 50 # 1 MB
|
||||
# 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.
|
||||
|
||||
+41
-12
@@ -984,6 +984,40 @@ class Repository:
|
||||
# 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.
|
||||
# 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")
|
||||
max_size_env = os.environ.get("BORG_PACK_MAX_SIZE")
|
||||
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 +1358,21 @@ class Repository:
|
||||
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):
|
||||
|
||||
@@ -16,7 +16,7 @@ from ..crypto.key import store_hash
|
||||
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,78 @@ class FailingPackStore:
|
||||
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_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
|
||||
|
||||
Reference in New Issue
Block a user