Bug 3827 - stale phasecache causes LookupError on branchmap.updatecache() after strip
Summary: stale phasecache causes LookupError on branchmap.updatecache() after strip
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: Mercurial (show other bugs)
Version: earlier
Hardware: PC Linux
: normal bug
Assignee: Bugzilla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-17 02:47 UTC by Yuya Nishihara
Modified: 2017-11-01 18:05 UTC (History)
7 users (show)

See Also:
Python Version: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yuya Nishihara 2013-02-17 02:47 UTC
Since Mercurial 2.5, the following steps result in LookupError: no node.

1. Process-A loads phaseroots
2. Process-B strips the node pointed by phaseroots
3. Process-A updates branchcache after repo.invalidate()

repo.invalidate() clears out most of cache data, but _phasecache isn't cleared.
This is the same for Mercurial 2.4, but 2.4 works well maybe because it doesn't
look up nodes by stale phasecache.

Script to reproduce
----
LANG=C
HGUSER=test
export HGUSER

#hg version -q
hg init issue2428
cd issue2428

cat <<EOF > .hg/hgrc
[defaults]
commit = -d "0 0"
[extensions]
mq =
EOF

touch toto
hg add toto
hg commit -m"add toto"
touch tata
hg add tata
hg commit -m"add tata"
hg up 0
touch titi
hg add titi
hg commit -m"add titi"
hg phase -p tip

{
    # load _phasecache.phaseroots
    printf 'runcommand\n\0\0\0\011phase\0-r1'
    sleep 1  # flush

    hg strip -r1 >&2  # by another process or thread

    # calls repo.invalidate() but _phasecache not cleared
    printf 'runcommand\n\0\0\0\010branches'
} | hg serve --cmdserver pipe --trace
----

Stack trace
----
Traceback (most recent call last):
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/dispatch.py", line 88, in _runcatch
    return _dispatch(req)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/dispatch.py", line 743, in _dispatch
    cmdpats, cmdoptions)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/extensions.py", line 189, in wrap
    return wrapper(origfn, *args, **kwargs)
  File "/home/yuya/.hgext/textful/__init__.py", line 152, in textfulcmd
    cmdoptions)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/dispatch.py", line 514, in runcommand
    ret = _runcommand(ui, options, cmd, d)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/extensions.py", line 189, in wrap
    return wrapper(origfn, *args, **kwargs)
  File "/home/yuya/work/hghacks/mercurial-stable/hgext/color.py", line 394, in colorcmd
    return orig(ui_, opts, cmd, cmdfunc)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/dispatch.py", line 833, in _runcommand
    return checkargs()
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/dispatch.py", line 804, in checkargs
    return cmdfunc()
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/dispatch.py", line 740, in <lambda>
    d = lambda: util.checksignature(func)(ui, *args, **cmdoptions)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/util.py", line 475, in check
    return func(*args, **kwargs)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/extensions.py", line 144, in wrap
    util.checksignature(origfn), *args, **kwargs)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/util.py", line 475, in check
    return func(*args, **kwargs)
  File "/home/yuya/work/hghacks/mercurial-stable/hgext/mq.py", line 3508, in mqcommand
    return orig(ui, repo, *args, **kwargs)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/util.py", line 475, in check
    return func(*args, **kwargs)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/commands.py", line 967, in branches
    for tag, heads in repo.branchmap().iteritems():
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/localrepo.py", line 634, in branchmap
    branchmap.updatecache(self)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/branchmap.py", line 75, in updatecache
    partial = subset.branchmap().copy()
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/localrepo.py", line 634, in branchmap
    branchmap.updatecache(self)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/branchmap.py", line 75, in updatecache
    partial = subset.branchmap().copy()
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/localrepo.py", line 634, in branchmap
    branchmap.updatecache(self)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/branchmap.py", line 62, in updatecache
    cl = repo.changelog
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/repoview.py", line 170, in changelog
    revs = filterrevs(unfi, self.filtername)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/repoview.py", line 117, in filterrevs
    repo.filteredrevcache[filtername] = func(repo.unfiltered())
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/repoview.py", line 68, in computemutable
    maymutable = filterrevs(repo, 'base')
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/repoview.py", line 117, in filterrevs
    repo.filteredrevcache[filtername] = func(repo.unfiltered())
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/repoview.py", line 92, in computeimpactable
    firstmutable = min(firstmutable, min(cl.rev(r) for r in roots))
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/repoview.py", line 92, in <genexpr>
    firstmutable = min(firstmutable, min(cl.rev(r) for r in roots))
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/changelog.py", line 184, in rev
    r = super(changelog, self).rev(node)
  File "/home/yuya/work/hghacks/mercurial-stable/mercurial/revlog.py", line 293, in rev
    raise LookupError(node, self.indexfile, _('no node'))
