Re: Keeping unreachable objects in a separate pack instead of loose?

[Nicolas Pitre <nico@fluxnic.net>]

On Tue, 12 Jun 2012, Jeff King wrote:

> On Tue, Jun 12, 2012 at 10:45:22AM -0700, Shawn O. Pearce wrote:
> 
> > > Then those objects will remain in the cruft pack. Which is why, as I
> > > said, it is not generally safe to just delete a cruft pack. However,
> > > when you do a full repack, those objects will be copied into the new
> > > pack (because they are referenced). Which is why I am claiming that it
> > > is safe to remove cruft packs at that point.
> > 
> > But there is a race condition with a concurrent fetch and a concurrent
> > repack. If that fetch needs those cruft objects, and sees them in the
> > cruft pack, and the repack sees the references before the fetch, the
> > repacker might delete things the fetch is about to reference and that
> > will leave you with a corrupt repository.
> > 
> > I think we already have this race condition with loose unreachable
> > objects whose mtimes are older than 2 weeks; they are removed by prune
> > but may have just become reachable by a concurrent fetch that doesn't
> > overwrite them because they already exist, and doesn't update the
> > mtime because they aren't writable.
> 
> Correct. There is a race condition, but it is there already. I have
> discussed this with other GitHub folks, because we prune fairly
> aggressively (in our case it would be a push, not a fetch, of course).
> So far we have not had any record of it actually happening in practice.
> 
> We could close it in both cases by tweaking the mtime of the file
> containing the object when we decide not to write because the object
> already exists.

Yes, that is a worthwhile thing to do.


Nicolas