Thread (41 messages) 41 messages, 2 authors, 9d ago

[PATCH v2 00/13] odb/source-files: move alternates into the backend

flat view
COOLING9d

From: Patrick Steinhardt <hidden>
Date: 2026-10-08 08:36:10

Revision v2 of 2 in this series.

Revisions (2)
  1. v1 [diff vs current]
  2. v2 current
Hi,

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.

This patch series corrects course by moving alternates into the "files"
backend itself so that they become another implementation detail. It's
unfortunately on the bigger side, and I'm sorry about that, but I
couldn't really find a way to split it up further in a sensible way.

Note that the above problems aren't fixed by this series yet, but it is
the prerequisite to fix them in subsequent patch series.

The series is built on top of c46c1e3772 (Start Git 2.98 cycle,
2026-09-30). See the first version of this patch series for the conflict
resolution that's required for this merge base.

Changes in v2:
  - Adapt the first commit message to better motivate the refactoring.
  - Link to v1: https://patch.msgid.link/20261002-pks-odb-move-alternates-v1-0-8a63507b88c4@pks.im (local)

Thanks!

Patrick

---
Patrick Steinhardt (13):
      commit-graph: require resolved packfile paths for `stdin_packs`
      commit-graph: stop depending on `struct odb_source`
      odb/source-files: introduce `struct odb_files_dir`
      odb: refactor `odb_for_each_alternate()` to yield dirs
      odb: refactor `odb_find_source()` to yield dirs
      odb/source-files: add the ability to have multiple object dirs
      tmp-objdir: absorb logic to set and restore primary sources
      tmp-objdir: manage quarantine as an object directory
      tmp-objdir: replace primary source at creation time
      odb/source: make `will_destroy` an implementation detail
      odb/source-files: extract reading alternates
      odb/source-files: move alternates into the backend
      odb/source: drop `read_alternates` callback

 builtin/commit-graph.c      |  41 ++--
 builtin/commit.c            |   2 +-
 builtin/count-objects.c     |   6 +-
 builtin/fast-import.c       |  20 +-
 builtin/fetch.c             |   4 +-
 builtin/fsck.c              |   6 +-
 builtin/gc.c                |  15 +-
 builtin/index-pack.c        |   4 +-
 builtin/merge.c             |   2 +-
 builtin/multi-pack-index.c  |  48 ++---
 builtin/pack-objects.c      |  66 +++---
 builtin/prune.c             |   2 +-
 builtin/repack.c            |   4 +-
 builtin/submodule--helper.c |   7 +-
 bundle.c                    |   2 +-
 commit-graph.c              | 150 +++++++-------
 commit-graph.h              |  31 ++-
 diagnose.c                  |   8 +-
 fetch-pack.c                |   2 +-
 http-walker.c               |   4 +-
 http.c                      |  12 +-
 log-tree.c                  |   3 +-
 loose.c                     |  18 +-
 midx.c                      |  43 ++--
 object-file.c               |   8 +-
 odb.c                       | 439 +++++-----------------------------------
 odb.h                       |  62 +-----
 odb/source-files.c          | 482 ++++++++++++++++++++++++++++++++++++--------
 odb/source-files.h          |  65 +++++-
 odb/source-inmemory.c       |   7 -
 odb/source-loose.c          |   9 +-
 odb/source-loose.h          |   3 +
 odb/source-packed.c         |   7 -
 odb/source.c                |   5 +-
 odb/source.h                |  57 +-----
 odb/streaming.c             |   8 +-
 odb/transaction.c           |   2 +-
 pack-bitmap.c               |   8 +-
 packfile.c                  |  28 ++-
 packfile.h                  |  21 +-
 path.c                      |   2 +-
 prune-packed.c              |   2 +-
 repack-geometry.c           |   2 +-
 repack-midx.c               |   6 +-
 repack.c                    |   6 +-
 repository.c                |   4 +-
 setup.c                     |   2 +-
 t/helper/test-read-graph.c  |   5 +-
 t/helper/test-read-midx.c   |   8 +-
 t/t4216-log-bloom.sh        |   4 +-
 tmp-objdir.c                |  67 ++++--
 tmp-objdir.h                |  18 +-
 52 files changed, 883 insertions(+), 954 deletions(-)

Range-diff versus v1:

 1:  985b068271 !  1:  43a288604d commit-graph: require resolved packfile paths for `stdin_packs`
    @@ Commit message
         for a set of packfiles via the "--stdin-packs" option. Those users are
         expected to pass in relative paths, and those eventually get resolved in
         `fill_oids_from_packs()`. This ties the logic in "commit-graph.c" to the
    -    specific object database source.
    +    specific object database source, as the subsystem now needs to assume
    +    where a specific packfile is located relative to the source itself.
     
         Refactor the logic to instead require the caller to pass in resolved
    -    packfiles to untangle that dependency.
    +    packfiles to untangle that dependency. This also makes the next change
    +    easier to implement, where we'll get rid of passing the source to the
    +    commit-graph subsystem.
     
         Signed-off-by: Patrick Steinhardt [off-list ref]
     
 2:  537c2895a9 =  2:  f455f1986c commit-graph: stop depending on `struct odb_source`
 3:  ce5ade106a =  3:  062f8b02d6 odb/source-files: introduce `struct odb_files_dir`
 4:  4336f3d53d =  4:  68e73bbdc6 odb: refactor `odb_for_each_alternate()` to yield dirs
 5:  77c5749db8 =  5:  078d51c82d odb: refactor `odb_find_source()` to yield dirs
 6:  3d59418c91 =  6:  b203565c43 odb/source-files: add the ability to have multiple object dirs
 7:  38b3658234 =  7:  c53ba11900 tmp-objdir: absorb logic to set and restore primary sources
 8:  928edd062b =  8:  92c0a38193 tmp-objdir: manage quarantine as an object directory
 9:  f51db68dea =  9:  e1fc5be9a7 tmp-objdir: replace primary source at creation time
10:  a71a69e2c3 = 10:  98605aa991 odb/source: make `will_destroy` an implementation detail
11:  fd434eaa74 = 11:  2ce6282162 odb/source-files: extract reading alternates
12:  a4a8e53216 = 12:  bc2a33d360 odb/source-files: move alternates into the backend
13:  39a3e9e1dd = 13:  41834f9155 odb/source: drop `read_alternates` callback

---
base-commit: 2f92b2890ddaf3d7ea29470c02418271c1a4cd79
change-id: 20260924-pks-odb-move-alternates-4a0babe4b0f3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help