Re: [PATCH] odb/files: be less aggressive with geometric repacking

Heh, looks like a typed up a response and got distracted just before
the end and never came back and sent it.  Sending now due to Patrick's
ping about this not showing up in What's Cooking; maybe an extra
review will help.  :-)

On Tue, Aug 11, 2026 at 2:17 AM Patrick Steinhardt <[email protected]> wrote:
>
> 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.

2?  Wouldn't you only need 1 (or if you could have fractional numbers
of objects, only 0.390625 of them)?  <looks around...>    Oh, huh:

        /*
         * This is weird, but stems from legacy behaviour: the GC auto
         * threshold was always essentially interpreted as if it was rounded up
         * to the next multiple 256 of, so we retain this behaviour for now.
         */
        return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);

So, indeed, you need 2.

> 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.

Unrelated to this patch...but should git avoid pruning the loose
object sharding directories?

> Improve the status quo by reducing the frequency at which we pack loose
> objects to the same frequency that git-gc(1) uses.

Makes sense.

> Reported-by: Stefan Haller <[email protected]>
> Signed-off-by: Patrick Steinhardt <[email protected]>
> ---
> Hi,
>
> as reported by Stefan at [1]. Thanks!
>
> Patrick
>
> [1]: <[email protected]>
> ---
>  Documentation/config/maintenance.adoc | 2 +-
>  odb/source-files.c                    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> 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",
>
> ---
> base-commit: 010afd3166ddc64c9863b1506f12cbcdda0d4ea1
> change-id: 20260810-pks-geometric-maintenance-reduce-frequency-5c1c9423ceb3

Looks good to me.