Patchwork [2,of,4,V2] verify: introduce a notion of "level"

login
register
mail settings
Submitter Pierre-Yves David
Date April 17, 2019, 10:49 a.m.
Message ID <55bd98999c25b10e2204.1555498161@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/39684/
State Superseded
Headers show

Comments

Pierre-Yves David - April 17, 2019, 10:49 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1555456341 -7200
#      Wed Apr 17 01:12:21 2019 +0200
# Node ID 55bd98999c25b10e220477fd4cc446a7c9c1f8ca
# Parent  f233cb63bc077267d8571378350d9563cbabcf3d
# EXP-Topic verify
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 55bd98999c25
verify: introduce a notion of "level"

Some checks are slower than others, to help the user to run the checks he needs,
we are about to introduce new flag to select faster vs deeper runs. This put
the scaffolding in place to do this.
via Mercurial-devel - April 17, 2019, 1:18 p.m.
On Wed, Apr 17, 2019, 03:49 Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1555456341 -7200
> #      Wed Apr 17 01:12:21 2019 +0200
> # Node ID 55bd98999c25b10e220477fd4cc446a7c9c1f8ca
> # Parent  f233cb63bc077267d8571378350d9563cbabcf3d
> # EXP-Topic verify
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> 55bd98999c25
> verify: introduce a notion of "level"


I feel like this is unnecessarily complicated. It feels like just passing a
boolean "full" would be enough. If one level has been enough for over ten
years, it seems likely that two levels will be enough for many years ahead.
Or do you plan to add more levels?
Pierre-Yves David - April 17, 2019, 1:27 p.m.
On 4/17/19 3:18 PM, Martin von Zweigbergk wrote:
> 
> 
> On Wed, Apr 17, 2019, 03:49 Pierre-Yves David 
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>> 
> wrote:
> 
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david@octobus.net
>     <mailto:pierre-yves.david@octobus.net>>
>     # Date 1555456341 -7200
>     #      Wed Apr 17 01:12:21 2019 +0200
>     # Node ID 55bd98999c25b10e220477fd4cc446a7c9c1f8ca
>     # Parent  f233cb63bc077267d8571378350d9563cbabcf3d
>     # EXP-Topic verify
>     # Available At https://bitbucket.org/octobus/mercurial-devel/
>     #              hg pull
>     https://bitbucket.org/octobus/mercurial-devel/ -r 55bd98999c25
>     verify: introduce a notion of "level"
> 
> 
> I feel like this is unnecessarily complicated. It feels like just 
> passing a boolean "full" would be enough. If one level has been enough 
> for over ten years, it seems likely that two levels will be enough for 
> many years ahead. Or do you plan to add more levels?

I am planning to pass a "quick" mode that only check for stuff like 
revlog size.

Since the python code is implementation details and the command line 
flag is experimental we have a lot of flexibility here.

Overall, the spirit of this series is only "I spent some time to 
diagnose a local issue, I would rather upstream that work so that the 
next person don't have to do it again", I am not in a long quest to 
improve `hg verify` and I don't plan to spend too much time on it in the 
near future.

note: patch 1 is fairly independent from the level and manifest question 
and "fix" hg recover for a large set of users. Please consider taking 
that one while we discuss the rest.
via Mercurial-devel - April 17, 2019, 1:47 p.m.
On Wed, Apr 17, 2019, 06:27 Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 4/17/19 3:18 PM, Martin von Zweigbergk wrote:
> >
> >
> > On Wed, Apr 17, 2019, 03:49 Pierre-Yves David
> > <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
>
> > wrote:
> >
> >     # HG changeset patch
> >     # User Pierre-Yves David <pierre-yves.david@octobus.net
> >     <mailto:pierre-yves.david@octobus.net>>
> >     # Date 1555456341 -7200
> >     #      Wed Apr 17 01:12:21 2019 +0200
> >     # Node ID 55bd98999c25b10e220477fd4cc446a7c9c1f8ca
> >     # Parent  f233cb63bc077267d8571378350d9563cbabcf3d
> >     # EXP-Topic verify
> >     # Available At https://bitbucket.org/octobus/mercurial-devel/
> >     #              hg pull
> >     https://bitbucket.org/octobus/mercurial-devel/ -r 55bd98999c25
> >     verify: introduce a notion of "level"
> >
> >
> > I feel like this is unnecessarily complicated. It feels like just
> > passing a boolean "full" would be enough. If one level has been enough
> > for over ten years, it seems likely that two levels will be enough for
> > many years ahead. Or do you plan to add more levels?
>
> I am planning to pass a "quick" mode that only check for stuff like
> revlog size.


> Since the python code is implementation details and the command line
> flag is experimental we have a lot of flexibility here.
>
> Overall, the spirit of this series is only "I spent some time to
> diagnose a local issue, I would rather upstream that work so that the
> next person don't have to do it again", I am not in a long quest to
> improve `hg verify` and I don't plan to spend too much time on it in the
> near future.
>
> note: patch 1 is fairly independent from the level and manifest question
> and "fix" hg recover for a large set of users. Please consider taking
> that one while we discuss the rest.
>

