[PATCH V2] wireproto: support disabling bundle1 only if repo is generaldelta

Augie Fackler raf at durin42.com
Tue Jan 5 11:14:04 CST 2016


On Sat, Jan 02, 2016 at 12:28:57PM -0800, Gregory Szorc 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 ab4f909ad85632312e1af5493cdf4915d7dc586d
> # Parent  b8405d739149cdd6d8d9bd5e3dd2ad8487b1f09a
> wireproto: support disabling bundle1 only if repo is generaldelta


Queued this, thanks.

>
> I recently implemented the server.bundle1* options to control whether
> bundle1 exchange is allowed.
>
> 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 set of options to control bundle1 behavior
> for generaldelta repos. These options enable server operators to limit
> bundle1 restrictions to the class of repos that can be 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).
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1340,24 +1340,36 @@ Controls generic server settings.
>  ``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)
>
> +``bundle1gd``
> +    Like ``bundle1` but only used if the repository is using the
> +    *generaldelta* storage format. (default: True)
> +
>  ``bundle1.push``
>      Whether to allow clients to push using the legacy bundle1 exchange
>      format. (default: True)
>
> +``bundle1gd.push``
> +    Like ``bundle1.push` but only used if the repository is using the
> +    *generaldelta* storage format. (default: True)
> +
>  ``bundle1.pull``
>      Whether to allow clients to pull using the legacy bundle1 exchange
>      format. (default: True)
>
> +``bundle1gd.pull``
> +    Like ``bundle1.pull` but only used if the repository is using the
> +    *generaldelta* storage format. (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,22 +486,43 @@ 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):
> -    """Whether a bundle1 operation is allowed from the server."""
> +def bundle1allowed(repo, action):
> +    """Whether a bundle1 operation is allowed from the server.
> +
> +    Priority is:
> +
> +    1. server.bundle1gd.<action> (if generaldelta active)
> +    2. server.bundle1.<action>
> +    3. server.bundle1gd (if generaldelta active)
> +    4. server.bundle1
> +    """
> +    ui = repo.ui
> +    gd = 'generaldelta' in repo.requirements
> +
> +    if gd:
> +        v = ui.configbool('server', 'bundle1gd.%s' % action, None)
> +        if v is not None:
> +            return v
> +
>      v = ui.configbool('server', 'bundle1.%s' % action, None)
>      if v is not None:
>          return v
>
> +    if gd:
> +        v = ui.configbool('server', 'bundle1gd', None)
> +        if v is not None:
> +            return v
> +
>      return ui.configbool('server', 'bundle1', True)
>
>  # list of commands
>  commands = {}
>
>  def wireprotocommand(name, args=''):
>      """decorator for wire protocol command"""
>      def register(func):
> @@ -660,17 +681,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 +797,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 ..
> +
> +bundle1 can still pull non-generaldelta repos when generaldelta bundle1 disabled
> +
> +  $ hg --config format.usegeneraldelta=false init notgdserver
> +  $ cd notgdserver
> +  $ cat > .hg/hgrc << EOF
> +  > [server]
> +  > bundle1gd.pull = false
> +  > 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
> +
> +bundle1 pull can be disabled for generaldelta repos only
> +
> +  $ cat > .hg/hgrc << EOF
> +  > [server]
> +  > bundle1gd.pull = false
> +  > 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]
> +  > bundle1gd = false
> +  > 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]
> +  > bundle1gd = false
> +  > 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
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list