This is an archive of the discontinued Mercurial Phabricator instance.

Differential D99

fastpartialmatch: fix devel.warn
ClosedPublic

Authored by stash on Jul 17 2017, 5:28 AM.
Tags
None
Subscribers
durham

Details

Reviewers
durham
Group Reviewers
Restricted Project
Commits
rFBHGXe1eed1cbdad6: fastpartialmatch: fix devel.warn
Summary

Upstream hg recently introduced new developer warning if files inside .hg are
modified without taking a lock. The usual fix is to add file in
localrepository._wlockfreeprefix list, but it doesn't work for "needrebuild"
fastpartialmatch file because it's stored inside .hg/store.
So to get rid of developer warning let's move needrebuild file to .hg/.
And since we are detecting that partial index needs to be rebuilt in revlog class,
I also had to wrap runcommand() and create needrebuild file there, because
runcommand() has vfs object, while revlog class doesn't.

Test Plan

Run tests

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

stash created this revision.Jul 17 2017, 5:28 AM
stash added a comment.Jul 17 2017, 5:29 AM

There is a diff that fixes the same issue - https://phab.mercurial-scm.org/D89, but this version seems a bit simpler and doesn't cause tests to fail

durham accepted this revision.Jul 17 2017, 5:46 AM
durham added a subscriber: durham.

Not a huge fan of passing the value back up via a config option, but I guess it's fine for now.

This revision is now accepted and ready to land.Jul 17 2017, 5:46 AM
Closed by commit rFBHGXe1eed1cbdad6: fastpartialmatch: fix devel.warn (authored by stash). · Explain WhyJul 17 2017, 7:09 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 201

hgext3rd/fastpartialmatch.py

