This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1453

remotefilelog: never deadlock on writing to a pipe
Needs RevisionPublic

Authored by ikostia on Nov 19 2017, 3:59 PM.
Tags
None
Subscribers
durham, wlis

Details

Reviewers
durham
Group Reviewers
Restricted Project
Summary

Instead of guesstimating that 10000 (or any fixed number) steps
always fit in a buffer, we can just count the bytes as we write.

Depends on D1436

Test Plan
  • not tested yet

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Branch
default
Lint
Lint OK
Unit
Unit Tests OK

Event Timeline

ikostia created this revision.Nov 19 2017, 3:59 PM
wlis added a subscriber: wlis.Nov 20 2017, 12:16 PM
wlis added inline comments.
remotefilelog/fileserverclient.py
230

Probably remove the "True"

372

Is there a way to get this value from the OS, in case someone has some custom setting for the default pipe size?

ikostia updated this revision to Diff 3663.Nov 20 2017, 12:27 PM

remove unnecessary condition

durham requested changes to this revision.Nov 20 2017, 1:37 PM
durham added a subscriber: durham.

Have you tested the performance impact of this change? Previously we would issue 10k at a time, now if the buffer is 65k and if each entry is 100 bytes (40 byte hash + 60 byte path) then we're only doing 650 at a time. You may need to set remotefilelog.cacheprocess= to force it to fallback to normal ssh fetching to execute this code path in bulk.

remotefilelog/fileserverclient.py
238

i += 1

This revision now requires changes to proceed.Nov 20 2017, 1:37 PM

Revision Contents
Changeset List

Diff 3663

remotefilelog/fileserverclient.py

b.submit() b.submit()
for m, v in zip(chunk, b.results()): for m, v in zip(chunk, b.results()):
file_ = idmap[m] file_ = idmap[m]
node = m[-40:] node = m[-40:]
receivemissing(io.BytesIO('%d\n%s' % (len(v), v)), file_, node) receivemissing(io.BytesIO('%d\n%s' % (len(v), v)), file_, node)
progresstick() progresstick()
def _getfiles( def _getfiles(
remote, receivemissing, progresstick, missed, idmap, step): remote, receivemissing, progresstick, missed, idmap, bufsize):
remote._callstream("getfiles") remote._callstream("getfiles")
i = 0 i = 0
pipeo = shallowutil.trygetattr(remote, ('_pipeo', 'pipeo')) pipeo = shallowutil.trygetattr(remote, ('_pipeo', 'pipeo'))
pipei = shallowutil.trygetattr(remote, ('_pipei', 'pipei')) pipei = shallowutil.trygetattr(remote, ('_pipei', 'pipei'))
while i < len(missed): while i < len(missed):
# issue a batch of requests # issue a batch of requests
start = i start = i
end = min(len(missed), start + step) written = 0
i = end while i < len(missed):
wlisUnsubmitted
Not Done

Probably remove the "True"

wlis: Probably remove the "True"
for missingid in missed[start:end]: missingid = missed[i]
# issue new request
versionid = missingid[-40:] versionid = missingid[-40:]
file = idmap[missingid] file = idmap[missingid]
sshrequest = "%s%s\n" % (versionid, file) sshrequest = "%s%s\n" % (versionid, file)
if len(sshrequest) > bufsize - written:
break
pipeo.write(sshrequest) pipeo.write(sshrequest)
i = i + 1
durhamUnsubmitted
Not Done

i += 1

durham: `i += 1`
written += len(sshrequest)
pipeo.flush() pipeo.flush()
# receive batch results # receive batch results
for missingid in missed[start:end]: for missingid in missed[start:i]:
versionid = missingid[-40:] versionid = missingid[-40:]
file = idmap[missingid] file = idmap[missingid]
receivemissing(pipei, file, versionid) receivemissing(pipei, file, versionid)
progresstick() progresstick()
# End the command # End the command
pipeo.write('\n') pipeo.write('\n')
pipeo.flush() pipeo.flush()
with self._connect() as conn: with self._connect() as conn:
remote = conn.peer remote = conn.peer
# TODO: deduplicate this with the constant in # TODO: deduplicate this with the constant in
# shallowrepo # shallowrepo
if remote.capable("remotefilelog"): if remote.capable("remotefilelog"):
if not isinstance(remote, sshpeer.sshpeer): if not isinstance(remote, sshpeer.sshpeer):
raise error.Abort('remotefilelog requires ssh ' raise error.Abort('remotefilelog requires ssh '
'servers') 'servers')
step = self.ui.configint('remotefilelog', # by default, Linux has a pipe capacity of 65536,
'getfilesstep', 10000) # while Windows - 4096. However, Windows pipe
# buffer size is now configurable
bufsize = 65536
wlisUnsubmitted
Not Done

Is there a way to get this value from the OS, in case someone has some custom setting for the default pipe size?

wlis: Is there a way to get this value from the OS, in case someone has some custom setting for the…
if os.name == 'nt':
bufsize = self.ui.configint('experimental',
'winpipebufsize',
4096)
_getfiles(remote, self.receivemissing, progresstick, _getfiles(remote, self.receivemissing, progresstick,
missed, idmap, step) missed, idmap, bufsize)
elif remote.capable("getfile"): elif remote.capable("getfile"):
if remote.capable('batch'): if remote.capable('batch'):
batchdefault = 100 batchdefault = 100
else: else:
batchdefault = 10 batchdefault = 10
batchsize = self.ui.configint( batchsize = self.ui.configint(
'remotefilelog', 'batchsize', batchdefault) 'remotefilelog', 'batchsize', batchdefault)
_getfilesbatch( _getfilesbatch(

tests/test-check-config-hg.t

undocumented: remotefilelog.backgroundrepack (bool) undocumented: remotefilelog.backgroundrepack (bool)
undocumented: remotefilelog.cachegroup (str) undocumented: remotefilelog.cachegroup (str)
undocumented: remotefilelog.data.gencountlimit (int) [2] undocumented: remotefilelog.data.gencountlimit (int) [2]
undocumented: remotefilelog.debug (bool) undocumented: remotefilelog.debug (bool)
undocumented: remotefilelog.excludepattern (list) undocumented: remotefilelog.excludepattern (list)
undocumented: remotefilelog.fastdatapack (bool) undocumented: remotefilelog.fastdatapack (bool)
undocumented: remotefilelog.fetchpacks (bool) undocumented: remotefilelog.fetchpacks (bool)
undocumented: remotefilelog.fetchwarning (str) undocumented: remotefilelog.fetchwarning (str)
undocumented: remotefilelog.getfilesstep (int) [10000]
undocumented: remotefilelog.history.gencountlimit (int) [2] undocumented: remotefilelog.history.gencountlimit (int) [2]
undocumented: remotefilelog.includepattern (list) undocumented: remotefilelog.includepattern (list)
undocumented: remotefilelog.pullprefetch (str) undocumented: remotefilelog.pullprefetch (str)
undocumented: remotefilelog.reponame (str) undocumented: remotefilelog.reponame (str)
undocumented: remotefilelog.server (bool) undocumented: remotefilelog.server (bool)
undocumented: remotefilelog.servercachepath (str) undocumented: remotefilelog.servercachepath (str)
undocumented: remotefilelog.serverexpiration (int) [30] undocumented: remotefilelog.serverexpiration (int) [30]
undocumented: remotefilelog.shallowtrees (bool) undocumented: remotefilelog.shallowtrees (bool)