This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6772

remotefilelog: reduce probability of race-condition in remotefilelog tests
ClosedPublic

Authored by lothiraldan on Aug 29 2019, 8:50 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG2c74337e6483: remotefilelog: reduce probability of race-condition in remotefilelog tests
Summary

ca1014ad3de4 introduced a new parameter ensurestart to speed up
remotefilelog background processes start. Unfortunately it seems to have
increased the possibility of race-conditions in remotefilelog tests testing
those background processes.

With ensurestart=False, it seems that it's more probable to enter in a race
condition with debugwaitonprefetch and debugwaitonrepack in remotefilelog
background tests. Our CI seems to have a high probability of triggering this
race condition so make it configurable to ensure tests are stable.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

lothiraldan created this revision.Aug 29 2019, 8:50 AM
lothiraldan added a subscriber: durin42.Sep 3 2019, 4:15 AM

@durin42 Could you take a look? I think you are in the best condition to review this patch.

lothiraldan added a commit: rHG2c74337e6483: remotefilelog: reduce probability of race-condition in remotefilelog tests.Sep 3 2019, 11:40 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG2c74337e6483: remotefilelog: reduce probability of race-condition in remotefilelog tests (authored by lothiraldan). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 16334

hgext/remotefilelog/__init__.py

configitem('remotefilelog', 'gcrepack', default=False) configitem('remotefilelog', 'gcrepack', default=False)
configitem('remotefilelog', 'repackonhggc', default=False) configitem('remotefilelog', 'repackonhggc', default=False)
configitem('repack', 'chainorphansbysize', default=True, experimental=True) configitem('repack', 'chainorphansbysize', default=True, experimental=True)
configitem('packs', 'maxpacksize', default=0) configitem('packs', 'maxpacksize', default=0)
configitem('packs', 'maxchainlen', default=1000) configitem('packs', 'maxchainlen', default=1000)
configitem('devel', 'remotefilelog.ensurestart', default=False)
# default TTL limit is 30 days # default TTL limit is 30 days
_defaultlimit = 60 * 60 * 24 * 30 _defaultlimit = 60 * 60 * 24 * 30
configitem('remotefilelog', 'nodettl', default=_defaultlimit) configitem('remotefilelog', 'nodettl', default=_defaultlimit)
configitem('remotefilelog', 'data.gencountlimit', default=2), configitem('remotefilelog', 'data.gencountlimit', default=2),
configitem('remotefilelog', 'data.generations', configitem('remotefilelog', 'data.generations',
default=['1GB', '100MB', '1MB']) default=['1GB', '100MB', '1MB'])
configitem('remotefilelog', 'data.maxrepackpacks', default=50) configitem('remotefilelog', 'data.maxrepackpacks', default=50)
def pull(orig, ui, repo, *pats, **opts): def pull(orig, ui, repo, *pats, **opts):
result = orig(ui, repo, *pats, **opts) result = orig(ui, repo, *pats, **opts)
if isenabled(repo): if isenabled(repo):
# prefetch if it's configured # prefetch if it's configured
prefetchrevset = ui.config('remotefilelog', 'pullprefetch') prefetchrevset = ui.config('remotefilelog', 'pullprefetch')
bgrepack = repo.ui.configbool('remotefilelog', 'backgroundrepack') bgrepack = repo.ui.configbool('remotefilelog', 'backgroundrepack')
bgprefetch = repo.ui.configbool('remotefilelog', 'backgroundprefetch') bgprefetch = repo.ui.configbool('remotefilelog', 'backgroundprefetch')
ensurestart = repo.ui.configbool('devel', 'remotefilelog.ensurestart')
if prefetchrevset: if prefetchrevset:
ui.status(_("prefetching file contents\n")) ui.status(_("prefetching file contents\n"))
revs = scmutil.revrange(repo, [prefetchrevset]) revs = scmutil.revrange(repo, [prefetchrevset])
base = repo['.'].rev() base = repo['.'].rev()
if bgprefetch: if bgprefetch:
repo.backgroundprefetch(prefetchrevset, repack=bgrepack) repo.backgroundprefetch(prefetchrevset, repack=bgrepack,
ensurestart=ensurestart)
else: else:
repo.prefetch(revs, base=base) repo.prefetch(revs, base=base)
if bgrepack: if bgrepack:
repackmod.backgroundrepack(repo, incremental=True) repackmod.backgroundrepack(repo, incremental=True,
ensurestart=ensurestart)
elif bgrepack: elif bgrepack:
repackmod.backgroundrepack(repo, incremental=True) repackmod.backgroundrepack(repo, incremental=True,
ensurestart=ensurestart)
return result return result
def exchangepull(orig, repo, remote, *args, **kwargs): def exchangepull(orig, repo, remote, *args, **kwargs):
# Hook into the callstream/getbundle to insert bundle capabilities # Hook into the callstream/getbundle to insert bundle capabilities
# during a pull. # during a pull.
def localgetbundle(orig, source, heads=None, common=None, bundlecaps=None, def localgetbundle(orig, source, heads=None, common=None, bundlecaps=None,
**kwargs): **kwargs):
opts = pycompat.byteskwargs(opts) opts = pycompat.byteskwargs(opts)
if not isenabled(repo): if not isenabled(repo):
raise error.Abort(_("repo is not shallow")) raise error.Abort(_("repo is not shallow"))
opts = resolveprefetchopts(ui, opts) opts = resolveprefetchopts(ui, opts)
revs = scmutil.revrange(repo, opts.get('rev')) revs = scmutil.revrange(repo, opts.get('rev'))
repo.prefetch(revs, opts.get('base'), pats, opts) repo.prefetch(revs, opts.get('base'), pats, opts)
ensurestart = repo.ui.configbool('devel', 'remotefilelog.ensurestart')
# Run repack in background # Run repack in background
if opts.get('repack'): if opts.get('repack'):
repackmod.backgroundrepack(repo, incremental=True) repackmod.backgroundrepack(repo, incremental=True,
ensurestart=ensurestart)
@command('repack', [ @command('repack', [
('', 'background', None, _('run in a background process'), None), ('', 'background', None, _('run in a background process'), None),
('', 'incremental', None, _('do an incremental repack'), None), ('', 'incremental', None, _('do an incremental repack'), None),
('', 'packsonly', None, _('only repack packs (skip loose objects)'), None), ('', 'packsonly', None, _('only repack packs (skip loose objects)'), None),
], _('hg repack [OPTIONS]')) ], _('hg repack [OPTIONS]'))
def repack_(ui, repo, *pats, **opts): def repack_(ui, repo, *pats, **opts):
if opts.get(r'background'): if opts.get(r'background'):
ensurestart = repo.ui.configbool('devel', 'remotefilelog.ensurestart')
repackmod.backgroundrepack(repo, incremental=opts.get(r'incremental'), repackmod.backgroundrepack(repo, incremental=opts.get(r'incremental'),
packsonly=opts.get(r'packsonly', False)) packsonly=opts.get(r'packsonly', False),
ensurestart=ensurestart)
return return
options = {'packsonly': opts.get(r'packsonly')} options = {'packsonly': opts.get(r'packsonly')}
try: try:
if opts.get(r'incremental'): if opts.get(r'incremental'):
repackmod.incrementalrepack(repo, options=options) repackmod.incrementalrepack(repo, options=options)
else: else:
repackmod.fullrepack(repo, options=options) repackmod.fullrepack(repo, options=options)
except repackmod.RepackAlreadyRunning as ex: except repackmod.RepackAlreadyRunning as ex:
# Don't propogate the exception if the repack is already in # Don't propogate the exception if the repack is already in
# progress, since we want the command to exit 0. # progress, since we want the command to exit 0.
repo.ui.warn('%s\n' % ex) repo.ui.warn('%s\n' % ex)

