This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1804

treemanifest: add a server side cache for tree entries
AbandonedPublic

Authored by durham on Jan 2 2018, 7:41 PM.
Tags
None
Subscribers
quark

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

We're seeing high load on the server when many clients are fetching treemanifest
entries. A lot of this stems from the cost of opening revlogs. Let's introduce a
simple on-disk cache that let's us avoid opening revlogs as much as possible.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

durham created this revision.Jan 2 2018, 7:41 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 2 2018, 7:41 PM
durham added inline comments.Jan 2 2018, 7:42 PM
treemanifest/__init__.py
1821

I need to add cache cleanup here, but I wanted to get the review started.

quark added a subscriber: quark.Jan 5 2018, 4:36 PM

Do we have ideas about the detailed profiling result of what's slow? Is it building revlog index, or applying deltas?

The cache seems suboptimal on disk usage and inode count. Will simplecache be a sane choice here?

treemanifest/__init__.py
1760–1762

Should {} be getmeta? I guess getdelta won't be called in critical path so it's okay to be more expensive.

durham abandoned this revision.Jan 24 2018, 1:20 PM

Revision Contents
Changeset List

Diff 4683

tests/test-treemanifest-server.t

saved backup bundle to $TESTTMP/master/.hg/strip-backup/4fd4fee9fca1-46b625db-backup.hg (glob) saved backup bundle to $TESTTMP/master/.hg/strip-backup/4fd4fee9fca1-46b625db-backup.hg (glob)
$ hg debugindex .hg/store/00manifesttree.i $ hg debugindex .hg/store/00manifesttree.i
rev offset length delta linkrev nodeid p1 p2 rev offset length delta linkrev nodeid p1 p2
0 0 50 -1 0 85b359fdb09e 000000000000 000000000000 0 0 50 -1 0 85b359fdb09e 000000000000 000000000000
1 50 62 0 1 7e680cec965b 85b359fdb09e 000000000000 1 50 62 0 1 7e680cec965b 85b359fdb09e 000000000000
$ hg debugindex .hg/store/meta/subdir/00manifest.i $ hg debugindex .hg/store/meta/subdir/00manifest.i
rev offset length delta linkrev nodeid p1 p2 rev offset length delta linkrev nodeid p1 p2
0 0 44 -1 0 bc0c2c938b92 000000000000 000000000000 0 0 44 -1 0 bc0c2c938b92 000000000000 000000000000
Test fetching from the server populates the cache
$ cd ../
$ hgcloneshallow ssh://user@dummy/master client2 -q -U
$ cd client2
$ cat >> .hg/hgrc <<EOF
> [extensions]
> treemanifest=$TESTDIR/../treemanifest
> fastmanifest=$TESTDIR/../fastmanifest
> [fastmanifest]
> usetree=True
> usecache=False
> [treemanifest]
> demanddownload=True
> sendtrees=True
> EOF
$ clearcache
$ hg status --change tip > /dev/null
2 trees fetched over * (glob)
2 trees fetched over * (glob)
$ ls_l ../master/.hg/cache/trees/v1/get
-rw-r--r-- 69 0b0fa4abc415aa6a46e003c61283b182ccc989b6
-rw-r--r-- 63 bfd3db72113838ac7ebcf260374e4bf2884b3ddd
-rw-r--r-- 63 d4395b5ffa18499864439ac2b1a731ff7b7491fa
-rw-r--r-- 119 db19d9b3947b6a67f2c1a7530fb4ff1b5f52cc12
$ ls_l ../master/.hg/cache/trees/v1/nodeinfo
-rw-r--r-- 84 0b0fa4abc415aa6a46e003c61283b182ccc989b6
-rw-r--r-- 84 6768033e216468247bd031a0a2d9876d79818f8f
-rw-r--r-- 84 bfd3db72113838ac7ebcf260374e4bf2884b3ddd
-rw-r--r-- 84 d4395b5ffa18499864439ac2b1a731ff7b7491fa
-rw-r--r-- 84 db19d9b3947b6a67f2c1a7530fb4ff1b5f52cc12
- Move the revlogs away to show that the cache is answering prefetches
$ mv ../master/.hg/store/meta ../master/.hg/store/meta.bak
$ clearcache
$ hg status --change tip > /dev/null
2 trees fetched over * (glob)
2 trees fetched over * (glob)
$ mv ../master/.hg/store/meta.bak ../master/.hg/store/meta

