create: rebuild the files cache from an archive of the same group - #10292
create: rebuild the files cache from an archive of the same group#10292ThomasWaldmann wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10292 +/- ##
==========================================
+ Coverage 87.55% 87.64% +0.09%
==========================================
Files 103 103
Lines 18676 18702 +26
Branches 2872 2877 +5
==========================================
+ Hits 16352 16392 +40
+ Misses 1622 1609 -13
+ Partials 702 701 -1 ☔ View full report in Codecov by Harness. |
8184b5d to
6fa4d5c
Compare
|
Rebased onto the updated #10291 (defaultdict + |
When the local files cache is missing, borg rebuilds it by reading the archive
this one continues from the repository. That archive was looked up by matching
the series name only:
archives = self.manifest.archives.list(match=[self.archive_name], ...)
Archive series names are not unique across hosts, so in a repository shared by
multiple machines or users this could pick a foreign archive: if host2 backed up
its own "home" series after host1, host1 would rebuild its files cache from
host2's archive. Almost nothing matches there, so borg reads and chunks
everything again - the files cache silently stops working for everyone but the
host that happened to write last.
The lookup now matches the archive attributes given by the new --group-by
option, defaulting to name,host. Valid keys are name, host and user; tags are
not usable because a new archive is not known to belong to the tag group of an
existing one, and an empty value is rejected because an archive must not
continue an arbitrary unrelated archive.
The host and user an archive gets stamped with now come from
archive_hostname() / archive_username() in helpers, so the metadata written by
create and the lookup done by the cache can not drift apart.
Note that the local files cache file name is still derived from the series name
alone. It lives on the client, so it is per host already, and keeping the name
avoids invalidating everybody's files cache.
6fa4d5c to
c71b90d
Compare
_build_files_cache read item.ctime unconditionally, but an archive item does not necessarily have a ctime: - borg create --noctime omits it, - on Windows it is never archived (st_ctime is the file creation time there and gets archived as birthtime, see borgbackup#8730), - very old archives only have mtime, as the comment in stat_attrs() says. Rebuilding the files cache from such an archive crashed: AttributeError: attribute ctime not found That is reachable whenever the local files cache is missing while a previous archive exists, i.e. after the cache directory was lost, or on a fresh machine - always on Windows, and with --noctime everywhere. Both timestamps are read with item.get() now. A timestamp the archive does not have is cached as 0, which can not compare equal to the timestamp seen in the file system, so the file is considered changed and gets chunked again if that timestamp is part of the files cache mode. The tracking of the newest ctime/mtime skips missing values instead of comparing them.
|
Both failures were the new tests, with:
if not self.noctime and not is_win32:
# win32: st_ctime is the file creation time, that is archived as birthtime, see #8730.
attrs["ctime"] = safe_ns(st.st_ctime_ns)and the comment right above it even says borg "can work with archives only having mtime". So rebuilding the files cache from a previous archive crashes with It reproduces on Linux/macOS too, which is how I confirmed it is not Windows-specific:
The regression test uses Happy to split that commit into its own PR if you would rather have the fix separate — I kept it here Full test suite locally: 2928 passed, 981 skipped. |
Rebased onto current master now that #10291 has merged, so this is a single commit again.
The bug
When the local files cache is missing — a fresh machine, a cleared cache dir — borg rebuilds it by
reading the archive this one continues from the repository. That archive was looked up by matching
the series name only:
Archive series names are not unique across hosts. In a repository shared by several machines or
users, this picks whichever archive of that name was written last, no matter by whom: if host2 backs
up its own
homeseries after host1, host1 rebuilds its files cache from host2's archive.Almost nothing matches there, so borg reads and chunks everything again. Nothing errors — the files
cache just silently stops doing its job for every host except the one that happened to write last.
Same root cause as #10288 and #10291: the series name alone is not an identity in a shared
repository.
The change
The lookup now matches the archive attributes given by a new
create --group-byoption, defaultname,host— the same defaultprune --group-byuses, so both commands agree on what an archive'sgroup is:
Valid keys are
name,hostanduser— a deliberately smaller set than prune's:tagsis excluded because a new archive is not known to belong to the tag group of an existingone, and
-a tags:is a superset match rather than equality, so it would not express a group.arbitrary unrelated archive", which is the bug generalized.
--group-by namegives the oldbehaviour if someone wants it (e.g. several hosts deliberately backing up the same files under one
series name).
--group-by name,host,usercovers one host backing up the same series as different users.Keeping the metadata and the lookup in sync
The host / user an archive is stamped with came from an expression open-coded in
archive.py; thecache now needs the same values, and a lookup that disagreed with what create writes would silently
never match. Both now go through
archive_hostname()/archive_username()inhelpers/misc.py,so they cannot drift apart. This also honours
BORG_HOSTNAME/BORG_USERNAMEconsistently on bothsides.
Unrelated but noticed while doing this:
{user}as an archive-name placeholder usesplatform.getosusername()(uid → name) while the archive'susernamemetadata usesgetpass.getuser()(env-based). These can disagree, e.g. under sudo. Left alone here since changingit would change generated archive names, but it may be worth a look.
Compatibility
The local files cache file name is still derived from the series name alone. It lives on the client
and is therefore per host already, so it needs no host in its name — and keeping it avoids
invalidating everybody's files cache on upgrade.
Behaviour change: on a host whose hostname is not stable (containers with a random hostname each
run), the rebuild will now find no archive and start from an empty files cache instead of rebuilding
from a foreign one. That is the correct outcome — the foreign rebuild was near-useless work — but it
is worth knowing. The epilog warns about it, and this only affects the path where the local files
cache is missing.
Tests
test_files_cache_rebuild_ignores_other_hostsis the repro: host1 backs uphome, then host2 backsup its own
home, then host1 loses its local files cache and backs up again. It asserts that thedebug log names host1's archive as the rebuild source. Verified that it fails without the fix
(both
archiverandremote_archiver) and passes with it.Also:
test_files_cache_rebuild_group_by_name_only(opting back into the old behaviour),test_files_cache_rebuild_group_by_invalid, and unit tests forarchive_group_patternsandFilesCacheGroupBySpec(rejectedtags, rejected empty, idempotency).Full test suite: 2926 passed, 980 skipped.
ruff checkclean.