This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1505

infinitepush: skip fastheadsdiscovery heuristic
ClosedPublic

Authored by stash on Nov 24 2017, 6:58 AM.
Tags
None
Subscribers
durham

Details

Reviewers
mbthomas
Group Reviewers
Restricted Project
Commits
rFBHGX23171cd2898a: infinitepush: skip fastheadsdiscovery heuristic
Summary

remotenames extension has a heuristic that speeds up discovery. Unfortunately
it may result in sending public commits to infinitepush server. This diff
intentionally disables this heuristic for infinitepush backups.

Test Plan

Run 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

stash created this revision.Nov 24 2017, 6:58 AM
mbthomas accepted this revision.Nov 24 2017, 12:35 PM
This revision is now accepted and ready to land.Nov 24 2017, 12:35 PM
durham added a subscriber: durham.Nov 28 2017, 11:11 AM

When were we seeing cases where there were public commits without a remote bookmark pointing at them?

stash added a comment.Nov 30 2017, 4:47 AM

When were we seeing cases where there were public commits without a remote bookmark pointing at them?

@durham I saw them when people did hg up FULL_COMMIT_HASH and download commit from infinitepush that is based on a public commit which is not in the repo.

Closed by commit rFBHGX23171cd2898a: infinitepush: skip fastheadsdiscovery heuristic (authored by stash). · Explain WhyNov 30 2017, 8:22 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 4027

infinitepush/backupcommands.py

def _sendbundle(bundler, other): def _sendbundle(bundler, other):
stream = util.chunkbuffer(bundler.getchunks()) stream = util.chunkbuffer(bundler.getchunks())
try: try:
other.unbundle(stream, ['force'], other.url()) other.unbundle(stream, ['force'], other.url())
except error.BundleValueError as exc: except error.BundleValueError as exc:
raise error.Abort(_('missing support for %s') % exc) raise error.Abort(_('missing support for %s') % exc)
def findcommonoutgoing(repo, other, heads): def findcommonoutgoing(repo, ui, other, heads):
if heads: if heads:
# Avoid using remotenames fastheaddiscovery heuristic. It uses
# remotenames file to quickly find commonoutgoing set, but it can
# result in sending public commits to infinitepush servers.
# For example:
#
# o draft
# /
# o C1
# |
# ...
# |
# o remote/master
#
# pushbackup in that case results in sending to the infinitepush server
# all public commits from 'remote/master' to C1. It increases size of
# the bundle + it may result in storing data about public commits
# in infinitepush table.
with ui.configoverride({("remotenames", "fastheaddiscovery"): False}):
nodes = map(repo.changelog.node, heads) nodes = map(repo.changelog.node, heads)
return discovery.findcommonoutgoing(repo, other, onlyheads=nodes) return discovery.findcommonoutgoing(repo, other, onlyheads=nodes)
else: else:
return None return None
def _getrevstobackup(repo, ui, other, headstobackup): def _getrevstobackup(repo, ui, other, headstobackup):
# In rare cases it's possible to have a local node without filelogs. # In rare cases it's possible to have a local node without filelogs.
# This is possible if remotefilelog is enabled and if the node was # This is possible if remotefilelog is enabled and if the node was
# stripped server-side. We want to filter out these bad nodes and all # stripped server-side. We want to filter out these bad nodes and all
# of their descendants. # of their descendants.
badnodes = ui.configlist('infinitepushbackup', 'dontbackupnodes', []) badnodes = ui.configlist('infinitepushbackup', 'dontbackupnodes', [])
badnodes = [node for node in badnodes if node in repo] badnodes = [node for node in badnodes if node in repo]
badrevs = [repo[node].rev() for node in badnodes] badrevs = [repo[node].rev() for node in badnodes]
badnodesdescendants = repo.set('%ld::', badrevs) if badrevs else set() badnodesdescendants = repo.set('%ld::', badrevs) if badrevs else set()
badnodesdescendants = set(ctx.hex() for ctx in badnodesdescendants) badnodesdescendants = set(ctx.hex() for ctx in badnodesdescendants)
filteredheads = filter(lambda head: head in badnodesdescendants, filteredheads = filter(lambda head: head in badnodesdescendants,
headstobackup) headstobackup)
if filteredheads: if filteredheads:
ui.warn(_('filtering nodes: %s\n') % filteredheads) ui.warn(_('filtering nodes: %s\n') % filteredheads)
ui.log('infinitepushbackup', 'corrupted nodes found', ui.log('infinitepushbackup', 'corrupted nodes found',
infinitepushbackupcorruptednodes='failure') infinitepushbackupcorruptednodes='failure')
headstobackup = filter(lambda head: head not in badnodesdescendants, headstobackup = filter(lambda head: head not in badnodesdescendants,
headstobackup) headstobackup)
revs = list(repo[hexnode].rev() for hexnode in headstobackup) revs = list(repo[hexnode].rev() for hexnode in headstobackup)
outgoing = findcommonoutgoing(repo, other, revs) outgoing = findcommonoutgoing(repo, ui, other, revs)
nodeslimit = 1000 nodeslimit = 1000
if outgoing and len(outgoing.missing) > nodeslimit: if outgoing and len(outgoing.missing) > nodeslimit:
# trying to push too many nodes usually means that there is a bug # trying to push too many nodes usually means that there is a bug
# somewhere. Let's be safe and avoid pushing too many nodes at once # somewhere. Let's be safe and avoid pushing too many nodes at once
raise error.Abort('trying to back up too many nodes: %d' % raise error.Abort('trying to back up too many nodes: %d' %
(len(outgoing.missing),)) (len(outgoing.missing),))
return outgoing, set(filteredheads) return outgoing, set(filteredheads)

