Re: [PATCH v3 1/9] t5326: demonstrate bitmap corruption after permutation

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jan 20, 2022, 17:55 UTC
Message-ID: <20220120175541.3099054-1-jonathantanmy@google.com>
In-Reply-To: <babce7d29a85df0da54cb651433111bc33097a4e.1641320129.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

Show 20 quoted lines

> This patch demonstrates a cause of bitmap corruption that can occur when
> the contents of the multi-pack index does not change, but the underlying
> object order does.
> 
> In this example, we have a MIDX containing two packs, each with a
> distinct set of objects (pack A corresponds to the tree, blob, and
> commit from the first patch, and pack B corresponds to the second
> patch).
> 
> First, a MIDX is written where the 'A' pack is preferred. As expected,
> the bitmaps generated there are in-tact. But then, we generate an
> identical MIDX with a different object order: this time preferring pack
> 'B'.
> 
> Due to a bug which will be explained and fixed in the following commit,
> the MIDX is updated, but the .rev file is not, causing the .bitmap file
> to be read incorrectly. Specifically, the .bitmap file will contain
> correct data, but the auxiliary object order in the .rev file is stale,
> causing readers to get confused by reading the new bitmaps using the old
> object order.

Thanks - overall, this looks like a bug that needs to be fixed.

For the benefit of other reviewers, here's my summary of the problem: the .midx, .rev, and .bitmap files are almost always generated together, and it is possible for two different invocations of Git to generate the same .midx but a different .rev and .bitmap. For example, when generating a .midx+.rev+.bitmap for 2 disjoint packfiles, the 1st time with one packfile as preferred and the 2nd time with the other packfile as preferred. In .midx, packfiles are always ordered by lexicographical order of their names, and the preferred status only matters when an object is in multiple packfiles (which never happens in this case, since the packfiles are disjoint). But the preferred status affects .rev and .bitmap, because they use a concept called "pseudo-pack order" (see pack-format.txt for more details) in which the preferred pack comes first.

As an effort to ensure that Git reads coherent .midx, .rev, and .bitmap files, both the .rev and .bitmap files are keyed on the checksum of the .midx file. But the issue here is that a .rev and a .bitmap could both refer to the same .midx checksum when the .rev and .bitmap files are not coherent with respect to each other (e.g. when a Git process has written the .rev, but not the .bitmap yet - but this would appear perfectly ordinary to another concurrently running Git process, since the .midx checksum in the .rev and .bitmap files match).

This problem is exacerbated by the fact that the .rev has its .midx checksum in its filename, whereas the .bitmap has its .midx checksum in its file contents. When generating .midx+.rev+.bitmap, it would write the .bitmap but not the .rev, since a .rev of the same filename already exists.

The solution is to embed the .rev in the .midx. This means that the checksum stored in .bitmap takes into account the contents of what would have been in .rev, solving the coherency issue. (There are other solutions like storing the name of the preferred pack in .midx, but I think that putting the contents of .rev in the .midx is best.)