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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html