repository corruption when pushing commits to a repository running 'git gc --prune='

repository corruption when pushing commits to a repository running 'git gc --prune='

[Jan Smets]

Hi

I've recently expired my reflog to prune loose objects. On a live, bare, 
repository I ran 'git gc --prune=now'

All clients ended up having problems, they would report:
  error: refs/heads/master does not point to a valid object!
Running 'git log' on the bare repo gave : fatal: bad object HEAD



So I did a couple of tries reproducing the issue and with this little 
test I got a couple of interesting different/similar outcomes

A: mkdir -p /tmp/test; cd /tmp/test ; git init --bare bare ;
B: cd /tmp/test ; git clone bare 1 ; cd 1;  touch test; git add test; 
while true; do date >> test ; git commit -m "$(date)" test ; git push || 
break; done
A: cd bare; git gc --prune=now  (or run it in a loop)


1)

fatal: bad object 22f0351258fa0bb4cd28984b6473510957fbce69
fatal: bad object 22f0351258fa0bb4cd28984b6473510957fbce69
To /tmp/test/bare
  ! [remote rejected] master -> master (missing necessary objects)

2)

remote: error: unable to write sha1 filename 
objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed: No such file or directory
remote: fatal: failed to write object
error: unpack failed: unpack-objects abnormal exit
To /tmp/test/bare
  ! [remote rejected] master -> master (unpacker error)

3)

error: Ref refs/heads/master is at 
e992810e70949e797d33041bf6bc961c9fa4f3e5 but expected 
0000000000000000000000000000000000000000
remote: error: Cannot lock ref 'refs/heads/master':
To /tmp/test/bare
  ! [remote rejected] master -> master (failed to update ref)

4)

remote: error: cannot lock ref 'refs/heads/master': Unable to create 
'/tmp/test/bare/refs/heads/master.lock': File exists.
remote:
remote: If no other git process is currently running, this probably means a
remote: git process crashed in this repository earlier. Make sure no 
other git
remote: process is running and remove the file manually to continue.
To /tmp/test/bare
  ! [remote rejected] master -> master (failed to update ref)
error: failed to push some refs to '/tmp/test/bare'


And eventually running 'git gc --prune=now/all' on the bare repository 
could end with this message:

$ git gc --prune=all
error: refs/heads/master does not point to a valid object!
fatal: bad object refs/heads/master
error: failed to run repack
$ git log -1
fatal: bad object HEAD


This behaviour has been observed with version 2.4.4, 2.5.[12] and 2.6.1

Thank you

- Jan

Re: repository corruption when pushing commits to a repository running 'git gc --prune='

[Matthieu Moy]

Jan Smets <jan.smets@alcatel-lucent.com> writes:

> Hi
>
> I've recently expired my reflog to prune loose objects. On a live,
> bare, repository I ran 'git gc --prune=now'
>
> All clients ended up having problems, they would report:
>  error: refs/heads/master does not point to a valid object!
> Running 'git log' on the bare repo gave : fatal: bad object HEAD
[...]
> fatal: bad object 22f0351258fa0bb4cd28984b6473510957fbce69
> fatal: bad object 22f0351258fa0bb4cd28984b6473510957fbce69
> To /tmp/test/bare
>  ! [remote rejected] master -> master (missing necessary objects)

I think this is the expected behavior. push will create new objects that
are not referenced until the ref is updated (at the very end). prune can
run concurrently since creating and deleting objects is done in a
lockless way (only the ref update needs a lock).

Still, this is not the *documented* behavior, and an easy way to corrupt
a repo should be very explicitly documented as very dangerous, and the
precautions to take when using it should be explained clearly.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: repository corruption when pushing commits to a repository running 'git gc --prune='

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> I think this is the expected behavior. push will create new objects that
> are not referenced until the ref is updated (at the very end). prune can
> run concurrently since creating and deleting objects is done in a
> lockless way (only the ref update needs a lock).
>
> Still, this is not the *documented* behavior, and an easy way to corrupt
> a repo should be very explicitly documented as very dangerous, and the
> precautions to take when using it should be explained clearly.

Yeah, I think this paragraph in the user-manual is the only thing
that mentions it:

    Anyway, once you are sure that you're not interested in any dangling
    state, you can just prune all unreachable objects:

    ------------------------------------------------
    $ git prune
    ------------------------------------------------

    and they'll be gone. (You should only run `git prune` on a quiescent
    repository--it's kind of like doing a filesystem fsck recovery: you
    don't want to do that while the filesystem is mounted.

