[PATCH] wireproto: support disabling bundle1 only if repo is generaldelta
Gregory Szorc
gregory.szorc at gmail.com
Sun Dec 20 14:37:34 CST 2015
On Sun, Dec 20, 2015 at 12:28 PM, Gregory Szorc <gregory.szorc at gmail.com>
wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1450641384 28800
> # Sun Dec 20 11:56:24 2015 -0800
> # Node ID a57fec21e479844c0ee493b727d9659698fc8352
> # Parent ff305ab2e0d7291da12a8b640ce8c9bb28e0cb03
> wireproto: support disabling bundle1 only if repo is generaldelta
>
Food for thought: we /could/ use sub-options here. e.g.
[server]
bundle1.pull:generaldelta = False
This would likely make it easier to expand the set of checks/values in the
future. I think it's also easier to document since options have consistent
types and meaning.
The reason I didn't do this is because we don't really use sub-options
outside of [paths]. But that's only because the concept is new.
How do people feel about using sub-options in more places?
>
> I recently implemented the server.bundle1* options to control whether
> bundle1 exchange is allowed. Initially, I implemented them as simple
> boolean options.
>
> After thinking about Mozilla's strategy for handling generaldelta
> rollout a bit more, I think server operators need an additional
> lever: disable bundle1 if and only if the repo is generaldelta.
>
> bundle1 exchange for non-generaldelta repos will not have the potential
> for CPU explosion that generaldelta repos do. Therefore, it makes sense
> for server operators to continue to allow bundle1 exchange for
> non-generaldelta repos without having to set a per-repo hgrc option
> to change the policy depending on whether the repo is generaldelta.
>
> This patch introduces a new string value for the server.bundle1*
> options that disallows bundle1 for generaldelta repos.
>
> This new value enables server operators to limit the bundle1 restriction
> to the class of repos that can cause performance issues. It also allows
> server operators to tie bundle1 access to store format. In many server
> environments (including Mozilla's), legacy repos will not be
> generaldelta and new repos will or might be. New repos often aren't
> bound by legacy access requirements, so setting a global policy that
> disallows access to new/generaldelta repos via bundle1 could be a
> reasonable policy in many server environments. This patch makes this
> policy very easy to implement (modify global hgrc, add options to
> existing generaldelta repos to grandfather them in).
>
> This patch shines light on a possibly missing ui.config* API to
> retrieve a value as a boolean or as a string. check-config doesn't
> allow treating values as multiple types, so I had to reimplement part
> of configbool() inline in this patch. I'm not sure if this pattern is
> common enough to warrant a new ui.config* method.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1330,25 +1330,39 @@ Controls generic server settings.
> present. (default: False)
>
> ``maxhttpheaderlen``
> Instruct HTTP clients not to send request headers longer than this
> many bytes. (default: 1024)
>
> ``bundle1``
> Whether to allow clients to push and pull using the legacy bundle1
> - exchange format. (default: True)
> + exchange format.
> +
> + Value is a boolean or the special string value ``nogeneraldelta``,
> + which indicates bundle1 is allowed unless the repo is using the
> + *generaldelta* storage format.
> +
> + (default: True)
>
> ``bundle1.push``
> Whether to allow clients to push using the legacy bundle1 exchange
> - format. (default: True)
> + format.
> +
> + Accepts the same values as ``bundle1``.
> +
> + (default: True)
>
> ``bundle1.pull``
> Whether to allow clients to pull using the legacy bundle1 exchange
> - format. (default: True)
> + format.
> +
> + Accepts the same values as ``bundle1``.
> +
> + (default: True)
>
> Large repositories using the *generaldelta* storage format should
> consider setting this option because converting *generaldelta*
> repositories to the exchange format required by the bundle1 data
> format can consume a lot of CPU.
>
> ``smtp``
> --------
> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
> --- a/mercurial/wireproto.py
> +++ b/mercurial/wireproto.py
> @@ -486,23 +486,49 @@ def options(cmd, keys, others):
> if k in others:
> opts[k] = others[k]
> del others[k]
> if others:
> sys.stderr.write("warning: %s ignored unexpected arguments %s\n"
> % (cmd, ",".join(others)))
> return opts
>
> -def bundle1allowed(ui, action):
> +def bundle1allowed(repo, action):
> """Whether a bundle1 operation is allowed from the server."""
> - v = ui.configbool('server', 'bundle1.%s' % action, None)
> + # Value can be bool or a special string "nogeneraldelta" to indicate
> + # not allowed on generaldelta repos. check-config doesn't allow us to
> + # treat a single option as both a str and a bool, so we do the boolean
> + # parsing manually.
> + ui = repo.ui
> + gd = 'generaldelta' in repo.requirements
> + v = ui.config('server', 'bundle1.%s' % action, None)
> + if v == 'nogeneraldelta':
> + return not gd
> +
> + # Other values must parse as bool.
> if v is not None:
> + v = util.parsebool(v)
> + if v is None:
> + raise error.ConfigError(_("%s.%s is not a boolean ('%s')") %
> + ('server', 'bundle1.%s' % action))
> return v
>
> - return ui.configbool('server', 'bundle1', True)
> + v = ui.config('server', 'bundle1', None)
> + if v == 'nogeneraldelta':
> + return not gd
> +
> + # Default to True.
> + if v is None:
> + return True
> +
> + v = util.parsebool(v)
> + if v is None:
> + raise error.ConfigError(_("%s.%s is not a boolean ('%s')") %
> + ('server', 'bundle1.%s' % action))
> + return v
>
> # list of commands
> commands = {}
>
> def wireprotocommand(name, args=''):
> """decorator for wire protocol command"""
> def register(func):
> commands[name] = (func, args)
> @@ -660,17 +686,17 @@ def getbundle(repo, proto, others):
> if v == '0':
> opts[k] = False
> else:
> opts[k] = bool(v)
> elif keytype != 'plain':
> raise KeyError('unknown getbundle option type %s'
> % keytype)
>
> - if not bundle1allowed(repo.ui, 'pull'):
> + if not bundle1allowed(repo, 'pull'):
> if not exchange.bundle2requested(opts.get('bundlecaps')):
> return ooberror(bundle2required)
>
> cg = exchange.getbundle(repo, 'serve', **opts)
> return streamres(proto.groupchunks(cg))
>
> @wireprotocommand('heads')
> def heads(repo, proto):
> @@ -776,17 +802,17 @@ def unbundle(repo, proto, heads):
> fd, tempname = tempfile.mkstemp(prefix='hg-unbundle-')
> fp = os.fdopen(fd, 'wb+')
> r = 0
> try:
> proto.getfile(fp)
> fp.seek(0)
> gen = exchange.readbundle(repo.ui, fp, None)
> if (isinstance(gen, changegroupmod.cg1unpacker)
> - and not bundle1allowed(repo.ui, 'push')):
> + and not bundle1allowed(repo, 'push')):
> return ooberror(bundle2required)
>
> r = exchange.unbundle(repo, gen, their_heads, 'serve',
> proto._client())
> if util.safehasattr(r, 'addpart'):
> # The return looks streamable, we are in the bundle2 case
> and
> # should return a stream.
> return streamres(r.getchunks())
> diff --git a/tests/test-bundle2-exchange.t b/tests/test-bundle2-exchange.t
> --- a/tests/test-bundle2-exchange.t
> +++ b/tests/test-bundle2-exchange.t
> @@ -972,16 +972,61 @@ Servers can disable bundle1 for clone/pu
>
> $ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT/
> not-bundle2
> requesting all changes
> abort: remote error:
> incompatible Mercurial client; bundle2 required
> (see https://www.mercurial-scm.org/wiki/IncompatibleClient)
> [255]
> $ killdaemons.py
> + $ cd ..
> +
> +"nogeneraldelta" value allows bundle1 on non-generaldelta repos
> +
> + $ hg --config format.usegeneraldelta=false init notgdserver
> + $ cd notgdserver
> + $ cat > .hg/hgrc << EOF
> + > [server]
> + > bundle1.pull = nogeneraldelta
> + > EOF
> +
> + $ touch foo
> + $ hg -q commit -A -m initial
> + $ hg serve -p $HGPORT -d --pid-file=hg.pid
> + $ cat hg.pid >> $DAEMON_PIDS
> +
> + $ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT/
> not-bundle2-1
> + requesting all changes
> + adding changesets
> + adding manifests
> + adding file changes
> + added 1 changesets with 1 changes to 1 files
> + updating to branch default
> + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +
> + $ killdaemons.py
> + $ cd ../bundle2onlyserver
> +
> +nogeneraldelta prevents serving of generaldelta repos
> +
> + $ cat > .hg/hgrc << EOF
> + > [server]
> + > bundle1.pull = nogeneraldelta
> + > EOF
> +
> + $ hg serve -p $HGPORT -d --pid-file=hg.pid
> + $ cat hg.pid >> $DAEMON_PIDS
> + $ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT/
> not-bundle2
> + requesting all changes
> + abort: remote error:
> + incompatible Mercurial client; bundle2 required
> + (see https://www.mercurial-scm.org/wiki/IncompatibleClient)
> + [255]
> +
> + $ killdaemons.py
>
> Verify the global server.bundle1 option works
>
> $ cat > .hg/hgrc << EOF
> > [server]
> > bundle1 = false
> > EOF
> $ hg serve -p $HGPORT -d --pid-file=hg.pid
> @@ -989,16 +1034,52 @@ Verify the global server.bundle1 option
> $ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT
> not-bundle2
> requesting all changes
> abort: remote error:
> incompatible Mercurial client; bundle2 required
> (see https://www.mercurial-scm.org/wiki/IncompatibleClient)
> [255]
> $ killdaemons.py
>
> + $ cat > .hg/hgrc << EOF
> + > [server]
> + > bundle1 = nogeneraldelta
> + > EOF
> + $ hg serve -p $HGPORT -d --pid-file=hg.pid
> + $ cat hg.pid >> $DAEMON_PIDS
> +
> + $ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT/
> not-bundle2
> + requesting all changes
> + abort: remote error:
> + incompatible Mercurial client; bundle2 required
> + (see https://www.mercurial-scm.org/wiki/IncompatibleClient)
> + [255]
> +
> + $ killdaemons.py
> +
> + $ cd ../notgdserver
> + $ cat > .hg/hgrc << EOF
> + > [server]
> + > bundle1 = nogeneraldelta
> + > EOF
> + $ hg serve -p $HGPORT -d --pid-file=hg.pid
> + $ cat hg.pid >> $DAEMON_PIDS
> +
> + $ hg --config experimental.bundle2-exp=false clone http://localhost:$HGPORT/
> not-bundle2-2
> + requesting all changes
> + adding changesets
> + adding manifests
> + adding file changes
> + added 1 changesets with 1 changes to 1 files
> + updating to branch default
> + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +
> + $ killdaemons.py
> + $ cd ../bundle2onlyserver
> +
> Verify bundle1 pushes can be disabled
>
> $ cat > .hg/hgrc << EOF
> > [server]
> > bundle1.push = false
> > [web]
> > allow_push = *
> > push_ssl = false
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20151220/c75f0523/attachment.html>
More information about the Mercurial-devel
mailing list