hgext/remotefilelog/repack.py

shallowutil, shallowutil,
) )
osutil = policy.importmod(r'osutil') osutil = policy.importmod(r'osutil')
class RepackAlreadyRunning(error.Abort): class RepackAlreadyRunning(error.Abort):
pass pass
def backgroundrepack(repo, incremental=True, packsonly=False): def backgroundrepack(repo, incremental=True, packsonly=False,
ensurestart=False):
cmd = [procutil.hgexecutable(), '-R', repo.origroot, 'repack'] cmd = [procutil.hgexecutable(), '-R', repo.origroot, 'repack']
msg = _("(running background repack)\n") msg = _("(running background repack)\n")
if incremental: if incremental:
cmd.append('--incremental') cmd.append('--incremental')
msg = _("(running background incremental repack)\n") msg = _("(running background incremental repack)\n")
if packsonly: if packsonly:
cmd.append('--packsonly') cmd.append('--packsonly')
repo.ui.warn(msg) repo.ui.warn(msg)
# We know this command will find a binary, so don't block on it starting. # We know this command will find a binary, so don't block on it starting.
procutil.runbgcommand(cmd, encoding.environ, ensurestart=False) procutil.runbgcommand(cmd, encoding.environ, ensurestart=ensurestart)
def fullrepack(repo, options=None): def fullrepack(repo, options=None):
"""If ``packsonly`` is True, stores creating only loose objects are skipped. """If ``packsonly`` is True, stores creating only loose objects are skipped.
""" """
if util.safehasattr(repo, 'shareddatastores'): if util.safehasattr(repo, 'shareddatastores'):
datasource = contentstore.unioncontentstore( datasource = contentstore.unioncontentstore(
*repo.shareddatastores) *repo.shareddatastores)
historysource = metadatastore.unionmetadatastore( historysource = metadatastore.unionmetadatastore(

hgext/remotefilelog/shallowrepo.py

if fparent1 != nullid: if fparent1 != nullid:
files.append((f, hex(fparent1))) files.append((f, hex(fparent1)))
self.fileservice.prefetch(files) self.fileservice.prefetch(files)
return super(shallowrepository, self).commitctx(ctx, return super(shallowrepository, self).commitctx(ctx,
error=error, error=error,
origctx=origctx) origctx=origctx)
def backgroundprefetch(self, revs, base=None, repack=False, pats=None, def backgroundprefetch(self, revs, base=None, repack=False, pats=None,
opts=None): opts=None, ensurestart=False):
"""Runs prefetch in background with optional repack """Runs prefetch in background with optional repack
""" """
cmd = [procutil.hgexecutable(), '-R', repo.origroot, 'prefetch'] cmd = [procutil.hgexecutable(), '-R', repo.origroot, 'prefetch']
if repack: if repack:
cmd.append('--repack') cmd.append('--repack')
if revs: if revs:
cmd += ['-r', revs] cmd += ['-r', revs]
# We know this command will find a binary, so don't block # We know this command will find a binary, so don't block
# on it starting. # on it starting.
procutil.runbgcommand(cmd, encoding.environ, ensurestart=False) procutil.runbgcommand(cmd, encoding.environ,
ensurestart=ensurestart)
def prefetch(self, revs, base=None, pats=None, opts=None): def prefetch(self, revs, base=None, pats=None, opts=None):
"""Prefetches all the necessary file revisions for the given revs """Prefetches all the necessary file revisions for the given revs
Optionally runs repack in background Optionally runs repack in background
""" """
with repo._lock(repo.svfs, 'prefetchlock', True, None, None, with repo._lock(repo.svfs, 'prefetchlock', True, None, None,
_('prefetching in %s') % repo.origroot): _('prefetching in %s') % repo.origroot):
self._prefetch(revs, base, pats, opts) self._prefetch(revs, base, pats, opts)

tests/test-remotefilelog-bgprefetch.t

#require no-windows #require no-windows
$ . "$TESTDIR/remotefilelog-library.sh" $ . "$TESTDIR/remotefilelog-library.sh"
# devel.remotefilelog.ensurestart: reduce race condition with
# waiton{repack/prefetch}
$ cat >> $HGRCPATH <<EOF
> [devel]
> remotefilelog.ensurestart=True
> EOF
$ hg init master $ hg init master
$ cd master $ cd master
$ cat >> .hg/hgrc <<EOF $ cat >> .hg/hgrc <<EOF
> [remotefilelog] > [remotefilelog]
> server=True > server=True
> EOF > EOF
$ echo x > x $ echo x > x

tests/test-remotefilelog-repack-fast.t

#require no-windows #require no-windows
$ . "$TESTDIR/remotefilelog-library.sh" $ . "$TESTDIR/remotefilelog-library.sh"
# devel.remotefilelog.ensurestart: reduce race condition with
# waiton{repack/prefetch}
$ cat >> $HGRCPATH <<EOF $ cat >> $HGRCPATH <<EOF
> [remotefilelog] > [remotefilelog]
> fastdatapack=True > fastdatapack=True
> [devel]
> remotefilelog.ensurestart=True
> EOF > EOF
$ hg init master $ hg init master
$ cd master $ cd master
$ cat >> .hg/hgrc <<EOF $ cat >> .hg/hgrc <<EOF
> [remotefilelog] > [remotefilelog]
> server=True > server=True
> serverexpiration=-1 > serverexpiration=-1

tests/test-remotefilelog-repack.t

#require no-windows #require no-windows
$ . "$TESTDIR/remotefilelog-library.sh" $ . "$TESTDIR/remotefilelog-library.sh"
# devel.remotefilelog.ensurestart: reduce race condition with
# waiton{repack/prefetch}
$ cat >> $HGRCPATH <<EOF
> [devel]
> remotefilelog.ensurestart=True
> EOF
$ hg init master $ hg init master
$ cd master $ cd master
$ cat >> .hg/hgrc <<EOF $ cat >> .hg/hgrc <<EOF
> [remotefilelog] > [remotefilelog]
> server=True > server=True
> serverexpiration=-1 > serverexpiration=-1
> EOF > EOF