Something along this line, perhaps?

 Documentation/git-gc.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 5223498..fa15104 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -63,8 +63,11 @@ automatic consolidation of packs.
 --prune=<date>::
 	Prune loose objects older than date (default is 2 weeks ago,
 	overridable by the config variable `gc.pruneExpire`).
-	--prune=all prunes loose objects regardless of their age.
-	--prune is on by default.
+	--prune=all prunes loose objects regardless of their age (do
+	not use --prune=all unless you know exactly what you are doing.
+	Unless the repository is quiescent, you will lose newly created
+	objects that haven't been anchored with the refs and end up
+	corrupting your repository).  --prune is on by default.
 
 --no-prune::
 	Do not prune any loose objects.

Re: repository corruption when pushing commits to a repository running 'git gc --prune='

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
> index 5223498..fa15104 100644
> --- a/Documentation/git-gc.txt
> +++ b/Documentation/git-gc.txt
> @@ -63,8 +63,11 @@ automatic consolidation of packs.
>  --prune=<date>::
>  	Prune loose objects older than date (default is 2 weeks ago,
>  	overridable by the config variable `gc.pruneExpire`).
> -	--prune=all prunes loose objects regardless of their age.
> -	--prune is on by default.
> +	--prune=all prunes loose objects regardless of their age (do
> +	not use --prune=all unless you know exactly what you are doing.
> +	Unless the repository is quiescent, you will lose newly created
> +	objects that haven't been anchored with the refs and end up
> +	corrupting your repository).  --prune is on by default.

Looks good to me. I think the same should be added to git-prune.txt
under --expire.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: repository corruption when pushing commits to a repository running 'git gc --prune='

[Matthieu Moy]

Jan Smets <jan.smets@alcatel-lucent.com> writes:

> 2)
>
> remote: error: unable to write sha1 filename
> objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed: No such file or
> directory
> remote: fatal: failed to write object

If I read correctly, this happens when we move the temp file to its
permanent position.

This looks like a race between deleting objects/05/ and creating
objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed.

I don't understand how this is possible, since the temporary file and
the final one are in the same directory, so deleting the directory
should fail.

What am I missing?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: repository corruption when pushing commits to a repository running 'git gc --prune='

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> This looks like a race between deleting objects/05/ and creating
> objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed.
>
> I don't understand how this is possible, since the temporary file and
> the final one are in the same directory, so deleting the directory
> should fail.
>
> What am I missing?

 1. tmp_obj_* gets created in objects/05/ by "git unpack-objects";

 2. prune_cruft() comes, notices tmp_obj_* that is sufficiently old,
    decides to remove in "git prune".

 3. prune_subdir() comes, notices objects/05/ is now empty, does
    rmdir().

 4. "git unpack-objects" closes the file whose inode was originally
    associated with the path objects/05/tmp_obj_*;

 5. "git unpack-objects" calls finalize_object_file() and tries to
    link(2) or rename(2) it to its final name.  It does not find the
    source of the link or rename and says "oops, no such file".

Re: repository corruption when pushing commits to a repository running 'git gc --prune='

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> This looks like a race between deleting objects/05/ and creating
>> objects/05/cdb51bb0ea3e229734a4b1bddd5ec70fbc65ed.
>>
>> I don't understand how this is possible, since the temporary file and
>> the final one are in the same directory, so deleting the directory
>> should fail.
>>
>> What am I missing?
>
>  1. tmp_obj_* gets created in objects/05/ by "git unpack-objects";
>
>  2. prune_cruft() comes, notices tmp_obj_* that is sufficiently old,
>     decides to remove in "git prune".

Ah, OK, the real problem is the temporary file removal. Thanks.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: repository corruption when pushing commits to a repository running 'git gc --prune='

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Looks good to me. I think the same should be added to git-prune.txt
> under --expire.

I thought about it, but decided against it, as the command is not
even recommended to end users.

Re: repository corruption when pushing commits to a repository running 'git gc --prune='

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Looks good to me. I think the same should be added to git-prune.txt
>> under --expire.
>
> I thought about it, but decided against it, as the command is not
> even recommended to end users.

Even non-"end users" can shoot themselves in the foot. Someone writting
a script using "git prune" may actually shoot in other people's foot ...

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: repository corruption when pushing commits to a repository running 'git gc --prune='

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>>> Looks good to me. I think the same should be added to git-prune.txt
>>> under --expire.
>>
>> I thought about it, but decided against it, as the command is not
>> even recommended to end users.
>
> Even non-"end users" can shoot themselves in the foot. Someone writting
> a script using "git prune" may actually shoot in other people's foot ...

Sure, patches welcome.

To be honest, I felt that

    --expire <time>::
            Only expire loose objects older than <time>.

is sufficiently clear for these people.