On 2017-03-07 13:33, Junio C Hamano wrote:
James Melvin <jmelvin@xxxxxxxxxxxxxx> 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