Re: [PATCH 1/8] t: fix races caused by background maintenance

On Mon, Aug 10, 2026 at 12:45:57PM +0200, Stefan Haller wrote:
> On 10.08.26 10:35, Patrick Steinhardt wrote:
> > On Mon, Aug 10, 2026 at 09:37:01AM +0200, Stefan Haller wrote:
[snip]
> geometric_repack_auto_condition() (builtin/gc.c) passes its threshold to
> too_many_loose_objects(), which does not count loose objects: it counts
> the entries of .git/objects/17 and scales by 256.  In v2.54.0:
> 
>     int auto_threshold = DIV_ROUND_UP(limit, 256);
>     [...]
>             if (++num_loose > auto_threshold) {
> 
> and equivalently after the rewrite in v2.55.0:
> 
>     /*
>      * This is weird, but stems from legacy behaviour: [...]
>      */
>     int auto_threshold = DIV_ROUND_UP(limit, 256) * 256;
>     [...]
>     return loose_count > auto_threshold;
> 
> with loose_count coming from ODB_COUNT_OBJECTS_APPROXIMATE, i.e. the
> same one-directory estimate.  Either way, any limit <= 256 collapses to
> "two or more objects share the objects/17 directory".

That's by design, and is also true for git-gc(1).

> That estimator is fine for gc.auto, whose default of 6700 needs 27
> entries in that directory -- a number you only reach with thousands of
> objects, and whose documentation says "approximately".  It falls apart
> for a threshold below 256, where the smallest representable estimate
> step exceeds the threshold itself and a single fanout collision decides
> the outcome.  For a repository with n objects the condition is satisfied
> with probability ~1-(1-p)^n-np(1-p)^(n-1), p=1/256: about 5% at 90
> objects, and much higher for repositories that accumulate objects over
> time.

But I tend to agree that the default value here is too low. That's an
easy-enough change to make:

diff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc
index b578856dde..da8be9f812 100644
--- a/Documentation/config/maintenance.adoc
+++ b/Documentation/config/maintenance.adoc
@@ -101,7 +101,7 @@ maintenance.geometric-repack.auto::
 	there are packfiles that need to be merged together to retain the
 	geometric progression, or when there are at least this many loose
 	objects that would be written into a new packfile. The default value is
-	100.
+	6700.
 
 maintenance.geometric-repack.splitFactor::
 	This integer config option controls the factor used for the geometric
diff --git a/odb/source-files.c b/odb/source-files.c
index 5a68af7d84..555e466145 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -521,7 +521,7 @@ bool odb_source_files_optimize_required(struct odb_source *source,
 		};
 		struct existing_packs existing_packs = EXISTING_PACKS_INIT;
 		struct string_list kept_packs = STRING_LIST_INIT_DUP;
-		int auto_value = 100;
+		int auto_value = 6700;
 		bool ret;
 
 		repo_config_get_int(repo, "maintenance.geometric-repack.auto",

> 2. The resulting background repacks break concurrent commands
> -------------------------------------------------------------
> 
> `git repack -d` installs the new pack, removes the redundant ones and
> then calls prune_packed_objects(), which unlinks the loose copies of
> objects that are now packed and rmdir()s the fanout directories it
> empties.  Doing that concurrently with unrelated git processes in the
> same repository is new exposure: before v2.54.0 the same repositories
> never reached the gc.auto threshold and no such repack ever ran.

Okay, so the issue is basically preexistent, but because we now repack a
lot more aggressively it's surfacing more often. Ideally, we'd fix that,
but it's also clear that repacking too often will make us race a lot
more, so we should avoid doing that too aggressively.

Does the issue go away if you set `maintenance.geometric-repack.auto=6700`?
If yes, I'd propose to simply change that default to be in line with
what git-gc(1) uses.

Thanks!

Patrick