treemanifest/__init__.py

Setting `treemanifest.treeonly` to True will force all manifest reads to use the Setting `treemanifest.treeonly` to True will force all manifest reads to use the
tree format. This is useful in the final stages of a migration to treemanifest tree format. This is useful in the final stages of a migration to treemanifest
to prevent accesses of flat manifests. to prevent accesses of flat manifests.
[treemanifest] [treemanifest]
treeonly = True treeonly = True
`treemanifest.cacheserverstore` causes the treemanifest server to store a cache
of treemanifest revisions in individual files. These improve lookup speed since
we don't have to open a revlog.
[treemanifest]
cacheserverstore = True
""" """
from mercurial import ( from mercurial import (
bundle2, bundle2,
bundlerepo, bundlerepo,
changegroup, changegroup,
commands, commands,
error, error,
_computeincrementaldatapack, _computeincrementaldatapack,
_computeincrementalhistorypack, _computeincrementalhistorypack,
_runrepack, _runrepack,
_topacks, _topacks,
backgroundrepack, backgroundrepack,
) )
import cstore import cstore
import hashlib
import os import os
import shutil import shutil
import struct import struct
import time import time
osutil = policy.importmod(r'osutil') osutil = policy.importmod(r'osutil')
cmdtable = {} cmdtable = {}
command = registrar.command(cmdtable) command = registrar.command(cmdtable)
configtable = {} configtable = {}
configitem = registrar.configitem(configtable) configitem = registrar.configitem(configtable)
configitem('treemanifest', 'sendtrees', default=False) configitem('treemanifest', 'sendtrees', default=False)
configitem('treemanifest', 'server', default=False) configitem('treemanifest', 'server', default=False)
configitem('treemanifest', 'cacheserverstore', default=True)
PACK_CATEGORY='manifests' PACK_CATEGORY='manifests'
TREEGROUP_PARTTYPE = 'b2x:treegroup' TREEGROUP_PARTTYPE = 'b2x:treegroup'
# Temporary part type while we migrate the arguments # Temporary part type while we migrate the arguments
TREEGROUP_PARTTYPE2 = 'b2x:treegroup2' TREEGROUP_PARTTYPE2 = 'b2x:treegroup2'
RECEIVEDNODE_RECORD = 'receivednodes' RECEIVEDNODE_RECORD = 'receivednodes'
def setuptreestores(repo, mfl): def setuptreestores(repo, mfl):
if repo.ui.configbool("treemanifest", "server"): if repo.ui.configbool("treemanifest", "server"):
packpath = repo.vfs.join('cache/packs/%s' % PACK_CATEGORY) packpath = repo.vfs.join('cache/packs/%s' % PACK_CATEGORY)
# Data store # Data store
datastore = cstore.datapackstore(packpath) datastore = cstore.datapackstore(packpath)
revlogstore = manifestrevlogstore(repo) revlogstore = manifestrevlogstore(repo)
if repo.ui.configbool("treemanifest", "cacheserverstore"):
revlogstore = cachestore(revlogstore, repo.cachevfs)
mfl.datastore = unioncontentstore(datastore, revlogstore) mfl.datastore = unioncontentstore(datastore, revlogstore)
# History store # History store
historystore = historypackstore(repo.ui, packpath) historystore = historypackstore(repo.ui, packpath)
mfl.historystore = unionmetadatastore( mfl.historystore = unionmetadatastore(
historystore, historystore,
revlogstore, revlogstore,
) )
mfl.historystore = unionmetadatastore( mfl.historystore = unionmetadatastore(
tempstore, tempstore,
mfl.historystore) mfl.historystore)
else: else:
orig(self, bundle, part) orig(self, bundle, part)
class MissingNodesError(error.Abort): class MissingNodesError(error.Abort):
def __init__(self, nodes, message=None, hint=None): def __init__(self, nodes, message=None, hint=None):
nodes = list(nodes)
nodestr = '\n'.join(hex(mfnode) for mfnode in nodes[:10]) nodestr = '\n'.join(hex(mfnode) for mfnode in nodes[:10])
if len(nodes) > 10: if len(nodes) > 10:
nodestr += '\n...' nodestr += '\n...'
fullmessage = _( fullmessage = _(
'unable to download the following trees from the server:') 'unable to download the following trees from the server:')
if message is not None: if message is not None:
fullmessage += ' ' + message fullmessage += ' ' + message
fullmessage += '\n' + nodestr fullmessage += '\n' + nodestr
super(MissingNodesError, self).__init__(fullmessage, hint=hint) super(MissingNodesError, self).__init__(fullmessage, hint=hint)
NODEINFOFORMAT='!20s20s20sI'
NODEINFOLEN=struct.calcsize(NODEINFOFORMAT)
class cachestore(object):
def __init__(self, store, vfs, version=1):
self.store = store
self.vfs = vfs
self.version = version
def _key(self, name, node, category):
shakey = hex(hashlib.sha1(name + node).digest())
return os.path.join('trees', 'v' + str(self.version), category, shakey)
def get(self, name, node):
if node == nullid:
return ''
try:
key = self._key(name, node, 'get')
return self._read(key)
except (IOError, OSError):
data = self.store.get(name, node)
self._write(key, data)
return data
def getdelta(self, name, node):
revision = self.get(name, node)
return (revision, name, nullid, {})
quarkUnsubmitted
Not Done

