From: Shawn O. Pearce <spearce@spearce.org>
Date: Fri, 19 Jan 2007 03:44:04 GMT
Subject: Re: [RFC] prune: --expire=seconds
Message-ID: <20070119034404.GA17521@spearce.org>
In-Reply-To: <7vy7o0klt1.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> Matthias Lederhofer <matled@gmx.net> writes:
> 
> > This option specifies the minimum age of an object before it
> > may be removed by prune.  The default value is 2 hours and
> > may be changed using gc.pruneexpire.
> 
> I am not sure if this is needed, as Shawn explained earlier
> rounds of loose-objects safety work.

I think we need this fix.  We still have a race condition between
the loose object creation and the ref update.

We've closed this hole completely in the large push case (objects
>=receive.unpackLimit) and 'fetch -k' case by creating .keep files
before the .pack file, updating refs, then deleting the .keep file;
and by making sure git-repack leaves packs with .keeps alone.  So
we cannot lose an object here.

But update-index/add/merge-recursive/write-tree/commit-tree, etc.
as well as small pushes (objects <receive.unpackLimit) and fetch
without -k option still have a race condition.  The objects will
be created/unpacked into the loose objects directory with nothing
referencing them, and a prune which gets to run just before before
the ref update becomes visible would probably whack those objects.

Given that 'git gc' is the encouraged way to maintain a repository,
and that 'repack -a -d' is safe, and prune-packed is equally safe,
I think we should try to make prune safe too.  Matthias' patch
does this by giving the ref update process a fairly large window
to perform its action within.
 
> If this is something we would want, it might make sense if we
> allowed "prune --expire='1.day'" syntax ;-).

Yes, I agree.

Matthias you can take a look at builtin-reflog.c's argument handling
for an example.  I think you just need to use approxidate() in both
your config function and in your command line argument handling.
Then the default becomes '2.hours.ago' instead of just "2" (at
least from a documentation perspective).

Though the more I think about this perhaps the default should be
'1.day'.  24 hours is a hellva large window for any current ref
update to complete in, even if the ref update was some massive rsync
which is doing a such a large volume of data on a small bandwidth
link that it takes 20 hours to complete.  Besides, users could
always force it to be much lower with the command line option if
they really need to prune _right_now_.

-- 
Shawn.