Thinking about this some more, it is clear that running `git gc` / `git repack` *and* `git maintenance` simultaneously is prone to badness since their locking mechanisms aren't aware of the other. I think it makes sense to consolidate onto `git maintenace` going forward as this seems to be where the inertia is. However, a barrier to that for me is the objects/maintenance.lock file has no acquisition timeout and will wait indefinitely. So in scenarios like mine where you have multiple processes looping over repos invoking `git maintenance run`, you can have extended execution stalls during long-running operations like `gc`. There should probably be a configurable timeout for the maintenance lock like there is for other locks. This shouldn't be too much work and I may throw up a patch if others feel this is a good idea. On Thu, Jun 30, 2022 at 1:12 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Taylor Blau <me@xxxxxxxxxxxx> writes: > > > On Wed, Jun 29, 2022 at 11:19:09PM -0400, Taylor Blau wrote: > >> > However, I think there is yet another bug at play: running > >> > `incremental-repack` appears to be able to repack the cruft packfile. > >> > In doing so, it deletes its .mtimes file and the metadata inside. > >> > >> That sounds like a bug to me. I'll take a look into it and see what I > >> can find. > > > > I actually think that there are two bugs here. > > > > One is that we run a MIDX repack and expire, which could lead to us > > repacking the entire contents of the cruft pack and then expiring the > > metadata file. This is a bug, and we should exclude cruft packs from > > this computation. > > > > Another bug can happen when the cruft pack gets written into the MIDX > > and is MIDX-expireable (meaning that no objects are selected from the > > cruft pack). In that case, the `git multi-pack-index expire` step would > > remove the cruft pack entirely, which is also incorrect. > > > > I'll take a look at fixing both of these, and thanks for pointing them > > out! > > Thanks, both. > > The fact that the semantics of the .mtimes file being not equivalent > to the mtime on individual loose objects does not help thinking > about the possible ways the "cruft" pack can break, and both of the > possible issues you discuss above are indeed tricky ones. > >