Thanks, will do. I'll use that option myself (I currently ^C the verify
step :))


>
> --
> Pierre-Yves David
>
via Mercurial-devel - April 17, 2019, 6:04 p.m.
On Wed, Apr 17, 2019 at 6:27 AM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 4/17/19 3:18 PM, Martin von Zweigbergk wrote:
> >
> >
> > On Wed, Apr 17, 2019, 03:49 Pierre-Yves David
> > <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
>
> > wrote:
> >
> >     # HG changeset patch
> >     # User Pierre-Yves David <pierre-yves.david@octobus.net
> >     <mailto:pierre-yves.david@octobus.net>>
> >     # Date 1555456341 -7200
> >     #      Wed Apr 17 01:12:21 2019 +0200
> >     # Node ID 55bd98999c25b10e220477fd4cc446a7c9c1f8ca
> >     # Parent  f233cb63bc077267d8571378350d9563cbabcf3d
> >     # EXP-Topic verify
> >     # Available At https://bitbucket.org/octobus/mercurial-devel/
> >     #              hg pull
> >     https://bitbucket.org/octobus/mercurial-devel/ -r 55bd98999c25
> >     verify: introduce a notion of "level"
> >
> >
> > I feel like this is unnecessarily complicated. It feels like just
> > passing a boolean "full" would be enough. If one level has been enough
> > for over ten years, it seems likely that two levels will be enough for
> > many years ahead. Or do you plan to add more levels?
>
> I am planning to pass a "quick" mode that only check for stuff like
> revlog size.
>

What about the revlog size would it check?
Pierre-Yves David - April 17, 2019, 6:27 p.m.
On 4/17/19 8:04 PM, Martin von Zweigbergk wrote:
> 
> 
> On Wed, Apr 17, 2019 at 6:27 AM Pierre-Yves David 
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>> 
> wrote:
> 
> 
> 
>     On 4/17/19 3:18 PM, Martin von Zweigbergk wrote:
>      >
>      >
>      > On Wed, Apr 17, 2019, 03:49 Pierre-Yves David
>      > <pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>
>     <mailto:pierre-yves.david@ens-lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>>>
>      > wrote:
>      >
>      >     # HG changeset patch
>      >     # User Pierre-Yves David <pierre-yves.david@octobus.net
>     <mailto:pierre-yves.david@octobus.net>
>      >     <mailto:pierre-yves.david@octobus.net
>     <mailto:pierre-yves.david@octobus.net>>>
>      >     # Date 1555456341 -7200
>      >     #      Wed Apr 17 01:12:21 2019 +0200
>      >     # Node ID 55bd98999c25b10e220477fd4cc446a7c9c1f8ca
>      >     # Parent  f233cb63bc077267d8571378350d9563cbabcf3d
>      >     # EXP-Topic verify
>      >     # Available At https://bitbucket.org/octobus/mercurial-devel/
>      >     #              hg pull
>      > https://bitbucket.org/octobus/mercurial-devel/ -r 55bd98999c25
>      >     verify: introduce a notion of "level"
>      >
>      >
>      > I feel like this is unnecessarily complicated. It feels like just
>      > passing a boolean "full" would be enough. If one level has been
>     enough
>      > for over ten years, it seems likely that two levels will be
>     enough for
>      > many years ahead. Or do you plan to add more levels?
> 
>     I am planning to pass a "quick" mode that only check for stuff like
>     revlog size.
> 
> 
> What about the revlog size would it check?

Mostly the part of the checks that do basic sanity check to the revlog, 
without looking individual revisions: 
https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/verify.py#l79

Only looking at revlog make the proces O(nbfiles) instead of 
O(nbfiles*nbrevs) The checks are also much cheaper

Patch

diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -1092,9 +1092,9 @@  def outgoing(ui, repo, dest, opts):
     recurse()
     return 0 # exit code is zero since we found outgoing changes
 
-def verify(repo):
+def verify(repo, level=None):
     """verify the consistency of a repository"""
-    ret = verifymod.verify(repo)
+    ret = verifymod.verify(repo, level=level)
 
     # Broken subrepo references in hidden csets don't seem worth worrying about,
     # since they can't be pushed/pulled, and --hidden can be used if they are a
diff --git a/mercurial/verify.py b/mercurial/verify.py
--- a/mercurial/verify.py
+++ b/mercurial/verify.py
@@ -22,9 +22,12 @@  from . import (
     util,
 )
 
-def verify(repo):
+VERIFY_DEFAULT = 0
+
+def verify(repo, level=None):
     with repo.lock():
-        return verifier(repo).verify()
+        v = verifier(repo, level)
+        return v.verify()
 
 def _normpath(f):
     # under hg < 2.4, convert didn't sanitize paths properly, so a
@@ -34,10 +37,13 @@  def _normpath(f):
     return f
 
 class verifier(object):
-    def __init__(self, repo):
+    def __init__(self, repo, level=None):
         self.repo = repo.unfiltered()
         self.ui = repo.ui
         self.match = repo.narrowmatch()
+        if level is None:
+            level = VERIFY_DEFAULT
+        self._level = level
         self.badrevs = set()
         self.errors = 0
         self.warnings = 0