This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1572

infinitepush: be compatible with upstream change
ClosedPublic

Authored by quark on Dec 1 2017, 4:18 PM.
Tags
None
Subscribers
singhsrb, durham

Details

Reviewers
durham
singhsrb
Group Reviewers
Restricted Project
Commits
rFBHGXe3c118224daf: infinitepush: be compatible with upstream change
Summary

The upstream has made the bundle parts non-seekable to reduce memory usage.
So we need to copy a bundle part if we need to process it later.

Also, fix an apparent issue where part should be scratchbookpart.

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

quark created this revision.Dec 1 2017, 4:18 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 1 2017, 4:18 PM
durham accepted this revision.Dec 1 2017, 4:22 PM
durham added a subscriber: durham.
durham added inline comments.
infinitepush/__init__.py
1093

I wonder if we could use your copy function here too. This code (below) copies the parameters individually, which might be safer than reusing the dictionaries.

No need to do so now. Just an observation at a common copy pattern.

This revision is now accepted and ready to land.Dec 1 2017, 4:22 PM
singhsrb accepted this revision.Dec 1 2017, 4:28 PM
singhsrb added a subscriber: singhsrb.
singhsrb added inline comments.
infinitepush/__init__.py
1136

Nice!

Closed by commit rFBHGXe3c118224daf: infinitepush: be compatible with upstream change (authored by quark). · Explain WhyDec 4 2017, 9:00 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Minfinitepush/__init__.py (9 lines)
Minfinitepush/bundleparts.py (25 lines)

Diff 4110

infinitepush/__init__.py

# alert the future pushkey and phase-heads handler to skip # alert the future pushkey and phase-heads handler to skip
# the part. # the part.
if not handleallparts: if not handleallparts:
op.records.add(scratchbranchparttype + '_skippushkey', True) op.records.add(scratchbranchparttype + '_skippushkey', True)
op.records.add(scratchbranchparttype + '_skipphaseheads', op.records.add(scratchbranchparttype + '_skipphaseheads',
True) True)
elif part.type == scratchbookmarksparttype: elif part.type == scratchbookmarksparttype:
# Save this for later processing. Details below. # Save this for later processing. Details below.
scratchbookpart = part #
# Upstream https://phab.mercurial-scm.org/D1389 and its
# follow-ups stop part.seek support to reduce memory usage
# (https://bz.mercurial-scm.org/5691). So we need to copy
# the part so it can be consumed later.
scratchbookpart = bundleparts.copiedpart(part)
else: else:
if handleallparts or part.type in partforwardingwhitelist: if handleallparts or part.type in partforwardingwhitelist:
# Ideally we would not process any parts, and instead just # Ideally we would not process any parts, and instead just
# forward them to the bundle for storage, but since this # forward them to the bundle for storage, but since this
# differs from previous behavior, we need to put it behind a # differs from previous behavior, we need to put it behind a
# config flag for incremental rollout. # config flag for incremental rollout.
bundlepart = bundle2.bundlepart(part.type, data=part.read()) bundlepart = bundle2.bundlepart(part.type, data=part.read())
durhamUnsubmitted
Not Done

I wonder if we could use your copy function here too. This code (below) copies the parameters individually, which might be safer than reusing the dictionaries.

No need to do so now. Just an observation at a common copy pattern.

durham: I wonder if we could use your copy function here too. This code (below) copies the parameters…
for key, value in part.params.iteritems(): for key, value in part.params.iteritems():
bundlepart.addparam(key, value) bundlepart.addparam(key, value)
# Certain parts require a response # Certain parts require a response
if part.type == 'pushkey': if part.type == 'pushkey':
if op.reply is not None: if op.reply is not None:
rpart = op.reply.newpart('reply:pushkey') rpart = op.reply.newpart('reply:pushkey')
rpart.addparam('in-reply-to', str(part.id), rpart.addparam('in-reply-to', str(part.id),
except Exception: except Exception:
# we would rather see the original exception # we would rather see the original exception
pass pass
# The scratch bookmark part is sent as part of a push backup. It needs to be # The scratch bookmark part is sent as part of a push backup. It needs to be
# processed after the main bundle has been stored, so that any commits it # processed after the main bundle has been stored, so that any commits it
# references are available in the store. # references are available in the store.
if scratchbookpart: if scratchbookpart:
bundle2._processpart(op, part) bundle2._processpart(op, scratchbookpart)
singhsrbUnsubmitted
Not Done

Nice!

singhsrb: Nice!
def storebundle(op, params, bundlefile): def storebundle(op, params, bundlefile):
log = _getorcreateinfinitepushlogger(op) log = _getorcreateinfinitepushlogger(op)
parthandlerstart = time.time() parthandlerstart = time.time()
log(scratchbranchparttype, eventtype='start') log(scratchbranchparttype, eventtype='start')
index = op.repo.bundlestore.index index = op.repo.bundlestore.index
store = op.repo.bundlestore.store store = op.repo.bundlestore.store
op.records.add(scratchbranchparttype + '_skippushkey', True) op.records.add(scratchbranchparttype + '_skippushkey', True)

infinitepush/bundleparts.py

# Copyright 2017 Facebook, Inc. # Copyright 2017 Facebook, Inc.
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from .common import ( from .common import (
encodebookmarks, encodebookmarks,
isremotebooksenabled, isremotebooksenabled,
) )
from mercurial import ( from mercurial import (
bundle2, bundle2,
changegroup, changegroup,
error, error,
extensions, extensions,
revsetlang, revsetlang,
util,
) )
from mercurial.i18n import _ from mercurial.i18n import _
scratchbranchparttype = 'b2x:infinitepush' scratchbranchparttype = 'b2x:infinitepush'
scratchbookmarksparttype = 'b2x:infinitepushscratchbookmarks' scratchbookmarksparttype = 'b2x:infinitepushscratchbookmarks'
def getscratchbranchparts(repo, peer, outgoing, confignonforwardmove, def getscratchbranchparts(repo, peer, outgoing, confignonforwardmove,
ui, bookmark, create): ui, bookmark, create):
to make sure large files are uploaded to lfs to make sure large files are uploaded to lfs
''' '''
try: try:
lfsmod = extensions.find('lfs') lfsmod = extensions.find('lfs')
lfsmod.wrapper.uploadblobsfromrevs(repo, missing) lfsmod.wrapper.uploadblobsfromrevs(repo, missing)
except KeyError: except KeyError:
# Ignore if lfs extension is not enabled # Ignore if lfs extension is not enabled
return return
class copiedpart(object):
"""a copy of unbundlepart content that can be consumed later"""
def __init__(self, part):
# copy "public properties"
self.type = part.type
self.id = part.id
self.mandatory = part.mandatory
self.mandatoryparams = part.mandatoryparams
self.advisoryparams = part.advisoryparams
self.params = part.params
self.mandatorykeys = part.mandatorykeys
# copy the buffer
self._io = util.stringio(part.read())
def consume(self):
return
def read(self, size=None):
if size is None:
return self._io.read()
else:
return self._io.read(size)