From: Karthik Nayak Date: Tue, 06 Oct 2026 20:51:33 GMT Subject: Re: [PATCH 12/13] odb/source-files: move alternates into the backend Message-ID: In-Reply-To: <20261002-pks-odb-move-alternates-v1-12-8a63507b88c4@pks.im> Patrick Steinhardt writes: > 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. > Could the design be instead to use a mapping function which allows us to map objects to sources, based on some characteristics of the object? > - 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. > This is not really a problem for having multiple sources though. > - 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. > These two points do make sense around moving alternates into the files source. > 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. > Okay, this makes sense, so the new source could be merged source of some sorts, with internal logic which it uses to map to different sources. Nice. > 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. > Yeah, this makes sense. The term 'DIRECTORY' itself is not always extensible to other sources. [snip] > diff --git a/midx.c b/midx.c > index c0f82c4163..8638ddf0be 100644 > --- a/midx.c > +++ b/midx.c > @@ -829,21 +829,15 @@ void clear_incremental_midx_files_ext(struct odb_source_packed *source, const ch > > void clear_midx_file(struct repository *r) > { > - struct odb_source_files *files; > + struct odb_source_files *files = odb_source_files_downcast(r->objects->source); We remove the the previous `if(r->objects)` check here, is that okay? > struct strbuf midx = STRBUF_INIT; > > - if (r->objects) { > - struct odb_source *source; > - > - for (source = r->objects->sources; source; source = source->next) { > - files = odb_source_files_downcast(source); > - if (files->dirs->packed->midx) > - close_midx(files->dirs->packed->midx); > - files->dirs->packed->midx = NULL; > - } > + for (struct odb_files_dir *dir = files->dirs; dir; dir = dir->next) { > + if (dir->packed->midx) > + close_midx(dir->packed->midx); > + dir->packed->midx = NULL; > } > > - files = odb_source_files_downcast(r->objects->sources); > get_midx_filename(files->dirs->packed, &midx); > > if (remove_path(midx.buf)) [snip] This is the only question I have in this big patch, I did a full read and tried to compare the moves visually and they looked okay to me.