[PATCH 0/2] maintenance: handle geometric repack tasks with promisor pack(s)

[PATCH 0/2] maintenance: handle geometric repack tasks with promisor pack(s)

[Taylor Blau]

The geometric-repack maintenance task predates support for keeping
promisor packs in their own geometric progression. After that support
was added in dcc9c7ef47 (builtin/repack: handle promisor packs with
geometric repacking, 2026-01-05), the maintenance task still made two
decisions from the ordinary-pack progression alone:

 - whether an explicit run should use `--geometric` or its
   all-into-one fallback; and

 - whether `--auto` sees enough work to run the task at all.

That can make partial clones rewrite more than necessary. If the
ordinary packs would all be rolled up, the task can choose the
all-into-one path even when the promisor progression would leave a
large pack alone. Likewise, an all-promisor repository can have a
promisor rollup ready while `--auto` sees neither an ordinary split
nor enough loose objects and skips the task.

The first patch makes the repack-mode choice consider both
progressions. It keeps the all-into-one fallback only when neither
progression leaves a pack above its split, so the fallback does not
rewrite packs that geometric repack would have kept.

The second patch makes the `--auto` condition consider
`geometry.promisor_split` alongside `geometry.split`. A non-zero split
on either side means that geometric repack can combine at least two
packs.

Both tests build three promisor packs whose object counts cause the two
smaller packs to roll up while leaving the large pack intact. The
`--auto` test uses a high loose-object threshold, so the promisor split
is the only reason the task runs.

Thanks in advance for your review!

Taylor Blau (2):
  maintenance: account for promisor pack geometry
  maintenance: trigger --auto for promisor rollups

 builtin/gc.c                  |  5 +--
 t/t5331-pack-objects-stdin.sh |  3 +-
 t/t7900-maintenance.sh        | 68 +++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 3 deletions(-)


base-commit: a97fcc37c2bc6340a8d7ce78dedf227aac4e9aa7
-- 
2.55.0.483.gdc2fffc37c

[PATCH 1/2] maintenance: account for promisor pack geometry

[Taylor Blau]

Commit 9bc151850c (builtin/maintenance: introduce
"geometric-repack" task, 2025-10-24) added a new maintenance task to
perform either a geometric repack, or an all-into-one repack if the
geometric repack would itself produce a single pack.

Some time later, commit dcc9c7ef47 (builtin/repack: handle promisor
packs with geometric repacking, 2026-01-05) taught the geometric
repacking machinery to separate promisor packs from ordinary ones, but
did not update the maintenance task accordingly.

As a consequence, the geometric-repack maintenance task only considers
the non-promisor pack progression. It falls back to all-into-one
whenever a geometric repack would roll up all non-promisor packs into a
single pack, even if the promisor progression would keep a large pack
and roll up only smaller ones.

Check both progressions before choosing the repack mode. If either
leaves a pack above its split, geometric repack still avoids rewriting
that pack, whereas the all-into-one fallback would rewrite it. Use the
fallback only when neither progression leaves a pack behind. That
preserves the reason for the fallback: let the all-into-one repack
handle unreachable objects when it is not rewriting more packs than the
geometric repack.

Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
 builtin/gc.c           |  3 ++-
 t/t7900-maintenance.sh | 45 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 49c8474fad..ed75c12c43 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1593,7 +1593,8 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
 	child.odb_to_close = the_repository->objects;
 
 	strvec_pushl(&child.args, "repack", "-d", "-l", NULL);
-	if (geometry.split < geometry.pack_nr)
+	if (geometry.split < geometry.pack_nr ||
+	    geometry.promisor_split < geometry.promisor_pack_nr)
 		strvec_pushf(&child.args, "--geometric=%d",
 			     geometry.split_factor);
 	else
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index a8d691719d..ba5b359e77 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -659,6 +659,51 @@ test_expect_success 'geometric repacking task' '
 	)
 '
 
