The object database files backend has been updated to reload
alternates when an object lookup fails on retry. This restores a lost
capability to find objects moved to a newly added alternate during a
concurrent repository rewrite, with caching and deduplication
optimizations to avoid performance regressions.
* ps/odb-reload-alternates:
odb/source-files: only reload alternates when the file has changed
odb/source-files: avoid stat'ing already-known alternates
odb/source-files: simplify directory duplication detection
odb/source-files: reload alternates on retry
odb/source-files: always load alternates via primary object directory
odb/source-files: rename alternates-related functions
The object storage format has been made configurable via the
'extensions.objectStorage' configuration variable, allowing users
to specify the storage backend and an optional payload, mirroring
the design of 'extensions.refStorage'.
* ps/odb-object-storage-extension:
builtin/submodule: propagate "--object-storage-format="
builtin/clone: introduce "--object-storage-format="
builtin/init: introduce handling of object storage format
builtin: expose the object storage format
setup: allow overriding object storage format via environment
setup: introduce object storage extension
loose: drop unused `repo_write_loose_object_map()`
Introduce a new "extensions.objectStorage" extension that allows the
user to configure the primary object database source of a repository.
The value is a URI in the format "<schema>[://<payload>]", mirroring
the format used by "extensions.refStorage":
- The schema identifies the backend that shall be used. For now the
only supported schema is "files", which is the existing object
database backed by loose objects and packfiles.
- The payload can be passed to configure the backend, and its
interpretation is backend-specific. For the "files" backend, it
allows you to configure a different location for the ".git/objects"
directory.
Making the location of the "files" backend configurable is not the most
thrilling feature of this new extension. But it gives us a couple of
advantages:
- It allows us to start exercising the payload logic in our tests so
that we know it's working properly for backends that will need it.
- It allows us to easily detect tests that are broken with the new
extension because they for example assume that an object is a file
in ".git/objects/". Testing is not part of this series yet, but will
be done in subsequent patch series.
- It aligns with how we handle the ref storage extension, where we can
also change its location to an arbitrary path.
- It allows for some very esoteric and edge-case-y gimmicks. For
example, you can initialize a single repository "skeleton" and then
multiplex therein by setting both GIT_REF_STORAGE_FORMAT and
GIT_OBJECT_STORAGE_FORMAT to point to different databases. These
edge cases won't matter to many people out there, but we will use
this for example as a building block for migrations.
Eventually, this mechanism will be used to support alternative backends.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a preceding commit we have fixed a regression where we didn't re-read
the alternates file anymore when we hit a missing object. But while
correct, this change significantly hampers performance, even though
we're roughly at the same level compared to before the regression.
But the checks are unnecessarily expensive, as we always read and parse
the alternates file even in the case where it has not changed. This of
course translates into a lot of busywork for the common case, where the
"objects/info/alternates" is still the exact same.
Improve this by using a `struct stat_validity`. Like this we can easily
short-circuit reading the alternates file in case we can determine via a
stat(3p) call that the file hasn't changed. This significantly improves
performance:
Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
mean ± σ min … max
Wall Time 288.5 ms ± 2.3 ms 284.9 ms … 293.7 ms
Memory 5.6 MiB ± 0.1 MiB 5.5 MiB … 5.7 MiB
Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
mean ± σ min … max
Wall Time 185.3 ms ± 3.4 ms 181.2 ms … 200.1 ms -35.8% (1.6x faster)
Memory 5.6 MiB ± 0.1 MiB 5.5 MiB … 5.7 MiB -0.1%
Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
mean ± σ min … max
Wall Time 262.7 ms ± 2.7 ms 257.1 ms … 268.0 ms -9.0%
Memory 5.9 MiB ± 0.1 MiB 5.6 MiB … 6.1 MiB +4.5%
Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
mean ± σ min … max
Wall Time 199.5 ms ± 2.5 ms 195.4 ms … 207.8 ms -30.9% (1.4x faster)
Memory 5.9 MiB ± 0.1 MiB 5.6 MiB … 6.0 MiB +4.3%
We're significantly faster compared to the original state now and are
almost as fast as the broken state where we don't reload alternates at
all. This is probably good enough for now.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When adding alternates we first check whether the alternate exists
before checking whether we already know about it. This results in a
useless stat(3p) syscall in the case where we already track the
alternate.
Reorder the logic so that we first check whether we know the alternate
already before checking whether its directory exists to avoid the
syscall. Using the same benchmark as in the preceding commit:
Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
mean ± σ min … max
Wall Time 289.5 ms ± 3.4 ms 285.0 ms … 300.7 ms
Memory 5.6 MiB ± 0.1 MiB 5.5 MiB … 5.7 MiB
Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
mean ± σ min … max
Wall Time 185.3 ms ± 3.5 ms 181.2 ms … 200.8 ms -36.0% (1.6x faster)
Memory 5.7 MiB ± 0.0 MiB 5.5 MiB … 5.7 MiB +0.2%
Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
mean ± σ min … max
Wall Time 269.2 ms ± 4.5 ms 264.9 ms … 290.0 ms -7.0%
Memory 5.9 MiB ± 0.1 MiB 5.6 MiB … 6.1 MiB +4.9%
Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
mean ± σ min … max
Wall Time 260.7 ms ± 3.4 ms 256.8 ms … 271.1 ms -10.0%
Memory 5.9 MiB ± 0.1 MiB 5.8 MiB … 6.1 MiB +4.7%
Note that this causes a user-visible change in behaviour: when an
alternate was already linked into the "files" backend, its directory
disappears afterwards and we later try to reload alternates, then we now
stop printing the error message that the object directory does not
exist. That feels sufficiently niche to not really matter in practice
though, and other than that the user-observable behaviour remains the
exact same.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When loading alternates it may happen that we try to add the same
alternate multiple times, either because "info/alternates" contains
duplicate entries or because one of its alternates matches the primary
object directory. We try to detect such duplicates via two mechanisms:
- We have a hashmap of `struct odb_files_dir`, mapped by their
absolute paths. If the path of an alternate is already present in
this map we skip it.
- We have another, explicit check to check whether the path of an
alternate matches the primary object directory.
We can simplify both of these mechanisms by turning the map into a
strset, as we never access the values of the map. On the one hand this
gives us simpler interfaces to access the set. And on the other hand
this allows us to add two entries for the primary object directory: one
for the real path, and one for the absolute path.
Note that there is one gotcha though: when "core.ignorecase" is set then
the expectation is that we deduplicate alternates case-insensitively.
But we can also make that work rather easily by downcasing all paths we
add in that case.
Despite being significantly simpler, this also gives us a speedup
because we don't have to resolve the realpath of the primary object
directory on every call to `odb_files_dir_is_usable()` anymore. Using
the same benchmark as in the preceding commit:
Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
mean ± σ min … max
Wall Time 289.0 ms ± 2.2 ms 285.8 ms … 295.1 ms
Memory 5.6 MiB ± 0.1 MiB 5.4 MiB … 5.7 MiB
Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
mean ± σ min … max
Wall Time 183.8 ms ± 1.2 ms 181.5 ms … 186.9 ms -36.4% (1.6x faster)
Memory 5.6 MiB ± 0.1 MiB 5.3 MiB … 5.7 MiB -0.5%
Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
mean ± σ min … max
Wall Time 293.2 ms ± 2.7 ms 288.8 ms … 299.7 ms +1.5%
Memory 5.9 MiB ± 0.1 MiB 5.7 MiB … 6.1 MiB +5.0%
Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
mean ± σ min … max
Wall Time 266.5 ms ± 2.8 ms 262.6 ms … 276.7 ms -7.8%
Memory 5.9 MiB ± 0.1 MiB 5.6 MiB … 6.1 MiB +5.1%
Rename `dirs_paths_icase` to `dir_paths_iscase` to match the naming of
the newly added set.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When an object ID cannot be found we retry reading it from our packfiles
after we have flushed the respective caches of the packed backends. But
this misses one scenario: it may happen that the object databases'
alternates get rewritten concurrently and all of its objects are moved
to such an alternate. If so, we won't be able to find any of the objects
anymore. This used to be working just fine, but with 5946a564cd
(odb/source: make `read_object_info()` function pluggable, 2026-03-05)
we have accidentally dropped this logic.
This scenario sounds somewhat unlikely, but we've actually been hitting
it in the wild: when running long-lived git-cat-file(1) processes that
we interactively query over many minutes, and that repository gets
forked concurrently, then we move all objects into an object pool, write
a new alternates file, and the git-cat-file(1) process will remain
broken for the rest of its lifetime.
Fix this bug by re-preparing alternates when the object in question
could not be found and then retrying the read. A couple things to note:
- This requires us to re-read loose objects, which is an operation
that we previously skipped on the retry. But we only need to do this
for the loose object directories that are actually new.
- The result is still incomplete, as we only reload the alternates of
the primary object directory. But alternates are added recursively,
so in theory we'd also have to check whether alternates of any of
the alternates have changed. But we never did that before the
regression, either, so for now we don't care about this edge case.
- It may happen that we reload when a quarantine directory is active.
If no alternates existed before that reload we may not have lazily
populated the map of object directories yet. So the assertion in
`odb_files_dir_is_usable()` does not necessarily hold anymore, and
we now have to populate the map with all object directories.
Furthermore, this change causes a performance regression when asking for
lots of missing objects. But that regression is basically in line with
the performance we had before this mechanism got broken. The following
benchmark demonstrates this when asking for 20k missing commits in an
empty repository with a single empty alternate:
Benchmark 1: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd~) (50 runs)
mean ± σ min … max
Wall Time 289.9 ms ± 3.4 ms 285.8 ms … 307.5 ms
Memory 5.6 MiB ± 0.1 MiB 5.5 MiB … 5.7 MiB
Benchmark 2: ./git -C /tmp/repo cat-file --batch-check (rev = 5946a564cd) (50 runs)
mean ± σ min … max
Wall Time 184.8 ms ± 2.5 ms 181.5 ms … 191.9 ms -36.2% (1.6x faster)
Memory 5.6 MiB ± 0.1 MiB 5.5 MiB … 5.7 MiB -0.1%
Benchmark 3: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD~) (50 runs)
mean ± σ min … max
Wall Time 189.1 ms ± 2.7 ms 185.9 ms … 196.1 ms -34.8% (1.5x faster)
Memory 5.9 MiB ± 0.1 MiB 5.6 MiB … 6.1 MiB +4.9%
Benchmark 4: ./git -C /tmp/repo cat-file --batch-check (rev = HEAD) (50 runs)
mean ± σ min … max
Wall Time 293.2 ms ± 2.6 ms 289.3 ms … 303.2 ms +1.1%
Memory 5.9 MiB ± 0.1 MiB 5.6 MiB … 6.1 MiB +4.4%
There's no helping this though, with the regression fixed we will be
slower compared to before as we have to do more work. But the next
commits will improve the status quo a bit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "files" source tracks a list of object directories. The head of the
list is typically expected to be the primary object directory, whereas
the tail is the list of alternates linked to it. This assumption does
not always hold though, as transactions cause us to prepend another
quarantine object directory to the list.
This is not much of a problem now, but in a subsequent commit we're
about to re-read "objects/info/alternates" when we hit a missing object.
So we may start calling `prepare_alternates()` in situations where the
head of the list is _not_ the primary object directory, but if so we
would now try to read alternates from the quarantine directory and not
from the primary object directory.
Fix this issue by instead using the path of the "files" source both when
reading alternates in `prepare_alternates()` and when deduplicating the
primary object directory in `odb_files_dir_is_usable()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The functions that prepare and add alternates in the "files" backend
were originally part of the object database layer, which explains why
they still have an "odb_" prefix. That prefix does not make much sense
anymore now that those are part of the "files" backend.
Drop the prefix.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `local` flag indicates whether a given `struct odb_source` is a
local source or not, where "local" is defined as being the object
directory that is directly attached to the current repository. But now
that alternates are handled in the "files" backend, this property is not
something that even makes sense anymore to have on the generic source.
In fact, we already have a partial replacement for this field in `struct
odb_files_dir`. The only other site that requires it is the packed
object database source, because it propagates the bit into the packfiles
it is opening.
Drop the flag from `struct odb_source` and instead move it into the
packed source.
Note that we stop considering the `local` bit when iterating through all
loose objects with the `ODB_FOR_EACH_OBJECT_LOCAL_ONLY` flag. This is
fine though, as we already handle the flag in the "files" backend
itself, where we skip over any object directory that is not marked as
local.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When looking up an object fails we perform a second read in all sources
with the `OBJECT_INFO_SECOND_READ` flag set. This flag tells sources
that the first lookup has failed and that they should reload any on-disk
state that may have changed in the meantime so that the object may be
surfaced after all. The "packed" source uses this to reprepare its
packfiles, whereas the "loose" source short-circuits the second read
because there are no caches to flush.
This logic lives in the generic object database layer even though it is
highly specific to how the "files" backend handles its reads. But now
that alternates are an implementation detail of that source, we can
easily move that logic into the backend, as well.
Move the logic into the "files" source and drop the flag.
Note that the "packed" source also used this flag to figure out whether
it should try to surface an object via a packfile that should have the
object as determined by a multi-pack index, but where the packfile has
vanished. The multi-pack index is stale in that case, and that scenario
should really only happen when the process is racing with a concurrent
maintenance task.
This code is adapted to instead use `OBJECT_INFO_QUICK`, which means
that the fallback may trigger on the first read already, but not in a
case where callers know to handle false negatives already. In the worst
case this could cause another scan through all packfiles covered by the
multi-pack index. But that scan should be fast enough anyway, and it
only triggers when we hit the race. So the effect thereof should be
limited.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This patch is essentially the equivalent to the preceding patch, but for
streaming objects instead of just reading them directly.
Exercising performance is a bit more complicated here as streaming only
really kicks in for large objects. Furthermore, I expect that the impact
is not as big as with the last commit, either, because for large objects
the time to stat the file will probably be dwarfed by the time it takes
to read the object itself.
But even though this is probably not as impactful, it makes sense anyway
to do this change to align behaviour across both of these functions.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
With the move towards pluggable object databases we reorganized how we
access alternates. Before the introduction of pluggability, we were
reading objects by first scanning through packfiles from all alternates
before scanning through all loose object directories. With the change
though, we are now iterating through the sources one by one, checking
each of them for whether they contain the object either as a packed or
loose object.
Checking whether an object exists in packfiles is comparatively cheap
because the list of packs is cached, and the pack indices are likely to
be in the page cache. In contrast to that, checking whether an object
exists in loose format always incurs one stat(3p) syscall. So scanning
for objects one alternate at a time led to a performance regression in
in the case where most of the objects are contained in a packfile of an
alternate, as reported in [1].
We could fix that issue by introducing another flag that tells the
backends to only search the optimized storage backend. But that is
adding more complexity to the object database layers that is ultimately
only really required because of the way the "files" backend stores its
data. So for an alternate backend, such new infrastructure would not
really be a good fit.
But since then we have moved handling of alternates into the "files"
backend itself. One of the consequences of this is that this backend is
now having full control over all of its sub-sources, and that also makes
it way easier for us to optimize for this access pattern again.
Do so by first trying to read objects from packfiles located in any of
its object directories before trying to read loose objects.
The following benchmark demonstrates that this significantly improves
performance in some cases. The benchmark uses the Git repository itself
and sets up its object directory as an alternate, while the primary
object directory is simply an empty object directory. It then reads all
commits enumerated via `git rev-list HEAD` with git-cat-file(1):
Benchmark 1: git cat-file --batch-check --unordered (rev = HEAD~)
Time (mean ± σ): 179.4 ms ± 2.6 ms [User: 114.6 ms, System: 61.4 ms]
Range (min … max): 177.1 ms … 187.7 ms 16 runs
Benchmark 2: git cat-file --batch-check --unordered (rev = HEAD)
Time (mean ± σ): 95.0 ms ± 1.5 ms [User: 79.8 ms, System: 15.0 ms]
Range (min … max): 93.0 ms … 97.9 ms 30 runs
Summary
git cat-file --batch-check --unordered (rev = HEAD) ran
1.89 ± 0.04 times faster than git cat-file --batch-check --unordered (rev = HEAD~)
[1]: <CAFXJcxvpKHoVDwE5mBOd=w-A5vPdUmehqr8SHLUD7qv1qB00rA@mail.gmail.com>
Reported-by: Wolfgang Kritzinger <wkritzinger@atlassian.com>
Helped-by: Tyrie Vella <tyrielv@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that alternates are an implementation detail of the "files" backend
there is no reason anymore to expose them anywhere else. The callback to
read alternates is thus no longer required. Remove it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Originally, when designing pluggable object databases the goal was that
the object database can have multiple sources, and every source attached
to it could use a different backend. This would have allowed for quite a
lot of flexibility, as you could trivially mix and match different kinds
of object storages in whatever way you like.
But while well-intentioned, this design led to a bunch of conceptual
problems:
- We're now trying to read objects in source order, whereas we
previously tried to read objects via packfiles before trying to read
them via loose objects. This led to a performance regression when
using alternates or when using a quarantine directory.
- Some data structures are supposed to only ever exist once, like for
example bitmaps and commit graphs. At the same time, those data
structures also span across the union of all objects, so they may
cross sources.
- It is unclear how we can extend GIT_OBJECT_DIRECTORY or
GIT_ALTERNATE_OBJECT_DIRECTORIES to become backend-agnostic in a
backwards-compatible way. In general, introducing an object storage
extension into the current status quo where alternates may have to
be extended to become generic was proving to be painful.
- Some mechanisms of alternates assume way too much about how exactly
their backends work. Alternate refs for example assume that the
alternate is backed by a filesystem path, and that this filesystem
path may also allow us to read references. This is not a given
though, as backends may not even have local data at all.
In short, there are a bunch of conceptual mismatches when we have
alternates and pluggable object databases coexist. So while the original
idea was nice, it does not result in a system that is easy to reason
about.
Correct course by moving alternates into the "files" source itself so
that it becomes an implementation detail thereof so that we can avoid
all of these shortcomings. While it's unfortunate that we cannot easily
mix and match sources now, that ability doesn't go away. It's still very
much feasible to introduce a new backend that allows for exactly that
use case, and such a backend may also be a lot more flexible as we can
now add new logic to determine which objects should be stored where. So
the original motivation for having per-source backends can still be
realized with the new architecture.
Note that as part of this move, we also handle the GIT_OBJECT_DIRECTORY
and GIT_ALTERNATE_OBJECT_DIRECTORIES environment variables in the
"files" backend. This may be surprising at first, but object directories
are very much a concept of that backend, too. So these variables would
have bad interactions with other backends, and they create a bit of a
mismatch with the eventual object storage extension that we plan to
introduce.
Note that this commit is way larger than I'd like it to be. I'm sorry, I
couldn't find a way to split it up further. That being said, most of the
changes are straight-forward conversions that go from iterating over
sources to iterating over object directories. The more involved changes
are moving the infrastructure to track individual sources from "odb.c"
and moving them into "odb/source-files.c".
This also serves to show a bit of a who's-who of commands that don't
work properly with pluggable object databases. Almost all of these
commands are related to housekeeping though -- some of them will be
converted eventually, like for example commit graphs or MIDXs. But many
of them will stay incompatible going forward as they are simply too
specific to the "files" backend.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the next commit we'll add a new callsite that wants to read
alternates without having a proper object database source for a given
alternate available to it. Prepare for this by extracting the logic into
a separate function that only requires an object directory path as
input.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `struct odb_source::will_destroy` flag tracks whether a source is
part of a transaction that we know we'll destruct anyway. If so, the
backend can optimize for that particular case, for example by not
flushing any data to disk.
While the intent is sensible, it assumes that transactions are backed by
a separate source that's being linked into the object database. But that
may or may not be true, as backends may have significantly better ways
to achieve the same. So the assumption doesn't make much sense in the
first place, as we're now tracking backend-specific details on the
generic `struct odb_source` level.
Move the field from the generic source into the "loose" source, as this
is the only source that ever makes use of it anyway.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a subsequent commit we'll migrate handling of alternates into the
"files" backend. As part of that, the backend will need to grow the
ability to handle multiple object directories for alternates and for
transactions.
Introduce a `struct odb_files_dir::next` pointer so that we can have
multiple directories. Adapt the backend to loop through this list as
necessary. In general:
- For reading paths we will loop through all object directories until
we have found the object.
- For writing paths we will typically write to the first of our object
directories.
This mechanism isn't used yet as we still only ever have a single object
directory at the current point in time. But that will change over
subsequent commits.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar as in the preceding commit, `odb_find_source()` can be used to
yield a single source by its object directory. This function is also
specific to the "files" backend once alternates are an implementation
detail thereof.
Refactor it to be specific to the "files" backend and return an `struct
odb_files_dir` to prepare for moving alternates into the "files" source.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "files" object database source consists of two separate backends for
storing loose and packed objects. These are managed as somewhat separate
entities even though they derive from the same object directory, whether
it's the primary object directory or that one from an alternate.
In a subsequent commit we'll move the handling of alternates into the
"files" backend completely so that it becomes another implementation
detail thereof. As part of this, we'll want to keep track of both of
these sub-sources as a single entity derived from their respective
object directory.
Prepare for this change by introducing a new `struct odb_files_dir` that
encapsulates them. For now, every "files" source has exactly one such
directory. In a subsequent commit though, we'll make it a linked list of
directories so that we can manage multiple such directories in a single
"files" source.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The setup of alternates has been deferred to object database
creation time during clone, which drops the unused ad-hoc alternate
writing API, simplifying the object database backend interface.
* ps/odb-alternates-at-creation:
odb/source: remove the ability to write alternates
builtin/clone: write alternates via `odb_create_on_disk()`
odb/source: support writing alternates when creating the database
builtin/clone: move setup of alternates for non-shared local clones
builtin/clone: move setup of alternates for shared local clones
builtin/clone: refactor handling of "--reference{,-if-able}"
builtin/clone: move around `setup_reference()`
builtin/clone: defer setup of the object database
setup: split up concerns of `init_db()`
The consistency checks for the object database (fsck) have been
decoupled from the generic builtin implementation and moved into the
backend-specific object source layers, making them pluggable for
different object storage formats.
* ps/odb-pluggable-fsck:
builtin/fsck: move loose object verification into the loose source
builtin/fsck: move multi-pack index verification into the packed source
builtin/fsck: move bitmap verification into the packed source
builtin/fsck: move reverse index verification into the packed source
builtin/fsck: move packfile verification into the packed source
odb: provide infrastructure for pluggable fsck checks
builtin/fsck: don't check alternates with "--no-full"
builtin/fsck: de-globalize option handling
builtin/fsck: merge `fsck_obj_buffer()` and `fsck_obj()`
builtin/fsck: use `fsck_obj_buffer()` when checking loose objects
The mechanism to register in-memory alternate object sources has
been removed, as submodule object databases are now accessed
natively via their own repository structures. This simplifies
object database management and prepares the codebase for migrating
alternate tracking into the files backend.
* ps/odb-stop-registering-in-memory-sources:
odb: remove the ability to link sources ad-hoc
t/helper: stop registering alternates in "ref-store" command
t/helper: adapt read-midx to not link ad-hoc source anymore
builtin/multi-pack-index: refuse unknown sources with "--object-dir="
odb/packed: fix memory leaks when freeing source
tmp-objdir: drop unused function to register alternate
odb: remove infrastructure to register submodule sources
builtin/grep: stop registering submodule ODB as source
submodule-config: stop registering submodule sources
submodule-config: stop using `the_hash_algo`
submodule-config: remove uses of `the_repository`
cache-tree: remove dependency on `the_repository`
cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
The consistency checks for loose objects are hosted by "builtin/fsck.c".
These checks are obviously specific to the "loose" backend.
Move the logic into `odb_source_loose_fsck()`. Introduce a new "verbose"
flag so that we can properly retain semantics around whether or not we
want to print some status messages.
Note that this fixes a bug as a side effect: the progress meter was
captured in the callback data before `start_progress()` was even called,
so the per-subdirectory progress updates always operated on a NULL
pointer and the meter jumped straight from 0 to 256 upon completion. The
new code only sets up the callback data's progress meter after it has
been created, so the progress display now advances incrementally again.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The checks for multi-pack indexes are hosted in `cmd_fsck()` directly.
These checks are obviously specific to the "packed" backend.
Move the logic into `odb_source_packed_fsck()`. As in preceding commits,
this means that we now properly honor both "--connectivity-only" and
"--no-full". Furthermore, we drop the dedicated `ERROR_MULTI_PACK_INDEX`
bit and instead use the generic `ERROR_OBJECT` bit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The checks for bitmaps live in `verify_bitmap_files()`, which is called
by "builtin/fsck.c". These checks are obviously specific to the "packed"
backend.
Move the logic into `odb_source_packed_fsck()`. As in preceding commits,
this means that we now properly honor both "--connectivity-only" and
"--no-full". Furthermore, we drop the dedicated `ERROR_BITMAP` bit and
instead use the generic `ERROR_OBJECT` bit.
Note that this change also adapts `verify_bitmap_files()` to be
focused on a single "packed" source instead of verifying bitmaps from
all sources. This change is required as we already know to loop around
the sources in `odb_fsck()` itself.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The checks for reverse indexes live in `check_pack_rev_indexes()`, which
is hosted in "builtin/fsck.c". These checks are obviously specific to
the "packed" backend.
Move the logic into `odb_source_packed_fsck()`. As in the preceding
commit, drop the dedicated `ERROR_PACK_REV_INDEX` bit and instead use
the generic `ERROR_OBJECT` bit.
Note that this changes behaviour in two ways:
- The checks are now skipped when "--connectivity-only" was passed.
This is because we don't even run `odb_fsck()` at all when that
flag has been passed by the user, and not verifying data structures
of the object database matches the documented intent of that flag,
which is to only check the connectivity of reachable objects.
- The checks are now skipped for non-local sources when "--no-full"
was passed. This is, again, in line with the documented intent of
that flag.
Add a test to cast these semantics into stone.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the packfile verification out of `cmd_fsck()` and into the "packed"
source. While doing so, thread the progress meter and object callback
through the newly introduced `struct odb_fsck_options` so that the
caller's preferences are honoured without exposing those details at the
"builtin/fsck.c" level.
Note that the old code reported failures when verifying packfiles with
the `ERROR_PACK` bit, which gets returned to the caller via the exit
code. This bit is neither exercised in our test suite nor is it
documented anywhere in our codebase. Furthermore, this bit is highly
specific to the object storage backend, which makes it a bad fit for the
new pluggable infrastructure. So instead of retaining these semantics,
we drop them and return the generic `ERROR_OBJECT` bit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The on-disk consistency checks in git-fsck(1) are conceptually
backend-specific: while connectivity checks and object-level parsing
checks are generic, verifying the physical integrity of packfiles and
loose objects is meaningful only to backends that use these formats:
Having these checks live in "builtin/fsck.c" violates that layering,
because it forces the command to reach directly into format-specific
internals.
Provide new infrastructure to make these format-specific checks
pluggable and implement stubs for the different source types we already
have. In subsequent commits we'll move functionality over piece by
piece.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When freeing a "packed" source we don't close either its packs nor its
multi-pack indices. This can cause memory leaks in case we create an
ad-hoc packed source. As we used to always link packed sources to the
main object database we never noticed this issue until now, but it's
going to surface in subsequent commits where we stop linking them.
Plug the memory leaks by closing the source first.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are no users of `odb_source_write_alternates()` in our tree
anymore. Remove that function and its supporting infrastructure.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add the ability to write alternates when creating the object database.
This change allows us to remove the `write_alternates()` callback in a
subsequent patch.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The object lookup machinery has been taught to gracefully recover
when a multi-pack-index points to an owning pack that was removed
during a concurrent geometric repack, and 'git replay' has been
fixed to not segfault when reading such missing objects.
* en/midx-missing-pack-fallback:
packfile: recover when a multi-pack-index names a removed pack
mktree: do not use OBJECT_INFO_QUICK when checking objects
mktree: plug per-tree leak in --batch mode
replay: fail gracefully when a merge input is unreadable
The mechanism to generate a packfile corresponding to the result of
a fetch/push has been made pluggable through a set of object
database callback functions, removing hardcoded references to
'pack-objects' and enabling alternative ODBs to serve packfiles
themselves.
* ps/odb-pluggable-pack-generation:
bundle: generate packfiles via the object database
bundle: get (mostly) rid of `the_repository`
builtin/bundle: refactor option handling for progress meter
send-pack: generate packfiles via the object database
upload-pack: generate packfiles via the object database
odb: introduce interface to generate packfiles
The 'git receive-pack' command has been updated to use a new ODB
transaction interface for writing incoming packfiles, making it more
backend-agnostic.
* jt/receive-pack-pluggable-writes:
odb/transaction: add transaction interface to write packfiles
odb: return temporary ODB source when set
builtin/receive-pack: explicitly pass packfile fd
builtin/receive-pack: report unpack errors via strbuf
builtin/receive-pack: lift global state out of unpack()
builtin/receive-pack: read unpack limit config lazily
builtin/receive-pack: pass shallow file explicitly
odb/transaction: add transaction finalize interface
builtin/receive-pack: properly clean up keep files
The threshold for geometric repacking to trigger based on loose
object count has been adjusted to match that of 'git gc --auto',
preventing over-aggressive repacking during concurrent writes.
* ps/odb-geometric-repack-loose-threshold:
odb/files: be less aggressive with geometric repacking
The object database layer has been simplified by eagerly loading
alternate object directories upon initialization, instead of
deferring it to the first object lookup. This eliminates the need
for scattered lazy-loading calls throughout the codebase and paves
the way for integrating alternates with the pluggable backends.
* ps/odb-eagerly-load-alternates:
odb: drop `alternates_db` field
odb: drop `loaded_alternates` field
odb: eagerly initialize alternates
odb: decouple source path comparisons from `the_repository`
setup: create ref and object databases after config is written
The object database (odb) API has been refactored to distinguish
between missing objects and corrupt ones by returning more
descriptive error statuses. Both the packed and loose backends now
faithfully propagate error details using a generic strbuf error
mechanism, removing backend-specific leakage from central lookup
paths.
* ps/odb-generic-corrupt-objects:
odb: handle `OBJECT_INFO_DIE_IF_CORRUPT` generically
odb/source: allow `read_object_info()` to bubble up error messages
odb/source: let callers discern missing and corrupt objects
odb/source: introduce error status when reading objects
odb/source-packed: flag known-bad objects as corrupt and not missing
A geometric repack writes a new pack and multi-pack-index and then
deletes the packs the new one subsumes. A process still using the
previous MIDX keeps seeing a removed pack listed as the owner of some
objects. Since a MIDX attributes each object to exactly one pack, such
an object is served only through its recorded owner; if that owner was
just removed, find_pack_entry() cannot serve it -- the MIDX lookup routes
to the missing pack, and the regular pack fallback deliberately skips
every MIDX-covered pack, so a surviving copy in another covered pack
(e.g. a kept base pack) is never consulted.
Unlike the ordinary "a pack's .idx is mapped but its .pack is gone"
race, the second read does not rescue us. Reloading the on-disk pack set
does not reload the borrowed, cached MIDX (freeing it under the code that
caches the "struct multi_pack_index *" would be a use-after-free), so the
stale MIDX keeps routing to the removed pack and the surviving copy stays
hidden behind the covered-pack skip. cat-file, rev-list and pack-objects
can thus all spuriously fail with "unable to read object".
Teach find_pack_entry() to recover. The MIDX lookup now returns a
tri-state, distinguishing an object absent from the MIDX from one it owns
via a pack that can no longer be opened; in the latter case, once the
regular fallback has also missed, scan the MIDX's packs directly for a
surviving copy. Because the return value is no longer a boolean, rename
fill_midx_entry() to midx_fill_entry() so callers must reckon with the
new enum rather than silently treat MIDX_FILL_OWNER_UNAVAILABLE as a hit.
Do the scan only on the second read (OBJECT_INFO_SECOND_READ): by then
the cheaper on-disk reload has run, so an object merely relocated into a
new (uncovered) pack has already been found by the regular fallback, and
only a genuine hidden duplicate reaches the rescan. A QUICK caller that
skips the second read simply accepts the false negative, as QUICK is
designed to.
Reloading the stale MIDX would be a more complete fix but is much more
involved (the borrowers above need proper invalidation), so leave that
for later.
Assisted-by: Claude Opus 4.8 & GPT-5.6 Sol
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* ps/odb-eagerly-load-alternates:
odb: drop `alternates_db` field
odb: drop `loaded_alternates` field
odb: eagerly initialize alternates
odb: decouple source path comparisons from `the_repository`
setup: create ref and object databases after config is written
* ps/odb-pluggable-pack-generation:
bundle: generate packfiles via the object database
bundle: get (mostly) rid of `the_repository`
builtin/bundle: refactor option handling for progress meter
send-pack: generate packfiles via the object database
upload-pack: generate packfiles via the object database
odb: introduce interface to generate packfiles
The unused name parameter in 'struct chdir_notify_entry' has been
removed from chdir_notify_register(), chdir_notify_unregister(), and
related callback signatures across several subsystems, simplifying the
API now that trace output no longer uses it.
* ch/chdir-notify-drop-name:
chdir-notify.h: Removed unused param 'name'
* ps/odb-eagerly-load-alternates:
odb: drop `alternates_db` field
odb: drop `loaded_alternates` field
odb: eagerly initialize alternates
odb: decouple source path comparisons from `the_repository`
setup: create ref and object databases after config is written
The 'struct odb_read_stream' and 'struct odb_write_stream'
structures have been consolidated into a single unified 'struct
odb_stream' structure, simplifying object database streaming APIs
and enabling streaming of arbitrary object types.
* ps/odb-streams:
odb/streaming: unify function names to create new streams
odb/streaming: rename `struct input_zstream_data`
odb/streaming: rename `struct read_object_fd_data`
odb/streaming: consolidate read and write streams
odb/streaming: rename `struct odb_read_stream`
odb/streaming: support streaming arbitrary object types
odb/streaming: drop `is_finished` field
odb/streaming: track write stream size in the structure
When performing auto-maintenance with geometric repacking we have two
conditions that may trigger a repack:
- Either the geometric sequence of packfiles is invalidated.
- Or we have too many loose objects.
The first condition shouldn't trigger all that often: it may be hit when
we fetch a new packfile, but users tend to not do that all the time. The
second condition is what typically triggers more regularly though, as
every command that ends up writing new objects may cause us to cross the
threshold of loose objects. It is thus preferable to not be too
aggressive here, as otherwise we may end up repacking objects quite
often.
For the geometric-repacking strategy though we have a default of 100
objects, only. As we're approximating the count of objects by only
reading the "objects/17/" shared, we'd only need 2 objects in there
before we perform a repack by default, which is quite aggressive.
git-gc(1) on the other hand has a default of 6700, so it is quite a bit
more conservative here.
Being this aggressive is also causing problems as reported by our users.
When running lots of concurrent writers, those writes will constantly
end up spawning maintenance jobs that end up repacking objects. As we
also prune objects, a concurrently running process that tries to write
an object may see that the sharding directories get removed under their
feet. While we try re-creating such leading directories, we only do so a
single time, and it may happen that the directory vanishes again before
we had the chance to create the loose object. This is not a new problem,
but it is exacerbated by us running maintenance this aggressively.
Improve the status quo by reducing the frequency at which we pack loose
objects to the same frequency that git-gc(1) uses.
Reported-by: Stefan Haller <lists@haller-berlin.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Packfiles have two primary use cases:
- They are used to store objects at rest in a Git repository.
- They are used on the transport layer to transfer objects between two
repositories.
The first class is closely tied to a given object database backend, and
as such this use is highly specific to how such a backend decides to
store its data. This shows in git-pack-objects(1), which is used by
git-repack(1) et al to optimize the object database, which supports lots
of options that are closely coupled with how data is stored.
But the second class is quite a lot more generic: we don't care about
specifics of how the object database stores its objects, but to generate
the packfiles we only care about the object graph itself. Still, this
use case is also coupled with git-pack-objects(1).
Unfortunately, because git-pack-objects(1) covers both classes, the
result is that it is very hard to port the whole command to properly
support pluggable object databases. There are simply way too many
options that an alternative implementation will have a very hard time to
support in the first place.
And despite being hard to implement, it's also quite unnecessary to
implement those backend-specific options. Optimizing the object database
has already been made pluggable, and an alternative implementation is
unlikely to care about cruft packs, unpacked objects, keep packs and the
like. But we still need to make at least _parts_ of the packfile
generation pluggable so that backends can generate packfiles for the
transport layer itself.
Introduce a new interface that lets backends generate a new packfile and
implement that interface for the "files" backend. The options supported
by the callback are exactly the set of options that are required for the
transport layer, but nothing more.
This means that git-pack-objects(1) itself cannot be ported over to this
new interface, but as explained above that's a hard feat to pull off due
to the backend-specific features. Ideally though, we should expose the
ability to generate arbitrary packfiles using this interface. The intent
of this is to eventually introduce a git-objects(1) subcommand (similar
to git-refs(1)) that exposes generic interfaces for accessing everything
related to the object database. In that case, we are able to expose only
those options that are generic.
Subsequent commits will convert git-upload-pack(1), git-send-pack(1) and
git-bundle(1) to use this interface.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In git-receive-pack(1), the incoming packfile is written to the ODB via
`unpack()`, which spawns git-index-pack(1) or git-unpack-objects(1)
directly. With pluggable object databases, an alternative backend may
need to handle writing packfile data differently though.
Introduce `odb_transaction_write_pack()` as a generic interface to
handle writing a packfile to a transaction and use the logic from
`unpack()` as the "files" backend implementation. Note that when storing
the objects as a packfile, git-index-pack(1) also writes a ".keep"
lockfile next to it to prevent a concurrent repack from removing the new
pack prior to reference updates being performed. The "files" transaction
backend is responsible for managing these ".keep" files and removes them
post-commit once the transaction is finalized.
Call sites in git-receive-pack(1) are updated accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When committing an ODB transaction via `odb_transaction_commit()`, the
staged objects are made visible and the underlying transaction is freed
at the same time. Coupling these two steps does not leave room for any
post-commit transaction operations to be introduced though. Such a
capability is useful if an ODB transaction backend needs to hold on to
lockfiles after transaction commit until references are updated, as is
the case with the existing "files" backend in git-receive-pack(1).
Stop freeing the transaction in `odb_transaction_commit()` and introduce
`odb_transaction_finalize()` to explicitly clean up the transaction
accordingly. Note that the finalize interface also provides an optional
callback for any backend-specific deferred cleanup. In a subsequent
commit, the "files" transaction backend will use this to remove ".keep"
files generated for packfiles received via git-receive-pack(1) after
references have been updated. In preparation for this, the
`odb_transaction_finalize()` call site in git-receive-pack(1) is made
after the reference updates are finished.
All other callers commit a transaction and immediately finalize it
without any work happening in between those two operations.
Consequently, they cannot meaningfully recover in case either of them
would fail, and spelling out these two separate steps with proper error
handling would be quite repetitive and pointless. Introduce a helper
`odb_transaction_commit_and_finalize_or_die()` for those call sites and
update them accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The creation of the on-disk data structures for the object database
has been made pluggable, allowing future backends to customize their
setup. As part of this, the initialization of the object database
has been deferred, and the loading of the loose-object map has been
detangled from repository initialization.
* ps/odb-make-creation-pluggable:
odb: make creation of on-disk structures pluggable
odb/source: introduce function to map source type to name
setup: defer object database creation
setup: handle ODB-related environment variables in `odb_new()`
setup: detangle loading of loose object maps
loose: load loose object map for the correct source
When a lookup with `OBJECT_INFO_DIE_IF_CORRUPT` fails we want to die in
case the object exists, but cannot be read. This flag is handled in two
different spots right now:
- `do_oid_object_info_extended()` calls `has_packed_and_bad()` to
check whether the object is known to be corrupt in any packfile.
This function reaches into the internals of the packed source and
thus breaks the abstraction provided by our object sources.
- The loose source handles the flag itself and dies directly in
`read_object_info_from_path()`, which means that we die even in
cases where another source may still have a good copy of the
object.
Besides being inconsistent, it also ties us to the specific backend used
by the database sources because `has_packed_and_bad()` assumes that they
use the "files" backend. Any other backend will instead cause us to die
when calling `odb_source_files_downcast()`, even if the object was
simply nonexistent.
In the preceding commits we've carved out the infrastructure to make
this mechanism fully generic. On the one hand, all backends now tell us
whether the object is missing or corrupt via their return values. And
on the other hand, they have been taught to provide a readable error
message to the caller.
Adapt `do_oid_object_info_extended()` to use those new mechanisms. This
means that we won't die immediately anymore when a loose object is
corrupt, and we properly handle backends other than the "files" backend.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano