git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] pack-bitmap: gracefully handle missing BTMP chunks

From
Patrick Steinhardt <ps@pks.im>
Date
Apr 15, 2024, 08:51 UTC
Message-ID
<ZhzqeRIcyR3lmBme@tanuki>
In-Reply-To
<a8251f8278ba9a3b41a8e299cb4918a62df6d1c7.1713163238.git.ps@pks.im>
On Mon, Apr 15, 2024 at 08:41:25AM +0200, Patrick Steinhardt wrote:
Show 46 quoted lines
> In 0fea6b73f1 (Merge branch 'tb/multi-pack-verbatim-reuse', 2024-01-12)
> we have introduced multi-pack verbatim reuse of objects. This series has
> introduced a new BTMP chunk, which encodes information about bitmapped
> objects in the multi-pack index. Starting with dab60934e3 (pack-bitmap:
> pass `bitmapped_pack` struct to pack-reuse functions, 2023-12-14) we use
> this information to figure out objects which we can reuse from each of
> the packfiles.
> 
> One thing that we glossed over though is backwards compatibility with
> repositories that do not yet have BTMP chunks in their multi-pack index.
> In that case, `nth_bitmapped_pack()` would return an error, which causes
> us to emit a warning followed by another error message. These warnings
> are visible to users that fetch from a repository:
> 
> ```
> $ git fetch
> ...
> remote: error: MIDX does not contain the BTMP chunk
> remote: warning: unable to load pack: 'pack-f6bb7bd71d345ea9fe604b60cab9ba9ece54ffbe.idx', disabling pack-reuse
> remote: Enumerating objects: 40, done.
> remote: Counting objects: 100% (40/40), done.
> remote: Compressing objects: 100% (39/39), done.
> remote: Total 40 (delta 5), reused 0 (delta 0), pack-reused 0 (from 0)
> ...
> ```
> 
> While the fetch succeeds the user is left wondering what they did wrong.
> Furthermore, as visible both from the warning and from the reuse stats,
> pack-reuse is completely disabled in such repositories.
> 
> What is quite interesting is that this issue can even be triggered in
> case `pack.allowPackReuse=single` is set, which is the default value.
> One could have expected that in this case we fall back to the old logic,
> which is to use the preferred packfile without consulting BTMP chunks at
> all. But either we fail with the above error in case they are missing,
> or we use the first pack in the multi-pack-index. The former case
> disables pack-reuse altogether, whereas the latter case may result in
> reusing objects from a suboptimal packfile.
> 
> Fix this issue by partially reverting the logic back to what we had
> before this patch series landed. Namely, in the case where we have no
> BTMP chunks or when `pack.allowPackReuse=single` are set, we use the
> preferred pack instead of consulting the BTMP chunks.
> 
> Helped-by: Taylor Blau <me@ttaylorr.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>

Junio, it would be great if we could still land this fix in Git v2.45 given that it is addressing a regression in Git v2.44. This of course assumes that the current version of this patch looks good to Taylor.

Patrick
Previous: Patrick SteinhardtNext: Junio C Hamano
Message 6 of 13 in “pack-bitmap: gracefully handle missing BTMP chunks”
  1. pack-bitmap: gracefully handle missing BTMP chunksPatrick Steinhardt, Apr 9, 2024
  2. Taylor BlauApr 10, 2024
  3. Patrick SteinhardtApr 15, 2024
  4. Taylor BlauApr 15, 2024
  5. pack-bitmap: gracefully handle missing BTMP chunksPatrick Steinhardt, Apr 15, 2024
  6. Patrick SteinhardtApr 15, 2024
  7. Junio C HamanoApr 15, 2024
  8. Taylor BlauApr 15, 2024
  9. Junio C HamanoApr 15, 2024
  10. Taylor BlauApr 15, 2024
  11. Patrick SteinhardtApr 16, 2024
  12. Jeff KingApr 16, 2024
  13. Patrick SteinhardtApr 16, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.