tests/test-infinitepush-remotenames-public.t

  • This file was added.
Remotenames extension has a shortcut that makes heads discovery work faster.
Unfortunately that may result in sending public commits to the server. This
test covers the issue.
$ . $TESTDIR/require-ext.sh remotenames
$ . $TESTDIR/library.sh
$ . $TESTDIR/library-infinitepush.sh
$ cat >> $HGRCPATH << EOF
> [extensions]
> infinitepush=$TESTDIR/../infinitepush
> [infinitepush]
> branchpattern=re:scratch/.+
> [ui]
> ssh = python "$TESTDIR/dummyssh"
> EOF
$ mkcommit() {
> echo "$1" > "$1"
> hg add "$1"
> hg ci -m "$1"
> }
$ scratchnodes() {
> for node in `find ../repo/.hg/scratchbranches/index/nodemap/* | sort`; do
> echo ${node##*/}
> done
> }
$ scratchbookmarks() {
> for bookmark in `find ../repo/.hg/scratchbranches/index/bookmarkmap/* -type f | sort`; do
> echo "${bookmark##*/bookmarkmap/} `cat $bookmark`"
> done
> }
$ enableremotenames() {
> printf '[extensions]\nremotenames=\n' >> .hg/hgrc
> }
Setup server with a few commits and one remote bookmark. This remotebookmark
may be used by remotenames extension in fastheaddiscovery heuristic
$ hg init repo
$ cd repo
$ setupserver
$ mkcommit first
$ hg book remotebook
$ hg up -q .
$ mkcommit second
$ mkcommit third
$ mkcommit fourth
$ cd ..
Create new client
$ hg clone ssh://user@dummy/repo --config extensions.remotenames= client -q
$ cd client
$ enableremotenames
Create scratch commit and back it up.
$ hg up -q -r 'desc(third)'
$ mkcommit scratch
created new head
$ hg log -r . -T '{node}\n'
ce87a066ebc28045311cd1272f5edc0ed80d5b1c
$ hg log --graph -T '{desc}'
@ scratch
|
| o fourth
|/
o third
|
o second
|
o first
$ hg pushbackup
starting backup * (glob)
searching for changes
remote: pushing 1 commit:
remote: ce87a066ebc2 scratch
finished in * (glob)
$ cd ..
Create second client
$ hg clone ssh://user@dummy/repo --config extensions.remotenames= client2 -q
$ cd client2
$ enableremotenames
Pull to get remote names
$ hg pull
pulling from ssh://user@dummy/repo
searching for changes
no changes found
$ hg book --remote
default/remotebook 0:b75a450e74d5
Strip public commits from the repo, otherwise fastheaddiscovery heuristic will
be skipped
$ hg strip -q -r '1:'
$ hg log --graph -T '{desc}'
@ first
Download scratch commit. It also downloads a few public commits
$ hg up -q ce87a066ebc28045311cd1272f5edc0ed80d5b1c
'ce87a066ebc28045311cd1272f5edc0ed80d5b1c' does not exist locally - looking for it remotely...
'ce87a066ebc28045311cd1272f5edc0ed80d5b1c' found remotely
$ hg log --graph -T '{desc}'
@ scratch
|
o third
|
o second
|
o first
$ hg book --remote
default/remotebook 0:b75a450e74d5
Run pushbackup and make sure only scratch commit is backed up.
$ hg pushbackup
starting backup * (glob)
searching for changes
remote: pushing 1 commit:
remote: ce87a066ebc2 scratch
finished in * (glob)