This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1416

prefetchtrees: add option to repack prefetched trees
ClosedPublic

Authored by singhsrb on Nov 14 2017, 3:43 PM.
Tags
None
Subscribers
durham

Details

Reviewers
durham
Group Reviewers
Restricted Project
Commits
rFBHGX1ebd6b179b2c: prefetchtrees: add option to repack prefetched trees
Summary

The prefetch command has an option to repack the prefetched files.
Eventually, we plan to merge prefetch and prefetchtrees into a single
command and therefore, this commit takes a step towards making the interface to
these commands exactly the same.

Test Plan

Ran all the 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

singhsrb created this revision.Nov 14 2017, 3:43 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 14 2017, 3:43 PM
singhsrb added a comment.EditedNov 14 2017, 3:50 PM

This is only an intermediate step to help out with the review. It might be useful to checkout D1417 first to get an idea of what this leads to.

durham requested changes to this revision.Nov 14 2017, 7:28 PM
durham added a subscriber: durham.
durham added inline comments.
tests/test-treemanifest-prefetch.t
378

We need to verify that the repack succeeded. Maybe we could first prefetch commit 0, then prefetch 2+4 and verify that they were all repacked together after debugwaitonrepack exits?

This revision now requires changes to proceed.Nov 14 2017, 7:28 PM
singhsrb planned changes to this revision.Nov 14 2017, 8:26 PM
singhsrb added inline comments.
tests/test-treemanifest-prefetch.t
378

I wasn't sure how to do that. That suggested strategy seems like a reasonable enough way of going about it.

singhsrb updated this revision to Diff 3533.Nov 15 2017, 3:06 PM
durham accepted this revision.Nov 15 2017, 6:03 PM
durham added inline comments.
tests/test-treemanifest-prefetch.t
386

Since all we're verifying is that there should be one pack, doing ls_l $CACHEDIR/master/packs/manifests/*.dataidx | wc -l might be more concise.

This revision is now accepted and ready to land.Nov 15 2017, 6:03 PM
singhsrb added inline comments.Nov 15 2017, 6:06 PM
tests/test-treemanifest-prefetch.t
386

That's a great idea!

singhsrb updated this revision to Diff 3538.Nov 15 2017, 6:33 PM
Closed by commit rFBHGX1ebd6b179b2c: prefetchtrees: add option to repack prefetched trees (authored by singhsrb). · Explain WhyNov 20 2017, 9:54 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 3638

tests/test-treemanifest-prefetch.t

z z
Test prefetch non-parent commits with no base node (should fetch minimal Test prefetch non-parent commits with no base node (should fetch minimal
trees - in this case 3 trees for commit 2, and 2 for commit 4 despite it having trees - in this case 3 trees for commit 2, and 2 for commit 4 despite it having
3 directories) 3 directories)
$ rm -rf $CACHEDIR/master $ rm -rf $CACHEDIR/master
$ hg prefetchtrees -r '2 + 4' $ hg prefetchtrees -r '2 + 4'
5 trees fetched over * (glob) 5 trees fetched over * (glob)
Test repack option
$ rm -rf $CACHEDIR/master
$ hg prefetchtrees -r '0'
2 trees fetched over *s (glob)
$ hg prefetchtrees -r '2'
3 trees fetched over *s (glob)
durhamUnsubmitted
Not Done

We need to verify that the repack succeeded. Maybe we could first prefetch commit 0, then prefetch 2+4 and verify that they were all repacked together after debugwaitonrepack exits?

durham: We need to verify that the repack succeeded. Maybe we could first prefetch commit 0, then…
singhsrbAuthorUnsubmitted
Not Done

I wasn't sure how to do that. That suggested strategy seems like a reasonable enough way of going about it.

singhsrb: I wasn't sure how to do that. That suggested strategy seems like a reasonable enough way of…
$ hg prefetchtrees -r '4' --repack
3 trees fetched over *s (glob)
(running background incremental repack)
$ sleep 0.5
$ hg debugwaitonrepack
$ ls_l $CACHEDIR/master/packs/manifests | grep datapack | wc -l
durhamUnsubmitted
Not Done

Since all we're verifying is that there should be one pack, doing ls_l $CACHEDIR/master/packs/manifests/*.dataidx | wc -l might be more concise.

durham: Since all we're verifying is that there should be one pack, doing `ls_l…
singhsrbAuthorUnsubmitted
Not Done

That's a great idea!

singhsrb: That's a great idea!
\s*1 (re)

treemanifest/__init__.py

mutablehistorypack, mutablehistorypack,
) )
from remotefilelog import shallowrepo, shallowutil, wirepack from remotefilelog import shallowrepo, shallowutil, wirepack
from remotefilelog.repack import ( from remotefilelog.repack import (
_computeincrementaldatapack, _computeincrementaldatapack,
_computeincrementalhistorypack, _computeincrementalhistorypack,
_runrepack, _runrepack,
_topacks, _topacks,
backgroundrepack,
) )
import cstore import cstore
import os import os
import struct import struct
import time import time
osutil = policy.importmod(r'osutil') osutil = policy.importmod(r'osutil')
break break
if currcls is object: if currcls is object:
raise AttributeError(_("%s has no property '%s'") % raise AttributeError(_("%s has no property '%s'") %
(type(currcls), propname)) (type(currcls), propname))
@command('prefetchtrees', [ @command('prefetchtrees', [
('r', 'rev', '', _("revs to prefetch the trees for")), ('r', 'rev', '', _("revs to prefetch the trees for")),
('', 'repack', False, _('run repack after prefetch')),
('b', 'base', '', _("rev that is assumed to already be local")), ('b', 'base', '', _("rev that is assumed to already be local")),
] + commands.walkopts, _('--rev REVS PATTERN..')) ] + commands.walkopts, _('--rev REVS PATTERN..'))
def prefetchtrees(ui, repo, *args, **opts): def prefetchtrees(ui, repo, *args, **opts):
revs = repo.revs(opts.get('rev')) revs = repo.revs(opts.get('rev'))
mfnodes = set() mfnodes = set()
for rev in revs: for rev in revs:
mfnodes.add(repo[rev].manifestnode()) mfnodes.add(repo[rev].manifestnode())
basemfnode = set() basemfnode = set()
base = opts.get('base') base = opts.get('base')
if base: if base:
basemfnode.add(repo[base].manifestnode()) basemfnode.add(repo[base].manifestnode())
_prefetchtrees(repo, '', mfnodes, basemfnode, []) _prefetchtrees(repo, '', mfnodes, basemfnode, [])
# Run repack in background
if opts.get('repack'):
backgroundrepack(repo, incremental=True)
def _prefetchtrees(repo, rootdir, mfnodes, basemfnodes, directories): def _prefetchtrees(repo, rootdir, mfnodes, basemfnodes, directories):
# If possible, use remotefilelog's more expressive fallbackpath # If possible, use remotefilelog's more expressive fallbackpath
if util.safehasattr(repo, 'fallbackpath'): if util.safehasattr(repo, 'fallbackpath'):
fallbackpath = repo.fallbackpath fallbackpath = repo.fallbackpath
else: else:
fallbackpath = repo.ui.config('paths', 'default') fallbackpath = repo.ui.config('paths', 'default')
start = time.time() start = time.time()