# if any index file has more than or equal to `unsortedthreshold` unsorted # if any index file has more than or equal to `unsortedthreshold` unsorted
# entries then index will be rebuilt when _changegrouphook will be triggered # entries then index will be rebuilt when _changegrouphook will be triggered
# (usually it's the next pull) # (usually it's the next pull)
unsortedthreshold = 1000 unsortedthreshold = 1000
# if fastpartialmatch extension was temporarily disabled then index may miss # if fastpartialmatch extension was temporarily disabled then index may miss
# some entries. By bumping generationnumber we can force index to be rebuilt # some entries. By bumping generationnumber we can force index to be rebuilt
generationnumber = 0 generationnumber = 0
# internal config setting. Marks index as needed to be rebuilt
rebuild = False
''' '''
from collections import defaultdict from collections import defaultdict
from functools import partial from functools import partial
from hgext3rd.generic_bisect import bisect from hgext3rd.generic_bisect import bisect
from mercurial import ( from mercurial import (
dispatch,
error, error,
extensions, extensions,
localrepo, localrepo,
registrar, registrar,
revlog, revlog,
util, util,
vfs as vfsmod, vfs as vfsmod,
) )
_nodesize = 20 _nodesize = 20
_entrysize = _nodesize + _packstruct.size _entrysize = _nodesize + _packstruct.size
_raiseifinconsistent = False _raiseifinconsistent = False
_usebisect = True _usebisect = True
_current_version = 1 _current_version = 1
_tip = 'run `hg debugrebuildpartialindex` to fix the issue' _tip = 'run `hg debugrebuildpartialindex` to fix the issue'
_unsortedthreshold = 1000 _unsortedthreshold = 1000
_needrebuildfile = os.path.join(_partialindexdir, 'needrebuild') _needrebuildfile = 'partialindexneedrebuild'
def extsetup(ui): def extsetup(ui):
extensions.wrapfunction(revlog.revlog, '_partialmatch', _partialmatch) extensions.wrapfunction(revlog.revlog, '_partialmatch', _partialmatch)
extensions.wrapfunction(localrepo.localrepository, 'commit', extensions.wrapfunction(localrepo.localrepository, 'commit',
_localrepocommit) _localrepocommit)
extensions.wrapfunction(localrepo.localrepository, 'transaction', extensions.wrapfunction(localrepo.localrepository, 'transaction',
_localrepotransaction) _localrepotransaction)
global _raiseifinconsistent global _raiseifinconsistent
_raiseifinconsistent = ui.configbool('fastpartialmatch', _raiseifinconsistent = ui.configbool('fastpartialmatch',
'raiseifinconsistent', False) 'raiseifinconsistent', False)
global _usebisect global _usebisect
_usebisect = ui.configbool('fastpartialmatch', 'usebisect', True) _usebisect = ui.configbool('fastpartialmatch', 'usebisect', True)
global _unsortedthreshold global _unsortedthreshold
_unsortedthreshold = ui.configint('fastpartialmatch', 'unsortedthreshold', _unsortedthreshold = ui.configint('fastpartialmatch', 'unsortedthreshold',
_unsortedthreshold) _unsortedthreshold)
def _runcommand(orig, lui, repo, cmd, fullargs, ui, *args, **kwargs):
res = orig(lui, repo, cmd, fullargs, ui, *args, **kwargs)
if ui.config('fastpartialmatch', 'rebuild', False):
_markneedsrebuilding(ui, repo)
return res
extensions.wrapfunction(dispatch, 'runcommand', _runcommand)
# Add _needrebuildfile to the list of files that don't need to be protected
# by wlock. Race conditions on _needrebuildfile are not important because
# at worst it may trigger rebuilding twice or postpone index rebuilding.
localrepo.localrepository._wlockfreeprefix.add(_needrebuildfile)
def reposetup(ui, repo): def reposetup(ui, repo):
isbundlerepo = repo.url().startswith('bundle:') isbundlerepo = repo.url().startswith('bundle:')
if repo.local() and not isbundlerepo: if repo.local() and not isbundlerepo:
# Add `ui` object and `usefastpartialmatch` to access it # Add `ui` object and `usefastpartialmatch` to access it
# from `_partialmatch` func # from `_partialmatch` func
repo.svfs.ui = ui repo.svfs.ui = ui
repo.svfs.usefastpartialmatch = True repo.svfs.usefastpartialmatch = True
ui.setconfig('hooks', 'pretxncommit.fastpartialmatch', _commithook) ui.setconfig('hooks', 'pretxncommit.fastpartialmatch', _commithook)
@command('^debugfastpartialmatchstat', []) @command('^debugfastpartialmatchstat', [])
def debugfastpartialmatchstat(ui, repo): def debugfastpartialmatchstat(ui, repo):
if not repo.svfs.exists(_partialindexdir): if not repo.svfs.exists(_partialindexdir):
ui.warn(_('partial index is not built\n')) ui.warn(_('partial index is not built\n'))
return 1 return 1
generationnum = _readgenerationnum(ui, repo.svfs) generationnum = _readgenerationnum(ui, repo.svfs)
ui.write(_('generation number: %d\n') % generationnum) ui.write(_('generation number: %d\n') % generationnum)
if _needsrebuilding(repo.svfs): if _needsrebuilding(repo):
ui.write(_('index will be rebuilt on the next pull\n')) ui.write(_('index will be rebuilt on the next pull\n'))
indexvfs = vfsmod.vfs(repo.svfs.join(_partialindexdir)) indexvfs = vfsmod.vfs(repo.svfs.join(_partialindexdir))
for indexfile in sorted(_iterindexfile(indexvfs)): for indexfile in sorted(_iterindexfile(indexvfs)):
size = indexvfs.stat(indexfile).st_size - _header.headersize size = indexvfs.stat(indexfile).st_size - _header.headersize
entriescount = size / _entrysize entriescount = size / _entrysize
with indexvfs(indexfile) as fileobj: with indexvfs(indexfile) as fileobj:
header = _header.read(fileobj) header = _header.read(fileobj)
ui.write(_('file: %s, entries: %d, out of them %d sorted\n') % ui.write(_('file: %s, entries: %d, out of them %d sorted\n') %
tempdir = '.tmp' + _partialindexdir tempdir = '.tmp' + _partialindexdir
if vfs.exists(_partialindexdir): if vfs.exists(_partialindexdir):
vfs.rmtree(_partialindexdir) vfs.rmtree(_partialindexdir)
if vfs.exists(tempdir): if vfs.exists(tempdir):
vfs.rmtree(tempdir) vfs.rmtree(tempdir)
vfs.mkdir(tempdir) vfs.mkdir(tempdir)
_unmarkneedsrebuilding(repo)
filesdata = defaultdict(list) filesdata = defaultdict(list)
for rev in repo.changelog: for rev in repo.changelog:
node = repo.changelog.node(rev) node = repo.changelog.node(rev)
hexnode = hex(node) hexnode = hex(node)
if hexnode in skiphexnodes: if hexnode in skiphexnodes:
continue continue
filename = hexnode[:2] filename = hexnode[:2]
# Ask changelog directly to avoid calling fastpartialmatch because # Ask changelog directly to avoid calling fastpartialmatch because
# it doesn't have the newest nodes yet # it doesn't have the newest nodes yet
rev_first = repo.changelog.rev(bin(hexnode_first)) rev_first = repo.changelog.rev(bin(hexnode_first))
rev_last = repo.changelog.rev(bin(hexnode_last)) rev_last = repo.changelog.rev(bin(hexnode_last))
newhexnodes = [] newhexnodes = []
for rev in xrange(rev_first, rev_last + 1): for rev in xrange(rev_first, rev_last + 1):
newhexnodes.append(repo[rev].hex()) newhexnodes.append(repo[rev].hex())
if not vfs.exists(_partialindexdir) or _needsrebuilding(vfs): if not vfs.exists(_partialindexdir) or _needsrebuilding(repo):
_rebuildpartialindex(ui, repo, skiphexnodes=set(newhexnodes)) _rebuildpartialindex(ui, repo, skiphexnodes=set(newhexnodes))
for i, hexnode in enumerate(newhexnodes): for i, hexnode in enumerate(newhexnodes):
_recordcommit(ui, tr, hexnode, rev_first + i, vfs) _recordcommit(ui, tr, hexnode, rev_first + i, vfs)
else: else:
ui.warn(_('unexpected hookargs parameters: `node` and ' + ui.warn(_('unexpected hookargs parameters: `node` and ' +
'`node_last` should be present\n')) '`node_last` should be present\n'))
def _recordcommit(ui, tr, hexnode, rev, vfs): def _recordcommit(ui, tr, hexnode, rev, vfs):
unsorted = 0 unsorted = 0
for node, rev in _readtillend(fileobj): for node, rev in _readtillend(fileobj):
hexnode = hex(node) hexnode = hex(node)
unsorted += 1 unsorted += 1
if hexnode.startswith(id): if hexnode.startswith(id):
candidates[node] = rev candidates[node] = rev
if unsorted >= _unsortedthreshold: if unsorted >= _unsortedthreshold:
_markneedsrebuilding(ui, vfs) ui.setconfig('fastpartialmatch', 'rebuild', True)
except Exception as e: except Exception as e:
ui.warn(_('failed to read partial index %s : %s\n') % ui.warn(_('failed to read partial index %s : %s\n') %
(fullpath, str(e))) (fullpath, str(e)))
return None return None
return candidates return candidates
class _header(object): class _header(object):
_versionpack = struct.Struct('!B') _versionpack = struct.Struct('!B')
headerversion = cls._versionpack.unpack(headerversion)[0] headerversion = cls._versionpack.unpack(headerversion)[0]
if headerversion != _current_version: if headerversion != _current_version:
raise ValueError(_('incompatible index version: %s') % _tip) raise ValueError(_('incompatible index version: %s') % _tip)
sortedcount = header[versionsize:versionsize + cls._intpacker.size] sortedcount = header[versionsize:versionsize + cls._intpacker.size]
sortedcount = cls._intpacker.unpack(sortedcount)[0] sortedcount = cls._intpacker.unpack(sortedcount)[0]
return cls(sortedcount) return cls(sortedcount)
def _needsrebuilding(vfs): def _needsrebuilding(repo):
return vfs.exists(_needrebuildfile) return repo.vfs.exists(_needrebuildfile)
def _markneedsrebuilding(ui, vfs): def _markneedsrebuilding(ui, repo):
try: try:
with vfs(_needrebuildfile, 'w') as fileobj: with repo.vfs(_needrebuildfile, 'w') as fileobj:
fileobj.write('content') # content doesn't matter fileobj.write('content') # content doesn't matter
except IOError as e: except IOError as e:
ui.warn(_('error happened while triggering rebuild: %s\n') % e) ui.warn(_('error happened while triggering rebuild: %s\n') % e)
def _unmarkneedsrebuilding(repo):
repo.vfs.tryunlink(_needrebuildfile)
def _readgenerationnum(ui, vfs): def _readgenerationnum(ui, vfs):
generationnumfile = os.path.join(_partialindexdir, 'generationnum') generationnumfile = os.path.join(_partialindexdir, 'generationnum')
if not vfs.exists(generationnumfile): if not vfs.exists(generationnumfile):
return 0 return 0
try: try:
with vfs(generationnumfile) as f: with vfs(generationnumfile) as f:
return int(f.read()) return int(f.read())
except Exception as e: except Exception as e:

tests/test-fastpartialmatch.t

ac536e: ac536ed8bde0682e30bb64c64570758903ce1aa6 3 ac536e: ac536ed8bde0682e30bb64c64570758903ce1aa6 3
Test cache rebuilding Test cache rebuilding
$ cd ../repo $ cd ../repo
$ mkcommit committotriggercacherebuilding $ mkcommit committotriggercacherebuilding
$ cd ../cloned2 $ cd ../cloned2
$ printf '[fastpartialmatch]\nunsortedthreshold=1' >> .hg/hgrc $ printf '[fastpartialmatch]\nunsortedthreshold=1' >> .hg/hgrc
$ hg up -q ac536ed8bde0 $ hg up -q ac536ed8bde0
$ [ -f .hg/partialindexneedrebuild ]
[1]
$ mkcommit commit $ mkcommit commit
$ hg log -r . -T '{node}\n' $ hg log -r . -T '{node}\n'
587cd78c6d0eb0259484b09a5983bcc2973f2245 587cd78c6d0eb0259484b09a5983bcc2973f2245
Next command should set that cache needs rebuilding Next command should set that cache needs rebuilding
$ hg log -r 587cd78c6d0eb0259 > /dev/null $ hg log -r 587cd78c6d0eb0259 > /dev/null
$ [ -f .hg/partialindexneedrebuild ]
$ hg debugfastpartialmatchstat $ hg debugfastpartialmatchstat
generation number: 0 generation number: 0
index will be rebuilt on the next pull index will be rebuilt on the next pull
file: 3d, entries: 1, out of them 1 sorted file: 3d, entries: 1, out of them 1 sorted
file: 58, entries: 1, out of them 0 sorted file: 58, entries: 1, out of them 0 sorted
file: 64, entries: 1, out of them 1 sorted file: 64, entries: 1, out of them 1 sorted
file: ac, entries: 1, out of them 1 sorted file: ac, entries: 1, out of them 1 sorted
file: b7, entries: 1, out of them 1 sorted file: b7, entries: 1, out of them 1 sorted
file: 12, entries: 1, out of them 0 sorted file: 12, entries: 1, out of them 0 sorted
file: 3d, entries: 1, out of them 1 sorted file: 3d, entries: 1, out of them 1 sorted
file: 58, entries: 1, out of them 1 sorted file: 58, entries: 1, out of them 1 sorted
file: 64, entries: 1, out of them 1 sorted file: 64, entries: 1, out of them 1 sorted
file: ac, entries: 1, out of them 1 sorted file: ac, entries: 1, out of them 1 sorted
file: b7, entries: 1, out of them 1 sorted file: b7, entries: 1, out of them 1 sorted
file: fd, entries: 1, out of them 0 sorted file: fd, entries: 1, out of them 0 sorted
Make a commit and change partialindex permissions to non-writabble. Then do Make a commit and change .hg permissions to non-writabble. Then do
partial lookup that should write needrebuild file but it couldn't because partial lookup that should write needrebuild file but it couldn't because
of permissions. Make sure it doesn't throw and just log the problem of permissions. Make sure it doesn't throw and just log the problem
$ mkcommit commitpermissionissue $ mkcommit commitpermissionissue
$ chmod u-w .hg/store/partialindex $ chmod u-w .hg/
$ hg log -r . -T '{node}\n' $ hg log -r . -T '{node}\n'
2b52832374dd7e499a1fbd172f1d75e13ee32477 2b52832374dd7e499a1fbd172f1d75e13ee32477
$ hg log -r 2b5283237 $ hg log -r 2b5283237
error happened while triggering rebuild: [Errno 13] Permission denied: '$TESTTMP/cloned2/.hg/store/partialindex/needrebuild'
error happened while triggering rebuild: [Errno 13] Permission denied: '$TESTTMP/cloned2/.hg/store/partialindex/needrebuild'
changeset: 7:2b52832374dd changeset: 7:2b52832374dd
tag: tip tag: tip
parent: 4:587cd78c6d0e parent: 4:587cd78c6d0e
user: test user: test
date: Thu Jan 01 00:00:00 1970 +0000 date: Thu Jan 01 00:00:00 1970 +0000
summary: commitpermissionissue summary: commitpermissionissue
$ chmod u+w .hg/store/partialindex error happened while triggering rebuild: [Errno 13] Permission denied: '$TESTTMP/cloned2/.hg/partialindexneedrebuild'
$ chmod u+w .hg/
Temporarily disable the fastpartialmatch and make a commit. Temporarily disable the fastpartialmatch and make a commit.
Bump index generation number version and check that index was deleted after next Bump index generation number version and check that index was deleted after next
invocation invocation
$ echo disabled > disabled $ echo disabled > disabled
$ hg add disabled $ hg add disabled
$ hg --config extensions.fastpartialmatch=! ci -m disabled $ hg --config extensions.fastpartialmatch=! ci -m disabled