[PATCH 18 of 18 "] verify: also check full manifest validity during verify runs [RFC]

[Pierre-Yves David]

On 3/6/19 7:31 PM, Pulkit Goyal wrote:
> 
> 
> On Wed, Mar 6, 2019 at 8:10 PM Pierre-Yves David 
> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>> 
> wrote:
> 
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david at octobus.net
>     <mailto:pierre-yves.david at octobus.net>>
>     # Date 1551881213 -3600
>     #      Wed Mar 06 15:06:53 2019 +0100
>     # Node ID 1e978412397a4fe629dfa9420b1a7773dc936b19
>     # Parent  92091b3de1503bf562fe5bf2b544781f92702738
>     # EXP-Topic verify
>     # Available At https://bitbucket.org/octobus/mercurial-devel/
>     #              hg pull
>     https://bitbucket.org/octobus/mercurial-devel/ -r 1e978412397a
>     verify: also check full manifest validity during verify runs [RFC]
> 
>     Before this changes, `hg verify` only checked if a manifest revision
>     existed and
>     referenced the proper files. However it never checked the manifest
>     revision
>     content itself.
> 
>     Mercurial is expecting manifest entries to be sorted and will crash
>     otherwise.
>     Since `hg verify` did not tried a full restoration of manifest
>     entry, it could
>     ignore this kind of corruption.
> 
>     This new check significantly increases the cost of a `hg verify`
>     run. This
>     especially affects large repository not using `sparse-revlog`. We
>     might want to
>     put it behind a new flag like `--full`. Another way to look at this
>     would be to
>     offer a `--quick` flag that disable it. In particular, since `hg
>     recover` runs
>     verify, this could impact users.
> 
> 
> A slow `hg recover` is already painful for us whenever we run it. So we 
> decided whatever happen, we will never Ctrl+C a hg process.

Yeah, I've experienced that too. Overall, the recover logic have been 
pretty solid in the case I end up using it. We should offer a way to run 
recover without the verify step.


> That said, what are the cases which can lead to unsorted manifest 
> revisions? It will be worth putting them in commit message.

There are a chicken and eggs issue here, I needed this command to learn 
more about the issue. And I now do know more:

There are 2 affected manifest used in 3 changesets for Mozilla-try 
(6b58490b1f9b, f13fe2770423, and 58842787f756).
All three changesets dates back from 2015 and are Authored by Mike 
Hommey. So I strongly suspect they were created by an early version of 
cinabar.

(I'm playing with Mozilla try as a test repository for our discovery works).

Even if such corruption will be very rare, I still think that running 
the extra check make sense by default. The following verify helps 
statement is currently wrong: "validating the hashes and checksums of 
each entry in the changelog, manifest, and tracked files". The manifest 
log hashes are not currently checked. This changeset fix this.

However this will make the recover situation worth. Here a wider plan to 
deal with that

1) add a --quick flag to `hg verify` that only check revlog length,
2) add a --verify flag to `hg recover` (--no-verify will skip the verify 
step),
3) make --no-verify the default,
3.5) maybe deprecated the --verify flag,
4) check manifest hashes (and order) by default.

What do you think ?

-- 
Pierre-Yves David