Should {} be getmeta? I guess getdelta won't be called in critical path so it's okay to be more expensive.

quark: Should `{}` be `getmeta`? I guess `getdelta` won't be called in critical path so it's okay to…
def getdeltachain(self, name, node):
revision = self.get(name, node)
return [(name, node, None, nullid, revision)]
def getmeta(self, name, node):
return self.store.getmeta(name, node)
def getmissing(self, keys):
missing = []
for name, node in keys:
if not self.vfs.exists(self._key(name, node, 'get')):
missing.append((name, node))
return self.store.getmissing(missing)
def getancestors(self, name, node, known=None):
return self.store.getancestors(name, node, known=known)
def _serializenodeinfo(self, nodeinfo):
p1, p2, linknode, copyfrom = nodeinfo
if copyfrom is None:
copyfrom = ''
raw = struct.pack(NODEINFOFORMAT, p1, p2, linknode, len(copyfrom))
return raw + copyfrom
def _deserializenodeinfo(self, raw):
p1, p2, linknode, copyfromlen = struct.unpack_from(NODEINFOFORMAT, raw,
0)
if len(raw) != NODEINFOLEN + copyfromlen:
raise IOError("invalid nodeinfo serialization: %s %s %s %s %s" %
(hex(p1), hex(p2), hex(linknode), str(copyfromlen),
raw[NODEINFOLEN:]))
return p1, p2, linknode, raw[NODEINFOLEN:NODEINFOLEN + copyfromlen]
def _verifyvalue(self, value):
sha, value = value[:20], value[20:]
realsha = hashlib.sha1(value).digest()
if sha != realsha:
raise IOError()
return value
def _read(self, key):
with self.vfs(key) as f:
raw = f.read()
if raw == '':
raise IOError("missing file contents: %s" % self.vfs.join(key))
sha, value = raw[:20], raw[20:]
realsha = hashlib.sha1(value).digest()
if sha != realsha:
raise IOError("invalid file contents: %s" % self.vfs.join(key))
return value
def _write(self, key, value):
with self.vfs(key, 'w+') as f:
sha = hashlib.sha1(value).digest()
f.write(sha)
f.write(value)
durhamAuthorUnsubmitted
Not Done

I need to add cache cleanup here, but I wanted to get the review started.

durham: I need to add cache cleanup here, but I wanted to get the review started.
def getnodeinfo(self, name, node):
key = self._key(name, node, 'nodeinfo')
try:
raw = self._read(key)
return self._deserializenodeinfo(raw)
except (IOError, OSError):
nodeinfo = self.store.getnodeinfo(name, node)
self._write(key, self._serializenodeinfo(nodeinfo))
return nodeinfo