Re: gc considered dangerous

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Feb 8, 2009, 17:18 UTC
Message-ID: <alpine.DEB.1.00.0902081815170.10279@pacific.mpi-cbg.de>
In-Reply-To: <200902081704.59439.robin.rosenberg.lists@dewire.com>

Hi,

On Sun, 8 Feb 2009, Robin Rosenberg wrote:

Show 13 quoted lines

> söndag 08 februari 2009 15:56:38 skrev Johannes Schindelin:
> 
> > On Sun, 8 Feb 2009, Robin Rosenberg wrote:
> > 
> > > I've seen this. Running git gc on Windows, while having Elipse open 
> > > can kill your object database.
> > 
> > You had me really scared with the mail subject!
> 
> Losing a repo *is* scary., especially if your boss is the one losing it. 
> That should simply not happen unless there is a disk failure. 
> Fortunately I believe the two lost branches did not contain anything 
> useful.

If you care about your data, you don't use Windows, IMHO.

Show 23 quoted lines

> > > if gc doesn't find a new optimal pack it tries to rewrite a new pack 
> > > with the same id. So it rm's the idx file (fine) and the the pack 
> > > file (not ok) and gives up,
> > 
> > I disagree with your notion that it is fine to kill the existing idx 
> > file until the new one has been written successfully.
>
> Then you misunderstood me. I meant that the repack script thought it was 
> fine.
> 
> > My preliminary guess is that this code in pack-write.c needs to use the 
> > lock file paradigm:
> 
> > 
> >         if (!index_name) {
> > 		[...]
> >         } else {
> >                 unlink(index_name);
> >                 fd = open(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
> >         }
> 
> Yeah, it needs to check that it can actually delete both files before 
> actually doing it.

Nope.  It needs to avoid unlinking until it knows what it did was fine.

> Then again if it wants to replace a file with a new one just like it,

It might have the same name, but that does not mean that it is bytewise identical. The name depends on the uncompressed objects.

> Isn't that the only case I see where we can lose data?

No, you said so yourself: Eclipse prevents the pack from being deleted. There are a gazillion idiotic programs on Windows who think they should lock random files.

So you could hit the very same issue even if your repository has twenty packs, and therefore could do with some gc'ing.

Ciao, Dscho