+objdir=.git/objects
+packdir=$objdir/pack
+
+pack_promisor () {
+	p="$(echo "$@" | git pack-objects --revs $packdir/pack)" &&
+	touch "$packdir/pack-$p.promisor" &&
+	echo "$p"
+}
+
+test_expect_success 'geometric repacking task handles promisor packs' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		git config set maintenance.auto false &&
+		git remote add promisor garbage &&
+		git config set remote.promisor.promisor true &&
+
+		for n in $(test_seq 6)
+		do
+			test_commit $n || return 1
+		done &&
+
+		A="$(pack_promisor 1)" &&
+		B="$(pack_promisor 1..2)" &&
+		C="$(pack_promisor 2..6)" &&
+		git prune-packed &&
+
+		ls $packdir/pack-*.promisor | sort >promisors.before &&
+		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+			git maintenance run --quiet --task=geometric-repack &&
+		ls $packdir/pack-*.promisor | sort >promisors.after &&
+
+		test_subcommand git repack -d -l --geometric=2 \
+			--quiet --write-midx <trace2.txt &&
+
+		test_line_count = 2 promisors.after &&
+
+		printf "$packdir/pack-%s.promisor\n" "$A" "$B" | sort >expect &&
+		comm -23 promisors.before promisors.after >actual &&
+
+		test_cmp expect actual
+	)
+'
+
 test_geometric_repack_needed () {
 	NEEDED="$1"
 	GEOMETRIC_CONFIG="$2" &&
-- 
2.55.0.483.gdc2fffc37c

[PATCH 2/2] maintenance: trigger --auto for promisor rollups

[Taylor Blau]

Commit 9bc151850c (builtin/maintenance: introduce "geometric-repack"
task, 2025-10-24) added an auto condition for the geometric-repack
task. It runs the task when ordinary packs need to be combined or when
the number of loose objects crosses the configured threshold.

Later on in commit dcc9c7ef47 (builtin/repack: handle promisor packs
with geometric repacking, 2026-01-05), the geometric repack machinery
started handling promisor packs separately, but did not correspondingly
update the auto condition.

As a result, a repository can have promisor packs ready to combine
while its non-promisor packs and loose object count require no work. In
that case, `--auto` skips the task even though a geometric repack
would combine at least two promisor packs.

Check `geometry.promisor_split` alongside `geometry.split`.

There is some fallout in t5331: the new condition makes a filtered
clone eligible for auto-maintenance before the test inspects its
promisor packs. Disable auto-maintenance in that fixture so it
continues to test `--stdin-packs`, not the maintenance task.

Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
 builtin/gc.c                  |  2 +-
 t/t5331-pack-objects-stdin.sh |  3 ++-
 t/t7900-maintenance.sh        | 23 +++++++++++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index ed75c12c43..e9572940dc 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1649,7 +1649,7 @@ static int geometric_repack_auto_condition(struct gc_config *cfg UNUSED)
 	 * When we'd merge at least two packs with one another we always
 	 * perform the repack.
 	 */
-	if (geometry.split) {
+	if (geometry.split || geometry.promisor_split) {
 		ret = 1;
 		goto out;
 	}
diff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh
index c74b5861af..2a983e28ac 100755
--- a/t/t5331-pack-objects-stdin.sh
+++ b/t/t5331-pack-objects-stdin.sh
@@ -368,7 +368,8 @@ test_expect_success '--stdin-packs does not perform backfill fetch' '
 	git -C remote config set --local uploadpack.allowfilter 1 &&
 	git -C remote config set --local uploadpack.allowanysha1inwant 1 &&
 
-	git clone --filter=tree:0 "file://$(pwd)/remote" client &&
+	git -c maintenance.auto=false clone --filter=tree:0 \
+		"file://$(pwd)/remote" client &&
 	(
 		cd client &&
 		ls .git/objects/pack/*.promisor | sed "s|.*/||; s/\.promisor$/.pack/" >packs &&
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index ba5b359e77..fb5f2d8902 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -759,6 +759,29 @@ test_expect_success 'geometric repacking with --auto' '
 	)
 '
 
+test_expect_success 'geometric repacking with --auto handles promisor packs' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		git config set maintenance.auto false &&
+		git remote add promisor garbage &&
+		git config set remote.promisor.promisor true &&
+
+		for n in $(test_seq 6)
+		do
+			test_commit $n || return 1
+		done &&
+
+		pack_promisor 1 >/dev/null &&
+		pack_promisor 1..2 >/dev/null &&
+		pack_promisor 2..6 >/dev/null &&
+		git prune-packed &&
+
+		test_geometric_repack_needed true auto=9000
+	)
+'
+
 test_expect_success 'geometric repacking honors configured split factor' '
 	test_when_finished "rm -rf repo" &&
 	git init repo &&
-- 
2.55.0.483.gdc2fffc37c

Re: [PATCH 1/2] maintenance: account for promisor pack geometry

[Patrick Steinhardt]

On Tue, Aug 04, 2026 at 08:57:40PM -0700, Taylor Blau wrote:
> Commit 9bc151850c (builtin/maintenance: introduce
> "geometric-repack" task, 2025-10-24) added a new maintenance task to
> perform either a geometric repack, or an all-into-one repack if the
> geometric repack would itself produce a single pack.
> 
> Some time later, commit dcc9c7ef47 (builtin/repack: handle promisor
> packs with geometric repacking, 2026-01-05) taught the geometric
> repacking machinery to separate promisor packs from ordinary ones, but
> did not update the maintenance task accordingly.
> 
> As a consequence, the geometric-repack maintenance task only considers
> the non-promisor pack progression. It falls back to all-into-one
> whenever a geometric repack would roll up all non-promisor packs into a
> single pack, even if the promisor progression would keep a large pack
> and roll up only smaller ones.
> 
> Check both progressions before choosing the repack mode. If either
> leaves a pack above its split, geometric repack still avoids rewriting
> that pack, whereas the all-into-one fallback would rewrite it. Use the
> fallback only when neither progression leaves a pack behind. That
> preserves the reason for the fallback: let the all-into-one repack
> handle unreachable objects when it is not rewriting more packs than the
> geometric repack.

Okay. The consequence of the status quo could be that we perform an
all-into-one repack more frequent than really desired because the set of
non-promised packs is small, and thus even writing a small set of new
objects could cause a full repack.

This might create the reverse situation though, where we don't perform
the all-into-one repack at all anymore. We could come up with a clever
solution here, like for example considering both sequences together and
repacking when we cross a certain combined threshold. But I'm not sure
it's worth it for now, and we can still evolve the strategy as needed.

Patrick

Re: [PATCH 2/2] maintenance: trigger --auto for promisor rollups

[Patrick Steinhardt]

On Tue, Aug 04, 2026 at 08:57:46PM -0700, Taylor Blau wrote:
> Commit 9bc151850c (builtin/maintenance: introduce "geometric-repack"
> task, 2025-10-24) added an auto condition for the geometric-repack
> task. It runs the task when ordinary packs need to be combined or when
> the number of loose objects crosses the configured threshold.
> 
> Later on in commit dcc9c7ef47 (builtin/repack: handle promisor packs
> with geometric repacking, 2026-01-05), the geometric repack machinery
> started handling promisor packs separately, but did not correspondingly
> update the auto condition.
> 
> As a result, a repository can have promisor packs ready to combine
> while its non-promisor packs and loose object count require no work. In
> that case, `--auto` skips the task even though a geometric repack
> would combine at least two promisor packs.
> 
> Check `geometry.promisor_split` alongside `geometry.split`.

Yeah, this is a more obviously correct thing to do compared to the
preceding patch.

> diff --git a/builtin/gc.c b/builtin/gc.c
> index ed75c12c43..e9572940dc 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1649,7 +1649,7 @@ static int geometric_repack_auto_condition(struct gc_config *cfg UNUSED)
>  	 * When we'd merge at least two packs with one another we always
>  	 * perform the repack.
>  	 */
> -	if (geometry.split) {
> +	if (geometry.split || geometry.promisor_split) {
>  		ret = 1;
>  		goto out;
>  	}

This looks obviously correct.

> diff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh
> index c74b5861af..2a983e28ac 100755
> --- a/t/t5331-pack-objects-stdin.sh
> +++ b/t/t5331-pack-objects-stdin.sh
> @@ -368,7 +368,8 @@ test_expect_success '--stdin-packs does not perform backfill fetch' '
>  	git -C remote config set --local uploadpack.allowfilter 1 &&
>  	git -C remote config set --local uploadpack.allowanysha1inwant 1 &&
>  
> -	git clone --filter=tree:0 "file://$(pwd)/remote" client &&
> +	git -c maintenance.auto=false clone --filter=tree:0 \
> +		"file://$(pwd)/remote" client &&
>  	(
>  		cd client &&
>  		ls .git/objects/pack/*.promisor | sed "s|.*/||; s/\.promisor$/.pack/" >packs &&

Curious that git-clone(1) already spawns maintenance, but with "tree:0"
we may end up fetching multiple promisor packs from the remote as we
discover more trees to backfill. So this makes sense.

> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index ba5b359e77..fb5f2d8902 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -759,6 +759,29 @@ test_expect_success 'geometric repacking with --auto' '
>  	)
>  '
>  
> +test_expect_success 'geometric repacking with --auto handles promisor packs' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		git config set maintenance.auto false &&
> +		git remote add promisor garbage &&
> +		git config set remote.promisor.promisor true &&
> +
> +		for n in $(test_seq 6)
> +		do
> +			test_commit $n || return 1
> +		done &&
> +
> +		pack_promisor 1 >/dev/null &&
> +		pack_promisor 1..2 >/dev/null &&
> +		pack_promisor 2..6 >/dev/null &&
> +		git prune-packed &&
> +
> +		test_geometric_repack_needed true auto=9000

The auto-value here doesn't matter at all, as we shouldn't have any
loose objects in the first place and really only want to trigger
maintenance because of the promisors. Makes sense.

Thanks!

Patrick

Re: [PATCH 1/2] maintenance: account for promisor pack geometry

[Taylor Blau]

On Mon, Aug 10, 2026 at 05:11:32PM +0200, Patrick Steinhardt wrote:
> > Check both progressions before choosing the repack mode. If either
> > leaves a pack above its split, geometric repack still avoids rewriting
> > that pack, whereas the all-into-one fallback would rewrite it. Use the
> > fallback only when neither progression leaves a pack behind. That
> > preserves the reason for the fallback: let the all-into-one repack
> > handle unreachable objects when it is not rewriting more packs than the
> > geometric repack.
>
> Okay. The consequence of the status quo could be that we perform an
> all-into-one repack more frequent than really desired because the set of
> non-promised packs is small, and thus even writing a small set of new
> objects could cause a full repack.

Right. I stumbled on this after a few colleagues had reported that their
geometric maintenance task didn't seem to be doing anything. When
looking into it, I found that they had many promisor packs, but the
non-promisor packs were already in a geometric progression, and thus we
did an all-into-one repack.

> This might create the reverse situation though, where we don't perform
> the all-into-one repack at all anymore. We could come up with a clever
> solution here, like for example considering both sequences together and
> repacking when we cross a certain combined threshold. But I'm not sure
> it's worth it for now, and we can still evolve the strategy as needed.

The change in this patch means that we will perform a geometric repack
when doing so would result in a new geometrically-repacked series of
promisor packs, in addition to non-promisor ones.

Is your concern that the non-promisor packs might be in a state where we
should compact them into a single pack, but that the sequence of
promisor packs would prevent us from doing so? In that case, we will
perform a geometric repack on both sets of packs independently. If the
non-promisor packs should be rolled up into a single pack (i.e.,
"geometry.split == geometry.pack_nr"), then the geometric repack *will*
produce a single pack, as if we had performed an all-into-one repack on
the set of non-promisor packs.

So I am not sure that I understand your concern here, but please let me
know if I am missing some aspect of it.

Thanks,
Taylor

Re: [PATCH 2/2] maintenance: trigger --auto for promisor rollups

[Taylor Blau]

On Mon, Aug 10, 2026 at 05:11:37PM +0200, Patrick Steinhardt wrote:
> Thanks!
>
> Patrick

Thanks for the review!

Thanks,
Taylor

Re: [PATCH 1/2] maintenance: account for promisor pack geometry

[Patrick Steinhardt]

On Mon, Aug 10, 2026 at 10:56:14AM -0500, Taylor Blau wrote:
> On Mon, Aug 10, 2026 at 05:11:32PM +0200, Patrick Steinhardt wrote:
> > > Check both progressions before choosing the repack mode. If either
> > > leaves a pack above its split, geometric repack still avoids rewriting
> > > that pack, whereas the all-into-one fallback would rewrite it. Use the
> > > fallback only when neither progression leaves a pack behind. That
> > > preserves the reason for the fallback: let the all-into-one repack
> > > handle unreachable objects when it is not rewriting more packs than the
> > > geometric repack.
> >
> > Okay. The consequence of the status quo could be that we perform an
> > all-into-one repack more frequent than really desired because the set of
> > non-promised packs is small, and thus even writing a small set of new
> > objects could cause a full repack.
> 
> Right. I stumbled on this after a few colleagues had reported that their
> geometric maintenance task didn't seem to be doing anything. When
> looking into it, I found that they had many promisor packs, but the
> non-promisor packs were already in a geometric progression, and thus we
> did an all-into-one repack.
> 
> > This might create the reverse situation though, where we don't perform
> > the all-into-one repack at all anymore. We could come up with a clever
> > solution here, like for example considering both sequences together and
> > repacking when we cross a certain combined threshold. But I'm not sure
> > it's worth it for now, and we can still evolve the strategy as needed.
> 
> The change in this patch means that we will perform a geometric repack
> when doing so would result in a new geometrically-repacked series of
> promisor packs, in addition to non-promisor ones.
> 
> Is your concern that the non-promisor packs might be in a state where we
> should compact them into a single pack, but that the sequence of
> promisor packs would prevent us from doing so? In that case, we will
> perform a geometric repack on both sets of packs independently. If the
> non-promisor packs should be rolled up into a single pack (i.e.,
> "geometry.split == geometry.pack_nr"), then the geometric repack *will*
> produce a single pack, as if we had performed an all-into-one repack on
> the set of non-promisor packs.
> 
> So I am not sure that I understand your concern here, but please let me
> know if I am missing some aspect of it.

The concern is that it's quite unlikely that both the geometric and
non-geometric sequence will merge all packs together at the same point
in time. Consequently, we'll never hit the case where we perform an
all-into-one pack to prune unreachable objects, and that may cause us to
never prune objects at all.

So what I'm wondering is whether we should be a bit more clever about
that and perform an all-into-one repack under a new condition, like for
example when the objects we're about to repack exceed a certain
percentage of the repository size.

Hope that clarifies it a bit :)

Thanks!

Patrick

Re: [PATCH 1/2] maintenance: account for promisor pack geometry

[Taylor Blau]

On Tue, Aug 11, 2026 at 12:01:23PM +0200, Patrick Steinhardt wrote:
> > So I am not sure that I understand your concern here, but please let me
> > know if I am missing some aspect of it.
>
> The concern is that it's quite unlikely that both the geometric and
> non-geometric sequence will merge all packs together at the same point
> in time. Consequently, we'll never hit the case where we perform an
> all-into-one pack to prune unreachable objects, and that may cause us to
> never prune objects at all.
>
> So what I'm wondering is whether we should be a bit more clever about
> that and perform an all-into-one repack under a new condition, like for
> example when the objects we're about to repack exceed a certain
> percentage of the repository size.
>
> Hope that clarifies it a bit :)

