Maintenance inception - #2146
Open
newren wants to merge 25 commits into
Open
Conversation
When considering an object pair for delta compression, try_delta() short-circuits (skipping a real delta search) when both objects came from the same on-disk pack and neither is stored there as a delta. The reasoning is that an earlier pack-objects run already had the chance to try this pair and chose not to delta either against the other, so trying again is unlikely to find anything new. That heuristic is correct only if the source pack was actually built by something that did a real delta search. That may not be the case if the pack was written by `git fast-import` (which only does a basic delta comparison for objects in the pack it writes), the bulk_checkin framework when streaming large blobs straight into a pack, or pack-objects with --window=0. Add a `.baddeltas` sidecar marker, parallel to `.keep`, `.promisor`, and `.mtimes`, that lets a producer flag its output pack as one whose delta layout should not be trusted by this skip. For context on the underlying delta skip and earlier discussions of a similar marker, see Jeff King's analysis on the list: https://lore.kernel.org/git/20231009202149.GA3281325@coredump.intra.peff.net/ Assisted-by: Claude Opus 4.7 Signed-off-by: Elijah Newren <newren@gmail.com>
newren
force-pushed
the
maintenance-inception
branch
3 times, most recently
from
June 11, 2026 14:34
9a7521d to
24bdb43
Compare
"git cat-file --batch-all-objects" opens each pack's .idx lazily, as its walk reaches that pack. If a concurrent repack removes a pack first, opening its index fails. The object database already turns this into a non-zero return from odb_for_each_object_ext(), but batch_each_object() discarded it and we reported success -- silently omitting the vanished pack's objects while still exiting 0, which is dangerous for tooling that trusts the listing to be complete. Propagate the error: return it from batch_each_object() and, when set, exit non-zero with a diagnostic instead of pretending a truncated listing was whole. To reproduce, loop "git repack -adq" in one process while another loops "git cat-file --batch-all-objects --batch-check"; before this change the reader occasionally stopped with status 0 mid-repack, and now it errors instead. Assisted-by: Claude Opus 4.8 Signed-off-by: Elijah Newren <newren@gmail.com>
"git cat-file --batch-all-objects --unordered" reads each object from the pack and offset the walk handed it, rather than looking it up again. If a concurrent repack removes that pack in between, packed_object_info() calls use_pack(), which cannot reopen the unlinked pack and dies with "packfile ... cannot be accessed" -- even though the object still lives in the pack repack just wrote. Guard the read with is_pack_valid(): if the pack can no longer be opened, clear the pointer so we fall back to a normal object-database lookup, which finds the object in whatever pack now holds it. is_pack_valid() opens and pins the pack's fd, so once it succeeds the pack stays readable even if it is unlinked an instant later. Assisted-by: Claude Opus 4.8 Signed-off-by: Elijah Newren <newren@gmail.com>
The previous commit made "git cat-file --batch-all-objects" error out when a concurrent repack removes a pack mid-walk. That is correct but pessimistic on servers that repack often. Reduce how often it triggers by opening every pack index up front, before the walk. An index mmap survives unlink() of the .idx and pack fd pressure (close_one_pack() closes only the pack fd), so the object set is fixed once the indexes are open. For --batch-all-objects this adds no work: the walk opens every index anyway. Like f6b2625 (fsck: snapshot default refs before object walk, 2026-01-09), this narrows rather than closes the race; the residue is still caught by the previous commit rather than silently dropped. Assisted-by: Claude Opus 4.8 Signed-off-by: Elijah Newren <newren@gmail.com>
"git fsck" enumerates packs up front and later verifies each pack's reverse index. If a concurrent "git repack" removes one of those packs in the meantime, load_pack_revindex_from_disk() fails and fsck reports "unable to load rev-index", implying corruption. That is misleading: the objects are safe in the replacement pack and a quiescent retry succeeds. When the load fails because the ".pack" itself is gone (ENOENT), say so and suggest retrying once maintenance completes, rather than blaming the rev-index. A failure with the pack still present is reported as before, so genuine corruption still surfaces. This can be provoked by looping "git repack -adq" in one process while another loops "git fsck --connectivity-only": occasionally fsck trips over a pack removed after it was enumerated, and now says the pack disappeared instead of reporting a bad rev-index. Assisted-by: Claude Opus 4.8 Signed-off-by: Elijah Newren <newren@gmail.com>
"git multi-pack-index verify" (which "git fsck" always spawns) walks the midx's packs, closing and reopening each by name as it checks object offsets. A concurrent "git repack" that unlinks a redundant pack in that window makes the reopen fail, and verify reports "failed to load pack in position N", "failed to load pack entry for oid[N]", or "failed to load pack-index for packfile ..." -- all of which read like midx corruption, though the objects are safe in the replacement pack and a quiescent retry succeeds. When such a failure is explained by a midx-referenced ".pack" having vanished (repack unlinks a redundant pack's ".idx" before its ".pack", so a missing ".pack" is the tell-tale), add a one-time hint to retry when quiescent. The existing per-failure messages are kept, so genuine corruption is still reported as before. To provoke it, "git multi-pack-index write" a repo with several packs and run "git multi-pack-index verify" while "git repack -adq" loops in the background; verify occasionally fails with one of the messages above, now followed by the retry hint. Assisted-by: Claude Opus 4.8 Signed-off-by: Elijah Newren <newren@gmail.com>
The previous patch taught pack-objects to honor a `.baddeltas` sidecar marker that tells the same-pack delta skip in try_delta() to fall through to a real delta search. Add a `--mark-bad-deltas` option that writes such a marker alongside each output pack. The flag is incompatible with `--stdout`, since the marker lives next to the output pack in `$GIT_DIR/objects/pack` and `--stdout` has no on-disk output to mark. When pack-objects splits its output across multiple packs (with `--max-pack-size`), a marker is written for each resulting pack inside the existing per-pack output loop, between stage_tmp_packfiles() and rename_tmp_packfile_idx() so the marker is in place before the `.idx` anchor becomes visible. Document the option, and extend t5335 to cover both the producer side (marker is written and disables the same-pack skip for the resulting pack) and the `--stdout` incompatibility. Assisted-by: Claude Opus 4.7 Signed-off-by: Elijah Newren <newren@gmail.com>
pack-objects performs a somewhat expensive revision walk to:
* fill in object namehashes, improving delta selection;
* with --stdin-packs=follow, pull in objects from unlisted packs to
close the set under reachability.
When --stdin-packs=follow is not in use and delta search is disabled
with --window=0 or --depth=0, neither purpose applies. Skip the walk
in that case.
Skipping the walk alone does not eliminate all of its cost. While
reading the listed packs, add_object_entry_from_pack() unconditionally
seeds the walk with every commit via add_pending_oid(). Even when the
walk will not run, this parses those commits and performs otherwise
unnecessary object lookups. If the commits are not covered by a
commit-graph or multi-pack-index and the listed packs do not sort to
the front, those lookups can map every pack index in the object store
and exhaust the system's per-process limit on memory mappings.
Gate the seeding on the same condition as the walk. On a repository
with about 35 million objects, packing 14,174 objects from 316 packs
drawn from a store of 1,427 packs gave:
variant time max RSS
----------------------------- ------- --------
seed and walk (before) 176s 8.2 GiB
skip walk, still seed 0.38s 154 MiB
skip walk and seeding (after) 0.31s 66 MiB
In the worst case, skipping the seeding also reduced distinct pack
index mappings from:
1427 mmaps -> 316 mmaps
The output pack was byte-identical in all three runs.
Assisted-by: Claude Opus 4.7
Signed-off-by: Elijah Newren <newren@gmail.com>
When `git pack-aggregate` rolls up small pushed packs with `pack-objects --stdin-packs --window=0`, it tries to reuse existing on-disk deltas rather than searching for new ones. However, the same object may appear as a base in some packs and as deltas in others. When the base copy is seen first, its delta twin is discarded, and the object is written out as a base even though a perfectly reusable delta was sitting in a sibling pack. A subsequent geometric/full repack then has to delta-search that object from scratch, losing the reuse the aggregate could have preserved. Measured on a ~37M-object repository, aggregating 7253 candidate packs turned 25,310 delta-capable objects into bases purely from this first-seen selection. Add a `--prefer-reused-deltas` option (only meaningful with `--stdin-packs`) that prefers recording the delta representation of an object over a base one, when possible. Only the object header is read to classify each copy, so the extra cost is small. On the same repository the option recovered 20,547 of the 25,310 lost deltas (81%), and the resulting pack verifies cleanly. Assisted-by: Claude Opus 4.8 & GPT-5.6 Sol Signed-off-by: Elijah Newren <newren@gmail.com>
--prefer-reused-deltas points an object's entry at a delta copy found in another included pack when the copy first recorded was a plain base. When the object it would become a delta against is itself already recorded as a delta against this same object, the switch creates a two-object delta cycle, which a later break_delta_chains() pass then has to cut. Cutting such a cycle keeps exactly one of the two deltas, and so does simply not forming it, so declining the switch does not change how many deltas survive for the pair. It does avoid the wasted work of building a cycle only to break it, and keeps the result from depending on the order in which break_delta_chains() happens to cut. Resolve the candidate delta's base cheaply (headers only) by exporting the existing get_delta_base_oid() and wrapping it in a small pack_entry_delta_base() helper, then skip the switch when that base is already a delta against the object we are about to convert. Whichever member of the pair is reached first keeps its delta and the other stays a base. Assisted-by: Claude Opus 4.8 Signed-off-by: Elijah Newren <newren@gmail.com>
Repositories that have gone without maintenance can accumulate large
numbers of loose objects and small packfiles. Unrelated Git commands
then pay the cost of scanning the resulting sprawl in `objects/` and
`objects/pack/`, even when those commands would otherwise be quick.
Introduce a new plumbing builtin, `git pack-aggregate`, as a cheap
cleanup pass. With `--once`, it performs two steps:
1. Bundle local loose objects into a new pack with `pack-objects
--window=0 --mark-bad-deltas --delta-base-offset
--no-write-bitmap-index`, then unlink the loose copies.
2. Aggregate small local packs, excluding packs in the
multi-pack-index and packs carrying `.keep`, `.promisor`,
`.mtimes`, or `.bitmap` sidecars, then unlink the source packs.
Both steps copy each object's existing on-disk representation without
delta search or recompression. The output carries a `.baddeltas`
marker so a later thorough repack knows to reconsider its intra-pack
deltas. The loose-object pack from the first step is normally folded
into the output of the second.
This gives repositories with many loose objects or small packs a way
to return quickly to a healthier state. A later `git repack` can then
perform the more expensive work needed to optimize deltas and pack
layout.
Make this a separate builtin rather than another `git repack` mode. Its
contract is deliberately narrow and cheap, unlike repack's range of
optimization modes. A later integration will run both processes at
once; distinct command names make their roles clear in process listings
and traces, while keeping the aggregator's looping and coordination
options out of repack's already broad option and configuration surface.
Use `pipe_command()` and run-command's child-cleanup machinery to avoid
I/O deadlocks and orphaned `pack-objects` processes.
Delete only the consumed pack's `.pack`, `.idx`, `.rev`, and
`.baddeltas` files after rechecking its protective sidecars.
`unlink_pack_path()` is not suitable here: its non-force mode protects
only `.keep` and otherwise also removes `.bitmap`, `.promisor`, and
`.mtimes` sidecars that cause aggregation to leave a pack alone.
Like `git repack`, `git pack-aggregate` takes no locks of its own. Do
not run it concurrently with other pack-rewriting maintenance.
Assisted-by: Claude Opus 4.7 & GPT-5.6 Sol
Signed-off-by: Elijah Newren <newren@gmail.com>
A pack-aggregate run rolls many small packs up into one aggregator output and writes a sibling `.baddeltas` marker, since the new pack reuses raw delta-chains from its inputs without any cross-input delta search. That marker tells future `pack-objects` runs over the pack to treat its existing deltas as stale and recompute new ones. The aggregator deliberately does not install a `.keep` marker on its output: we want the next opportunity to re-delta its contents to roll up the pack like any other. But a `git repack --geometric=N` pass walks the packs sorted ascending by object count and only rolls up packs below the geometric split; anything already at the heavy end of the progression sits above the split and is kept as-is. Aggregator outputs are commonly sized to land there, so a single aggregator pass can leave a `.baddeltas` pack camped above the split for many subsequent geometric passes -- long enough that the unrelated bad-delta marker is the only signal that the pack's deltas are stale, and that signal gets ignored. Teach `split_pack_geometry()` to do a post-pass demotion of any `.baddeltas` packs from the kept region into the rollup region. The demotion preserves the ascending sort order of the kept region so that `get_preferred_pack()` continues to return the largest local kept pack. Demotion is unconditional: a `.baddeltas` marker is an explicit "these deltas need redoing" tag from a previous repack step, and the next geometric pass is the cheapest place to honor it. `.keep`-marked packs are still excluded from the rollup by the existing `pack_kept_objects` / `remove_redundant_packs` paths, so a user who really wants to pin a `.baddeltas` pack in place has that escape hatch. Promisor packs are intentionally not touched: the aggregator skips them, and rolling a promisor pack up into a normal pack would silently strip its "objects may live remotely" semantics. Add two tests to t5336: one that confirms a `.baddeltas` pack above the natural split is rolled up and its marker disappears from the resulting pack, and a control test that confirms a non-`.baddeltas` pack above the split is still preserved. Assisted-by: Claude Opus 4.7 & GPT-5.6 Sol Signed-off-by: Elijah Newren <newren@gmail.com>
`git pack-aggregate` is meant to cheaply roll up small push packs and
loose objects that accumulate during a long maintenance pass, so that
unrelated operations do not pay readdir cost in a sprawling object
directory. Its candidate selection, however, only filtered on MIDX
membership, protective sidecars, and `.idx` presence -- it had no notion
of pack size, so any eligible pack was rolled up no matter how large.
That is a poor fit for the intended use. A large repository can carry
very large packs that are not in the multi-pack-index even when it is
being maintained as aggressively as possible; rolling those up rewrites
tens of millions of already-packed objects for little file-count
benefit.
Add a `--max-objects=<n>` cap (and matching `pack.aggregateMaxObjects`
config, default 100000, 0 to disable) that leaves any pack with more
than <n> objects untouched. We gauge a pack's heft by object count
rather than byte size because the work we want to bound -- enumerating
objects and rebuilding the output index -- scales with object count.
A pack containing a few enormous blobs can therefore fall below the
cap despite being large in bytes, but such packs are uncommon in the
target workload and aggregation copies their existing representation
without delta search or recompression.
Estimate the object count from the size of the pack's `.idx` file,
which is linear in the number of objects for the v2 index format:
size = 8 + 1024 + N*(rawsz + 8) + 2*rawsz (+ 8 per large offset)
We already `lstat()` the `.idx` to confirm it exists, so reusing that
`st_size` is much faster than opening, mapping, and reading each
candidate index. The tempting alternative, `p->num_objects`, is only
populated by `open_pack_index()`, which mmaps the index; on repositories
with an enormous number of packs, mapping every candidate merely to
apply this gate can also exhaust `vm.max_map_count`.
Ignoring the rare large-offset table makes the estimate slightly high,
which errs toward skipping a borderline pack; that is the safe direction
here. Packs already in the MIDX remain excluded regardless of the cap,
so the new limit only affects packs outside it.
Assisted-by: Claude Opus 4.8 & GPT-5.6 Sol
Signed-off-by: Elijah Newren <newren@gmail.com>
pack-aggregate currently feeds every eligible loose object to a single
pack-objects process. That is acceptable for the normal case, but an
extremely neglected repository can have millions of loose objects. Such a
repository remains burdened by the full loose-object fanout until that one
large pack is complete, and the pack-objects process must hold bookkeeping
for the entire backlog at once.
Add --max-loose-objects=<n> and pack.aggregateMaxLooseObjects to split the
loose-object backlog into bounded tranches. Install each completed pack
and unlink its loose copies before starting the next tranche, allowing the
repository to improve incrementally and preserving completed work if the
process is interrupted.
Create a temporary marker in the object directory and use its filesystem
mtime as the cycle cutoff. Rescans ignore objects that are not older than
that marker, so concurrent writers cannot keep a cycle running
indefinitely. Using a marker on the same filesystem also avoids comparing
file mtimes against a potentially different host clock.
When more than one loose-object tranche is needed, leave all of those
output packs out of the pack-aggregation phase of the same cycle. This
avoids immediately copying the entire recovered loose-object backlog a
second time. A single loose-object output retains the existing behavior
and can be combined with other small packs immediately.
Default the limit to 100000. On a synthetic repository containing ten
million small loose blobs, the following limits produced:
limit first pack total time peak RSS
100000 12.4s 1520.0s 26MB
500000 64.1s 1530.8s 115MB
1000000 132.6s 1537.0s 226MB
The total time was effectively unchanged, while 100000 made the first
improvement visible much sooner and used substantially less memory. A
value of 0 disables tranche splitting.
Assisted-by: GPT-5.6 Sol
Signed-off-by: Elijah Newren <newren@gmail.com>
pack-aggregate folds all aggregatable packs into a single output pack. On a normally-maintained repository that is fine, but repositories can accumulate an extreme number of packs -- tens or even hundreds of thousands -- before maintenance catches up. Folding that many packs into one output pack forces the pack-objects run to keep the .idx, .pack, and .rev files of every input pack mmapped at once (three mappings per pack), which can exceed the vm.max_map_count limit and fail outright. Add a --max-packs=<n> option (and a pack.aggregateMaxPacks config) that caps how many packs are folded into each output pack. When more than <n> candidates are present, split them into several evenly-sized batches and roll each batch up into its own output pack. For example, 12000 packs with --max-packs=5000 are rolled up as three output packs of 4000 -- not 5000, 5000, and 2000 -- bounding the mmap count of any single pack-objects run. Split on whole-pack boundaries. Because on-disk packs are always self-contained (git never stores thin packs), a delta and its base stay together in the same batch, so each batch can reuse the existing deltas as-is without having to reason about how to segregate the objects within a pack. Default --max-packs to 10000; use a value of 0 to fold all aggregatable packs into one output pack. Assisted-by: Claude Opus 4.8 & GPT-5.6 Sol Signed-off-by: Elijah Newren <newren@gmail.com>
Repositories that have gone without maintenance can accumulate enough loose objects and small packs to slow down every Git command that needs to inspect the object store. A thorough repack will eventually repair that state, but it must first operate while the repository is still at its least healthy. Teach `git repack` to optionally run `git pack-aggregate --once` before it inspects the existing packs and loose objects. This cheap preliminary pass quickly reduces the number of files the main repack must contend with, leaving the thorough repack to optimize deltas and pack layout. Add `--aggregate-once` and `--no-aggregate-once`, together with the matching `repack.aggregateOnce` configuration variable. Keep the behavior disabled by default. Run the aggregation pass before repack snapshots refs for a MIDX bitmap, reads the existing MIDX, or computes geometric pack selection. If the aggregation fails, abort before starting the main pack-objects process. Since aggregation mutates the repository, validate incompatible `--geometric` and `-a`/`-A` options, and `--filter-to` without `--filter`, before invoking it. Invalid commands therefore remain side-effect free. Like `git repack` and `git pack-aggregate` themselves, this integration takes no locks. Serialization with other pack-rewriting maintenance remains the caller's responsibility. Signed-off-by: Elijah Newren <newren@gmail.com>
If a process desires to work on packs other than what pack-objects may preserve or consume, there is currently no way to identify a safe, disjoint set. Callers cannot simply enumerate packs before calling pack-objects, because that leaves a race between their enumeration and pack-objects' enumeration; large repositories push often enough to make losing that race inevitable. Address this with `--emit-input-packs`, which writes a conservative snapshot of every local pack visible after pack-objects prepares its packing data. Emitting all local packs rather than only those selected for rewriting is intentional. In particular, a geometric repack leaves the upper portion of its pack geometry untouched but may include those packs in its replacement MIDX. A concurrent pack aggregator must therefore leave those packs alone too, or the new MIDX could reference a pack that the aggregator retired. Similarly, add `--emit-input-loose` to write the local loose objects visible at the same point. An external caller could enumerate loose objects itself: losing that race would merely cause an object to appear temporarily in multiple packs, since deleting one loose copy does not prevent pack-objects from writing another packed copy. Emitting the snapshot from pack-objects nevertheless reduces that duplication. Both options write to `<file>.tmp` first and then rename, so the file appears atomically and a reader either sees the previous contents (if any) or the complete new contents, never a partial write. Since these options exist only to support the handshake between repack and the upcoming pack-aggregate builtin, and are not meant for end users, they are currently undocumented. Assisted-by: Claude Opus 4.7 & GPT-5.6 Sol Signed-off-by: Elijah Newren <newren@gmail.com>
The `--once` mode provides a quick manual cleanup pass, but a long-running repack leaves time for new loose objects and small push packs to accumulate while it works. Repeated aggregation can keep that new material under control without interfering with the larger repack, provided the two processes operate on disjoint inputs. Add a `--loop` mode that runs aggregation passes repeatedly, sleeping `--interval` seconds between them. Add exclusion files for packs and loose objects so a concurrent `pack-objects` can declare the inputs it owns; the aggregator skips those inputs on every pass. Add `--parent-pipe-fd` so a parent process can keep the aggregator alive by holding a pipe open. Poll the pipe during the interval so the aggregator exits promptly when its parent goes away. Handle termination signals by recording a stop request so the current cycle can unwind and the loop can exit at a safe boundary. An in-flight `pack-objects` is already marked `clean_on_exit`, so run-command's standard signal cleanup forwards the signal to it before chaining to the loop's handler. This stops the child promptly while allowing the loop to unwind in an orderly way. Assisted-by: Claude Opus 4.7/4.8 & GPT-5.6 Sol Signed-off-by: Elijah Newren <newren@gmail.com>
On a busy server hosting a large repository, `git repack` routinely
takes long enough for thousands of packs and tens of thousands of loose
objects to accumulate before it finishes. Every subsequent Git
operation pays the price: each pack lookup walks the longer list, and
each loose-object miss has to consult more sources.
Wire `git repack` to optionally drive `git pack-aggregate` as a
background subprocess, controlled by `repack.aggregateLoop` or the
equivalent `--aggregate-loop` and `--no-aggregate-loop` command-line
options. Keep the behavior disabled by default and independent of the
one-shot aggregation pass.
When enabled:
* Create a small tempdir under `objects/` for two exclude files.
* Pass `--emit-input-packs=<tmp>/packs --emit-input-loose=<tmp>/loose`
to the main pack-objects so it records the local packs and loose
objects visible when it prepares its inputs.
* Launch `git pack-aggregate --loop --exclude-pack-file=<tmp>/packs
--exclude-loose-file=<tmp>/loose` once the emit files exist.
* Drop a `.keep` marker on every pack written by the main repacking
work, so the aggregator does not roll it up.
Together these protections cover the parent's MIDX transition.
pack-aggregate independently skips packs named by the current MIDX; the
emitted pack list protects the parent's existing-pack snapshot, while
the temporary `.keep` files protect its new packs before their indexes
become visible and remain through the MIDX write. The explicit MIDX
include list is drawn from those protected sets, so the aggregator
cannot retire a pack that the parent is about to reference.
Mark unrelated pipe ends close-on-exec before launching pack-aggregate.
Without these flags, an inherited geometric pack-objects input writer
could prevent it from seeing EOF and deadlock the repack, while an
inherited parent-pipe writer could prevent pack-aggregate from detecting
that its parent exited. If either flag cannot be set, skip
pack-aggregate and clean up any newly-created pipe.
pack-aggregate keeps running through the cruft pack-objects subprocess
and through `write_midx_included_packs()`; it is SIGTERM'd only when it
comes time to delete redundant packs and objects.
Finally, unlink the recorded `.keep` markers after pack-aggregate has
been shut down.
`git repack` itself takes no locks, and neither does the aggregator we
spawn; serialization across maintenance runs is the caller's
responsibility (typically `git gc` or a server-side maintenance driver).
This matches the prior behavior of `git repack`.
Assisted-by: Claude Opus 4.7 & GPT-5.6 Sol
Signed-off-by: Elijah Newren <newren@gmail.com>
Both `git repack` and `git pack-aggregate` stage a new pack as ".tmp-<pid>-pack-<hash>.*" and rename it to "pack-<hash>.*" only once it is fully written (the .idx last). In combined mode the two run concurrently, so one process's in-flight staging files are visible to the other. The object-store scan treats any "*.idx" as a pack regardless of prefix, while the aggregator's exclude list and ".keep" markers key on the final "pack-<hash>" name; nothing then stops a temp pack from being picked up, and pack-aggregate deletes it out from under its owner while geometric repack feeds it to pack-objects only for it to vanish mid-run. Require the canonical "pack-<hash>" basename where packs are selected -- in collect_pack_candidates() and in the geometry roll-up -- rather than in the object-store scan, since `git repack -ad` legitimately enumerates stale ".tmp-*" packs in order to delete them. Signed-off-by: Elijah Newren <newren@gmail.com>
A pack-aggregate cycle that finds nothing to do leaves no on-disk artifact, so process lifetime and repository state are insufficient to count cleanup cycles. Add trace2 regions around each pack-aggregate cycle and repack's redundant-pack removal so trace consumers can account for these operations, including no-op aggregation cycles. Signed-off-by: Elijah Newren <newren@gmail.com>
MIDX verification closes packs between object groups and later reopens them by name. A concurrent repack can remove one in that interval, causing verification to fail on an otherwise benign race. When the referenced packs fit within a conservative fd budget, open them before verifying offsets and keep them open through the walk. Most geometrically maintained MIDXes have only O(log N) packs; larger MIDX chains retain the close-as-we-go path and its race-aware diagnostic. Do not use do_not_close for this: find_lru_pack() ignores it, so close_one_pack() may still reclaim the fd under pressure. Assisted-by: Claude Opus 4.8 Signed-off-by: Elijah Newren <newren@gmail.com>
newren
force-pushed
the
maintenance-inception
branch
from
September 9, 2026 06:45
24bdb43 to
07ca9ca
Compare
|
There are merge commits in this Pull Request: Please rebase the branch and force-push. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
During the time between when a repack starts and when it completes,
large repositories can gain thousands of packfiles and tens of thousands
of loose objects that are not part of that repack. Not only does this
mean that the repository immediately needs maintenance again, but git
operations slow down dramatically during the repack -- sometimes to the
point that the service is effectively considered offline.
This series proposes a simple (optional, off by default) solution:
or, more specifically:
during (far more thorough) maintenance (of older objects and packs).
=== Concerns/Objection ===
The description above probably raises several serious concerns; let me
attempt to address them (feedback on whether I've missed any, or
insufficiently addressed some very welcome):
Objection 1: What does "cheap mere aggregation" mean?
It means omitting all of the following:
Objection 2: Aggregating packs without delta searches causes bad deltas
Yes, but this series adds a .baddeltas marker file (similar to .keep,
.mtimes, .bitmap, .promisor that already exist) to notify future
pack-objects invocations that two objects within that pack should be
checked for deltas, so the effect does not persist.
(On the flip side, see Trade-off 1)
Objection 3: Repack or its subprocess can fail if packfiles disappear
repack calls pack-objects, which enumerates the existing packs, and
only repacks those (or a subset of those if doing a geometric repack),
and then writes a new pack. The remainder of repack (e.g. multi-pack
index creation, bitmap generation) -- at least up until the
odb_reprepare() call immediately before deleting redundant packs --
only works on this subset of packs. Thus, if we have pack-objects
tell us the packs it is working on and we avoid touching any packs:
we throw in '.promisor' and '.bitmap' for good measure)
Then we should be safe; this does require modifying pack-objects to:
(We also have pack-objects notify us which loose objects exist at that
time, even though that is not needed for correctness, but does allow
us to generally avoid putting a loose object into two new packs.)
Objection 4: The .keep created by pack-objects might persist
It is removed at the end of the repack process.
Further, if for some reason the repack process is killed or dies, the
.keep file contains a marker making it clear the keep was meant to be
temporary and has the corresponding PID of the repack process, meaning
that if that PID no longer exists, the .keep file can be cleaned up.
See also the next point.
Objection 5: Will this "cheap" maintenance actually be cheap?
There's an unnecessary reachability walk today which makes this kind
of "cheap" maintenance somewhat expensive. Patch 3 removes it and
makes a typical case 500x faster and take 50x less memory.
That's fast enough that I not only run "cheap" maintenance during
"real" maintenance, I also run a "cheap" maintenance step before "real"
maintenance. The reasons for this inserted preparatory step are:
the "real" maintenance to complete
to be tracked for exclusion
previous repack process that died early and remove them
=== Important tradeoffs ===
Trade-off 1: need to repeat some delta checks
Two objects being in the same pack that aren't a delta against each
other usually means a previous process has checked and determined
those two objects are not a good delta. Any such objects placed into
a .baddeltas pack will lose that information and we'll have to
re-check for deltas.
Trade-off 2: mtime delay for expiring objects
When we repack loose objects into a (non-cruft) pack, that effectively
updates their mtime when they could have been ready to expire. If
these objects are unreachable, then the "real" maintenance's
reachability check will determine that and either unpack these objects
or move them to a cruft pack. Either way, they'll be stamped with the
pack's mtime they were removed from. For large repositories where we
essentially continuously run maintenance all day long, this can only
add a delay equal to how long a maintenance run takes and is thus
considered inconsequential.
=== Series overview ===
Patch 1: Add support for a ".baddeltas" marker for packs, in try_delta()
Patch 2: Support --mark-bad-deltas option in pack-objects
Patch 3: Accelerate pack-objects when delta search is off
Patch 4: Add --emit-input-{packs,loose} plumbing options to pack-objects
Patch 5: New pack-aggregate builtin for cheap object/pack aggregation
Patch 6: Allow repack to spawn pack-aggregate while doing regular repacking