Re: [PATCH v2 3/4] packfile: recover object lookups racing a concurrent repack

>   1. open_pack_index() fails, so we print
> 
>         error: packfile <path> index unavailable
> 
>      and report the pack as unusable, even though the object still lives
>      in the replacement pack.
> 
>   2. A normal lookup recovers: odb_read_object_info_extended() issues a
>      second read that reloads the on-disk pack state and finds the object
>      in its new home, making the message above mere noise.  But an
>      OBJECT_INFO_QUICK lookup deliberately skips that second read to stay
>      fast on a genuine miss, so it does *not* recover: it reports the
>      object as absent even though it still lives in the replacement pack.
>      A resident reader that resolves objects with a QUICK lookup -- such
>      as the `git mktree --batch` process the tests below drive -- then
>      produces wrong results.  Even where a spurious miss is not fatal it
>      is not harmless: `git upload-pack` checks a client's "have" lines
>      with a QUICK lookup, and a dropped "have" removes a common object
>      from the negotiation, so the client is sent more than it needs.

> +	/*
> +	 * Set when a lookup finds that a pack we already know about has
> +	 * vanished -- its ".idx" or ".pack" removed out from under us, the
> +	 * signature of a concurrent "git repack".  It tells
> +	 * odb_read_object_info_extended() to reprepare and retry even for an
> +	 * OBJECT_INFO_QUICK lookup, which normally skips that rescan to stay
> +	 * fast on a genuine miss.  Reset when the packfiles are reprepared
> +	 * (see odb_source_packed_prepare()).
> +	 */
> +	unsigned stale_packs_detected : 1;

> @@ -535,8 +550,20 @@ static int open_packed_git_1(struct packed_git *p)
>  	ssize_t read_result;
>  	const unsigned hashsz = p->repo->hash_algo->rawsz;
>  
> -	if (open_pack_index(p))
> +	if (open_pack_index(p)) {
> +		/*
> +		 * A concurrent repack may have removed this pack, deleting its
> +		 * ".idx" before its ".pack" (see unlink_pack_path()).  If the
> +		 * index simply vanished, note the stale pack set and stay
> +		 * quiet; the pack is still reported unusable.  Only a
> +		 * still-present but unreadable index is worth an error.
> +		 */
> +		if (pack_index_is_missing(p)) {
> +			p->repo->objects->stale_packs_detected = 1;
> +			return -1;
> +		}
>  		return error("packfile %s index unavailable", p->pack_name);
> +	}