Re: [PATCH v2 3/4] midx: verify: add midx packfiles to the packed_git list
- From
- Jeff Hostetler <git@jeffhostetler.com>
- Date
- Mar 19, 2019, 19:53 UTC
- Message-ID
- <e09812cf-5c9a-4829-9b6e-0cf8c4fb79c7@jeffhostetler.com>
- In-Reply-To
- <ced7f1cb3486885aef018d91fae37958282a351e.1553006268.git.gitgitgadget@gmail.com>
On 3/19/2019 10:37 AM, Jeff Hostetler via GitGitGadget wrote:
Show 39 quoted lines
> 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