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

Jeff King [off-list ref] writes:

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.

Though there is always the window between the existence check and the
mtime update where pruning can hit you.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

On Tue, Jun 12, 2012 at 08:43:41PM +0200, Andreas Schwab wrote:

Jeff King [off-list ref] writes:

quoted

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.

Though there is always the window between the existence check and the
mtime update where pruning can hit you.

For the loose object case, you could do them both atomically by calling
utime() on the object, and considering the object to exist only if it
succeeds.

Doing it safely for packs would be harder, though; I think you'd
have to bump the mtime forward, do the search, and then bump it back.
You might err by causing a pack not to be pruned, but that is better
than the opposite.

Unfortunately it gets trickier with network transfers. If somebody is
pushing to your repository, you might tell them you have some set of
objects, then they prepare a pack based on that assumption (which might
take minutes or hours to transfer), and then finally at the end you find
that you actually need the objects in question. Of course, that race is
even harder to trigger, because we do not advertise unreachable objects.
So you would have to have a sequence where the objects are reachable,
the client connects and receives your ref advertisement, then the
objects become unreachable (e.g., due to a simultaneous non-ff push or
deletion), and you do a prune in that interval which removes the
objects.  Unlikely, but still possible.

-Peff

On Tue, 12 Jun 2012, Andreas Schwab wrote:

Jeff King [off-list ref] writes:

quoted

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.

Though there is always the window between the existence check and the
mtime update where pruning can hit you.

This is a tiny window compared to 2 weeks.

This could be avoided entirely with:

1. check presence of object X

2. update its mtime

3. check presence of object X again

4. create if doesn't exist


Nicolas

On Tue, Jun 12, 2012 at 03:09:25PM -0400, Nicolas Pitre wrote:

quoted

Jeff King [off-list ref] writes:

quoted

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.

Though there is always the window between the existence check and the
mtime update where pruning can hit you.

This is a tiny window compared to 2 weeks.

I don't think the race window is actually 2 weeks long. If you have this
sequence:

  1. object X becomes unreferenced

  2. 1 week later, you create a new ref that mentions X

  3. >2 weeks later, you run "git prune --expire=2.weeks.ago"

we will not consider the object for pruning in step 3, because it is
reachable. The race is more like:

  1. object X becomes unreferenced

  2. >2 weeks later, you run "git prune --expire=2.weeks.ago"

  3. git-prune reads the list of refs

  4. simultaneous to the git-prune, you reference X

  5. git-prune removes X

  6. your reference is now broken

So the race window depends on the time it takes "git prune" to run.

I wonder if git-prune could do a double-check of the refs. Something
like:

  1. calculate reachability on all refs

  2. read list of objects to prune, and make a list of unreachable ones

  3. calculate reachability again (which should be very cheap, because
     you can stop when you get to an object you have already seen)

  4. Drop any objects found in (3) from the list in (2), and delete
     items from your list

But I think that still has a race where objects are created before
step 2, but are not actually referenced until after step 3. I think
doing it safely may actually require a repo-wide prune lock.

-Peff

On Tue, 12 Jun 2012, Jeff King wrote:

So the race window depends on the time it takes "git prune" to run.

I wonder if git-prune could do a double-check of the refs. Something
like:

  1. calculate reachability on all refs

  2. read list of objects to prune, and make a list of unreachable ones

  3. calculate reachability again (which should be very cheap, because
     you can stop when you get to an object you have already seen)

  4. Drop any objects found in (3) from the list in (2), and delete
     items from your list

But I think that still has a race where objects are created before
step 2, but are not actually referenced until after step 3. I think
doing it safely may actually require a repo-wide prune lock.

Yeah... that's what I was thinking too.  Maybe we're making our life 
overly miserable by trying to avoid any locking here.


Nicolas

On Tue, Jun 12, 2012 at 03:39:05PM -0400, Nicolas Pitre wrote:

quoted

So the race window depends on the time it takes "git prune" to run.

I wonder if git-prune could do a double-check of the refs. Something
like:

  1. calculate reachability on all refs

  2. read list of objects to prune, and make a list of unreachable ones

  3. calculate reachability again (which should be very cheap, because
     you can stop when you get to an object you have already seen)

  4. Drop any objects found in (3) from the list in (2), and delete
     items from your list

But I think that still has a race where objects are created before
step 2, but are not actually referenced until after step 3. I think
doing it safely may actually require a repo-wide prune lock.

Yeah... that's what I was thinking too.  Maybe we're making our life 
overly miserable by trying to avoid any locking here.

I think I would be OK with "prune" locking, as long as everything else
was able to happen simultaneously. Especially if we can keep prune's
lock as short as possible through double-reads or similar tricks (like
we do for ref updates).

-Peff