Re: [PATCH] repack: Add options to preserve and prune old pack files

[jmelvin@codeaurora.org]

On 2017-03-07 13:33, Junio C Hamano wrote:
> James Melvin <jmelvin@codeaurora.org> writes:
> 
...
> 
> I am not sure if I understand your design.  Your model looks to me
> like there are two modes of operation.  #1 uses "--preserve-old" and
> sends old ones to purgatory instead of removing them and #2 uses
> "--prune-preserved" to remove all the ones in the purgatory
> immediately.  A few things that come to my mind immediately:
> 
>  * When "--prune-preseved" is used, it removes both ancient ones and
>    more recent ones indiscriminately.  Would it make more sense to
>    "expire" only the older ones while keeping the more recent ones?
> 
>  * It appears that the main reason you would want --prune-preserved
>    in this design is after running with "--preserve-old" number of
>    times, you want to remove really old ones that have accumulated,
>    and I would imagine that at that point of time, you are only
>    interested in repack, but the code structure tells me that this
>    will force the users to first run a repack before pruning.
> 
> I suspect that a design that is simpler to explain to the users may
> be to add a command line option "--preserve-pruned=<expiration>" and
> a matching configuration variable repack.preservePruned, which
> defaults to "immediate" (i.e. no preserving), and
> 
>  - When the value of preserve_pruned is not "immediate", use
>    preserve_pack() instead of unlink();
> 
>  - At the end, find preserved packs that are older than the value in
>    preserve_pruned and unlink() them.
> 
> It also may make sense to add another command line option
> "--prune-preserved-packs-only" (without matching configuration
> variable) that _ONLY_ does the "find older preserved packs and
> unlink them" part, without doing any repack.

I like the idea of only having a single option to do both the preserving 
and the pruning. It makes the operation more cycle based, which is more 
closely tied to the use case this is intended for. Ie. Run repack while 
preserving old pack files so that any open file handles to those pack 
files will continue to work, while the next time repack is run it is 
highly likely those will no longer be needed, so they can be cleaned up. 
Obviously this depends on how frequent repack is run, but something an 
Administrator can determine.

I'll push a patch that combines that functionality into a single option 
to review.


-James

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project