Ah, I see what you're saying. We should still be OK here as the goal of
geometric repacking is to converge both the promisor and non-promisor
packs towards a single pack, at which point we would do an all-into-one
repack.

If the two are perfectly out of phase, then this change would prevent us
from running all-into-one maintenance. But that does not seem like a
likely scenario, and the behavior here should be a strict improvement in
the meantime otherwise.

Thanks,
Taylor

Re: [PATCH 1/2] maintenance: account for promisor pack geometry

[Patrick Steinhardt]

On Tue, Aug 11, 2026 at 10:49:49AM -0500, Taylor Blau wrote:
> On Tue, Aug 11, 2026 at 12:01:23PM +0200, Patrick Steinhardt wrote:
> > > So I am not sure that I understand your concern here, but please let me
> > > know if I am missing some aspect of it.
> >
> > The concern is that it's quite unlikely that both the geometric and
> > non-geometric sequence will merge all packs together at the same point
> > in time. Consequently, we'll never hit the case where we perform an
> > all-into-one pack to prune unreachable objects, and that may cause us to
> > never prune objects at all.
> >
> > So what I'm wondering is whether we should be a bit more clever about
> > that and perform an all-into-one repack under a new condition, like for
> > example when the objects we're about to repack exceed a certain
> > percentage of the repository size.
> >
> > Hope that clarifies it a bit :)
> 
> Ah, I see what you're saying. We should still be OK here as the goal of
> geometric repacking is to converge both the promisor and non-promisor
> packs towards a single pack, at which point we would do an all-into-one
> repack.
> 
> If the two are perfectly out of phase, then this change would prevent us
> from running all-into-one maintenance. But that does not seem like a
> likely scenario, and the behavior here should be a strict improvement in
> the meantime otherwise.

Yeah, I tend to agree. It's heuristics anyway, and from my point of view
it's something that we can iterate on going forward.

Thanks!

Patrick