Re: Question: .idx without .pack causes performance issues?

[Doug Kelly <dougk.ff7@gmail.com>]

On Mon, Aug 3, 2015 at 8:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Doug Kelly <dougk.ff7@gmail.com> writes:
>
>> Here's a change to prune.c that at least addresses the issue by removing
>> .idx files without an associated pack, but it's by no means pretty.  If anyone
>> has any feedback before I turn this into a formal patch, it's more than welcome!
>
> I'd hesitate to see removal of a file (for that matter, a creation
> too) inside a "while (de = readdir)" loop.  As the original function
> is about temporary files, and the new thing is not about temporary
> files at all, I'd further prefer that we do not do it in the same
> loop.
>
> I am wondering if we can add a new mode to report_pack_garbage() in
> sha1_file.c to allow it to remove stale and lone ".idx".  Most of
> the time we are accessing packs read-only, and I do not want the
> function to unconditionally remove lone ".idx", but perhaps we
> can teach "prune" to set a custom report_garbage() routine and
> react to a call to its custom report_garbage()?
>
> Perhaps that custom report_garbage() can make a list of ".idx"
> files, iterate over it to pick the lone one without ".pack" and
> remove them.  Or the custom report_garbage() can make a list of lone
> ".idx" files, if you tweak the interface to report_garbage() to
> contain th seen_bits value, avoiding the need to check the existence
> of ".pack" for the second time.

Yeah, I didn't think this was the cleanest solution, and I wasn't even
thinking about removing while inside the readdir loop, but I can see
how that might be a very bad idea.  In any case, thanks for the
suggestions... I'll be completely blunt in saying I'm far less than
well-versed in the Git internals.  Looking at the implementation of
report_pack_garbage(), it does look like seen_bits already has this
logic, and indeed, git count-objects -v reports the files as garbage.

So, I think you're right: prune would need to set report_garbage
appropriately, then call count-objects to clean that up.  If we wanted
it to *only* care for lone idx files, we would have to string match on
the message (seems fragile), but perhaps a more observant approach
would be to add a custom flag to prune to clean *all* garbage in the
repository, as passed to report_garbage?  Probably wouldn't want to be
enabled by default, but only on invocation or with careful
consideration and setting an appropriate config flag.

Thoughts?