Re: [PATCH v2 05/11] maintenance: add commit-graph task

From: Jonathan Tan <jonathantanmy@google.com>
Date: Aug 19, 2020, 17:43 UTC
Message-ID: <20200819174322.3087791-1-jonathantanmy@google.com>
In-Reply-To: <66ae9121-80c9-4030-1119-a11eab8d392b@gmail.com>

Show 14 quoted lines

> The cases we've seen this happen were root-caused to hardware
> problems (disk or RAM), and the invalid data was present immediately
> after the "git commit-graph write" command. Since implementing the
> "verify" step after each write, we have not had any user reports of
> problems with these files.
> 
> Are you suggesting one of these options?
> 
>  1. Remove this validation and rewrite entirely.
> 
>  2. Remove the rewrite and only delete the known-bad data.
> 
>  3. Insert a way to cause the verify to return failure mid-process,
>     so that this function can be covered by tests.

I was suggesting 1, although 2 and 3 would assuage my concerns also (2 less so, but just deleting the file is relatively simple and easier to reason about). I don't think Git in general defends much against hardware problems, but if an issue is noticeable in some hardware (as is in this case, at least in the past) I can see why we would want to defend against it. My concern is that if we do defend against it, but leave it untested, several versions of Git later we might find that we need this defense but it does not work.