Re: fetching packs and storing them as packs

From: Junio C Hamano <junkio@cox.net>
Date: Oct 28, 2006, 05:42 UTC
Message-ID: <7vwt6l9etn.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <Pine.LNX.4.64.0610272109500.3849@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

Show 20 quoted lines

> On Fri, 27 Oct 2006, Shawn Pearce wrote:
>> 
>> So a reader-writer lock is preferred over
>> a non-locking solution such as I posted in
>> http://article.gmane.org/gmane.comp.version-control.git/30288 ?
>> 
>> Not to mention that such a solution would also fix the -d issue
>> Linus points out above.
>
> Be very careful.
>
> There's a good reason why git doesn't use locking, and tends to use the 
> "create file exclusively and move over the old version after having tested 
> that the old version is still relevant" approach.
>
> Two _major_ issues:
>...
>
> I would MUCH rather we just rename the index/pack file to something that 
> git can _use_, but that "git repack -a -d" won't remove....

Two points.

The "locking" I mentioned was between receive-pack and repack -a -d; upload-pack (what millions people are using to read from the repository you are pushing into) is not affected. So in that sense, we can afford to use lock without much contention.

I just thought of a cute hack that does not involve renaming packs at all (so no need to match new-pack-X.pack with pack-X.idx), and Shawn's sequence actually would work, which is:

The receive-pack side:

  a. Create temporary pack file in $GIT_DIR/objects/pack_XXXXX.
  b. Create temporary index file in $GIT_DIR/objects/index_XXXXX.
  c. Write pack and index, in "inactive" state.
  d. Move pack to $GIT_DIR/objects/pack/...
  e. Move idx to $GIT_DIR/objects/pack...
  f. Update refs.
  g. Mark new pack and idx as "active".

The "repack -a -d" side:

  1. List all active packs and store in memory.
  2. Repack only loose objects and objects contained in active packs.
  3. Move new pack and idx into $GIT_DIR/objects/pack/...
  4. Mark new pack and idx as "active".
  5. Delete active packs found by step #1.

Pack-idx pair is marked "active" by "chmod u+s" the .pack file. During the normal operation, all .pack/.idx pair in objects/pack/ directories are usable regardless of the setuid bit; we would never make .pack files executable so u+s would not otherwise hurt us either. "active" probably is better read as "eligible for repacking".