LookupError: 00changelog.i@92a86c53f65b: no node
----

This issue was originally reported to TortoiseHg:
https://bitbucket.org/tortoisehg/thg/issue/2428/strip-revisions-with-some-specific-phase
Comment 1 Idan Kamara 2013-02-18 04:40 UTC
The culprit is in this line:

http://selenic.com/repo/hg/file/4921b5c2aeed/mercurial/localrepo.py#l1406

What happens is when the strip command is run, it calls repo.destroyed, which in turn checks if we read _phasecache, and if we did calls filterunknown on it and flushes the changes immediately. But in this case, nothing caused _phasecache to be read, so we miss out on this and the file remains the same on-disk.

Then a call to invalidate comes, which should refresh _phasecache if it changed, but it didn't, so it keeps using the old one with the stripped revision.

So if we stop assuming _phasecache was read and always call filterunknown, that will fix things.

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1403,9 +1403,8 @@
         # removed. We can either remove phasecache from the filecache,
         # causing it to reload next time it is accessed, or simply filter
         # the removed nodes now and write the updated cache.
-        if '_phasecache' in self._filecache:
-            self._phasecache.filterunknown(self)
-            self._phasecache.write()
+        self._phasecache.filterunknown(self)
+        self._phasecache.write()
 
         # update the 'served' branch cache to help read only server process
         # Thanks to branchcache collaboration this is done from the nearest
Comment 2 Idan Kamara 2013-03-18 06:19 UTC
(In reply to comment #0)

This one has been sitting here for quite a while (I was away). The only thing I haven't been able to do successfully is add a test to test-commandserver.py.

Yuya, could you try to translate your original test to something that fits in the current one?
Comment 3 Yuya Nishihara 2013-03-20 07:55 UTC
(In reply to comment #2)

This will suit test-commandserver.py. Should I send it as a patch?

def phasecacheafterstrip(server):
    readchannel(server)

    # create new head, 5:731265503d86
    runcommand(server, ['update', '-C', '0'])
    f = open('a', 'ab')
    f.write('a\n')
    f.close()
    runcommand(server, ['commit', '-Am.', 'a'])
    runcommand(server, ['log', '-Gq'])

    # make it public; draft marker moves to 4:7966c8e3734d
    runcommand(server, ['phase', '-p', '.'])
    runcommand(server, ['phase', '.'])  # load _phasecache.phaseroots

    # strip 1::4 outside server
    os.system('hg --config extensions.mq= strip 1')

    # shouldn't raise "7966c8e3734d: no node!"
    runcommand(server, ['branches'])
Comment 4 Idan Kamara 2013-03-23 07:36 UTC
(In reply to comment #3)

Added it to my fix, thanks.

http://markmail.org/message/iss62ry64rqjfqyn
Comment 5 HG Bot 2013-03-29 16:30 UTC
Fixed by http://selenic.com/repo/hg/rev/1c8e0d6ac3b0
Idan Kamara <idankk86@gmail.com>
localrepo: always write the filtered phasecache when nodes are destroyed (issue3827)

When the strip command is run, it calls repo.destroyed, which in turn checks if
we read _phasecache, and if we did calls filterunknown on it and flushes the
changes immediately. But in some cases, nothing causes _phasecache to be read,
so we miss out on this and the file remains the same on-disk.

Then a call to invalidate comes, which should refresh _phasecache if it
changed, but it didn't, so it keeps using the old one with the stripped
revision which causes an IndexError.

Test written by Yuya Nishihara.

(please test the fix)
Comment 6 Yuya Nishihara 2013-04-07 07:39 UTC
No problem in Mercurial 2.5.4. Thanks.