Re: [PATCH v2 3/4] midx: verify: add midx packfiles to the packed_git list

[Jeff Hostetler <git@jeffhostetler.com>]

On 3/19/2019 10:37 AM, Jeff Hostetler via GitGitGadget wrote:
> From: Jeff Hostetler <jeffhost@microsoft.com>
> 
> Fix "git multi-pack-index verify" to handle repos with thousands
> of packfiles.
> 
> Midx verify adds the individual "packed_git" structures to the
> multi_pack_index.packs array, but it does not add them to the
> "repository.objects.packed_git" list.  During the verification
> code, each packfile is opened and scanned.  And "pack_open_fds"
> is incremented.  If "pack_open_fds" equals the "pack_max_fds"
> open_packed_git_1() calls close_one_pack() to LRU-style close
> an already open packfile.  But because the packfiles were never
> added to the "packed_git" list, close_one_pack() does nothing.
> If there are very many packfiles, Git runs out of file descriptors
> and fails.
> 
> Note that this was observed on Windows when build with GCC and
> in a repository with more than (2048-25) packfiles.
> 
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> ---
>   midx.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/midx.c b/midx.c
> index 24141a7c62..b2f33bcd90 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -975,6 +975,9 @@ int verify_midx_file(const char *object_dir)
>   	for (i = 0; i < m->num_packs; i++) {
>   		if (prepare_midx_pack(m, i))
>   			midx_report("failed to load pack in position %d", i);
> +
> +		if (m->packs[i])
> +			install_packed_git(the_repository, m->packs[i]);
>   	}
>   
>   	for (i = 0; i < 255; i++) {
> 

I'd like to drop this commit from this series.

I talked with Stolee offline about this.  It does
address the problem in this one instance, but it leads
us to think about other places where there may be a
similar problem.

Also, the sort by packfile in the next commit in this
series means we'll only have 1 packfile open at a time
and so this commit isn't needed in this particular case.

Thanks,
Jeff