Re: [PATCH v3 09/25] midx: avoid opening multiple MIDXs when writing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 12, 2021 at 04:15:32PM -0400, Jeff King wrote:

> I think we'd need something like this:
> 
> diff --git a/midx.c b/midx.c
> index 6dfafe7a8c..bfb6afea2e 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -1123,8 +1123,7 @@ static int write_midx_internal(const char *object_dir,
>  	hold_lock_file_for_update(&lk, midx_name, LOCK_DIE_ON_ERROR);
>  	f = hashfd(get_lock_file_fd(&lk), get_lock_file_path(&lk));
>  
> -	if (ctx.m)
> -		close_midx(ctx.m);
> +	close_object_store(the_repository->objects);
>  
>  	if (ctx.nr - dropped_packs == 0) {
>  		error(_("no pack files to index."));
> 
> though I'm not sure:
> 
>  - if this should be unconditional or dependent on ctx.m (I think the
>    latter, because if we are renaming over any open midx, we would have
>    filled in ctx.m earlier).
> 
>  - if this should go below the "no pack files to index" check (i.e., is
>    there any point in closing if we know we will not write?). In fact,
>    its purpose might be more obvious right before finalize_hashfile(),
>    but I am OK either way on that.

Ah, this close_midx() actually gets moved and made unconditional later
in the series.  But it still needs to be close_object_store() instead.

Also, my mention of finalize_hashfile() is wrong. It's
commit_lock_file() that does the actual rename, and indeed, that's where
you moved it to in the end, which is good.

-Peff




> 
> -Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux