[PATCH 09 of 11] wireproto: advertise supported compression formats in capabilities

Kyle Lippincott spectral at pewpew.net
Mon Nov 21 19:24:01 EST 2016


On Sun, Nov 20, 2016 at 2:23 PM, Gregory Szorc <gregory.szorc at gmail.com>
wrote:

> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1479667112 28800
> #      Sun Nov 20 10:38:32 2016 -0800
> # Node ID f3f2bb7d66a45f16856ad890a8892b3dbafa480e
> # Parent  9e0c42d347fd8bcba87561c92fc93b3ba597ec6f
> wireproto: advertise supported compression formats in capabilities
>
> This commit introduces support for advertising a server capability
> listing available compression formats.
>
> The bulk of the new code is a helper function in wireproto.py to
> obtain a prioritized list of compression engines available to the
> wire protocol. While not utilized yet, we implement support
> for obtaining the list of compression engines advertised by the
> client.
>
> The upcoming HTTP protocol enhancements are a bit lower-level than
> existing tests (most existing tests are command centric). So,
> this commit establishes a new test file that will be appropriate
> for holding tests around the functionality of the HTTP protocol
> itself.
>
> Rounding out this change, `hg debuginstall` now prints compression
> engines available to the server.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -2588,6 +2588,13 @@ def debuginstall(ui, **opts):
>               fm.formatlist(sorted(e.name() for e in compengines
>                                    if e.available()),
>                             name='compengine', fmt='%s', sep=', '))
> +    wirecompengines = util.compengines.supportedwireengines('server',
> +
> onlyavailable=True)
> +    fm.write('compenginesserver', _('checking available compression
> engines '
> +                                    'for wire protocol (%s)\n'),
> +             fm.formatlist([e.name() for e in wirecompengines
> +                            if e.wireprotosupport()],
> +                           name='compengine', fmt='%s', sep=', '))
>
>      # templates
>      p = templater.templatepaths()
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1523,6 +1523,21 @@ Alias definitions for revsets. See :hg:`
>
>  Controls generic server settings.
>
> +``compressionengines``
> +    List of compression engines and their relative priority to advertise
> +    to clients.
> +
> +    The order of compression engines determines their priority, the first
> +    having the highest priority. If a compression engine is not listed
> +    here, it won't be advertised to clients.
> +
> +    If not set (the default), built-in defaults are used. Run
> +    :hg:`debuginstall` to list available compression engines and their
> +    default wire protocol priority.
> +
> +    Older Mercurial clients only support zlib compression and this setting
> +    has no effect for legacy clients.
> +
>  ``uncompressed``
>      Whether to allow clients to clone a repository using the
>      uncompressed streaming protocol. This transfers about 40% more
> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
> --- a/mercurial/wireproto.py
> +++ b/mercurial/wireproto.py
> @@ -608,6 +608,59 @@ def bundle1allowed(repo, action):
>
>      return ui.configbool('server', 'bundle1', True)
>
> +def supportedcompengines(ui, proto, role):
> +    """Obtain the list of supported compression engines for a request."""
> +    assert role in ('client', 'server')
> +
> +    if not proto.supportscompression:
> +        return []
> +
> +    compengines = util.compengines.supportedwireengines(role,
> +
> onlyavailable=True)
> +
> +    # Allow config to override default list and ordering.
> +    if role == 'server':
> +        configengines = ui.configlist('server', 'compressionengines')
> +        config = 'server.compressionengines'
> +    else:
> +        # This is currently implemented mainly to facilitate testing. In
> most
> +        # cases, the server should be in charge of choosing a compression
> engine
> +        # because a server has the most to lose from a sub-optimal
> choice. (e.g.
> +        # CPU DoS due to an expensive engine or a network DoS due to poor
> +        # compression ratio).
> +        configengines = ui.configlist('experimental',
> +                                           'clientcompressionengines')
> +        config = 'experimental.clientcompressionengines'
> +
> +    # No explicit config. Filter out the ones that aren't supposed to be
> +    # advertised and return default ordering.
> +    if not configengines:
> +        idx = 1 if role == 'server' else 2
>

This is hard to read, it would be nice if wireprotosupport() returned a
dict or something, so one could do something like:
wireprotosupport()['roleweight:'+role], instead of just knowing that it's
index 1 or 2.  If this is idiomatic and I just haven't interacted with the
code enough to realize it, feel free to ignore.

+        return [e for e in compengines if e.wireprotosupport()[idx] > 0]
> +
> +    # If compression engines are listed in the config, assume there is a
> good
> +    # reason for it (like server operators wanting to achieve specific
> +    # performance characteristics). So fail fast if the config references
> +    # unusable compression engines.
> +    validnames = set(e.name() for e in compengines)
> +    invalidnames = set(e for e in configengines if e not in validnames)
> +
> +    if invalidnames:
> +        raise error.Abort(_('invalid compression engine defined in %s:
> %s') %
> +                          (config, ', '.join(sorted(invalidnames))))
> +
> +    compengines = [e for e in compengines if e.name() in configengines]
> +    compengines = sorted(compengines,
> +                         key=lambda e: configengines.index(e.name()))
> +
> +    if not compengines:
> +        raise error.Abort(_('%s config option does not specify any known '
> +                            'compression engines') % config,
> +                          hint=_('usable compression engines: %s') %
> +                          ', '.sorted(validnames))
> +
> +    return compengines
> +
>  # list of commands
>  commands = {}
>
> @@ -721,6 +774,12 @@ def _capabilities(repo, proto):
>          'httpheader=%d' % repo.ui.configint('server', 'maxhttpheaderlen',
> 1024))
>      if repo.ui.configbool('experimental', 'httppostargs', False):
>          caps.append('httppostargs')
> +
> +    compengines = supportedcompengines(repo.ui, proto, 'server')
> +    if compengines:
> +        comptypes = ','.join(e.wireprotosupport()[0] for e in
> compengines)
>

Currently, these engine codes are limited to two bytes (not characters, I
think); are there any bytes, such as comma, that are disallowed?  (I don't
know how caps are escaped).


> +        caps.append('compression=%s' % comptypes)
> +
>      return caps
>
>  # If you are writing an extension and consider wrapping this function.
> Wrap
> diff --git a/tests/test-hgweb-commands.t b/tests/test-hgweb-commands.t
> --- a/tests/test-hgweb-commands.t
> +++ b/tests/test-hgweb-commands.t
> @@ -1903,7 +1903,7 @@ capabilities
>    $ get-with-headers.py 127.0.0.1:$HGPORT '?cmd=capabilities'; echo
>    200 Script output follows
>
> -  lookup changegroupsubset branchmap pushkey known getbundle unbundlehash
> batch bundle2=HG20%0Achangegroup%3D01%2C02%0Adigests%3Dmd5%
> 2Csha1%2Csha512%0Aerror%3Dabort%2Cunsupportedcontent%
> 2Cpushraced%2Cpushkey%0Ahgtagsfnodes%0Alistkeys%0Apushkey%0Aremote-changegroup%3Dhttp%2Chttps
> unbundle=HG10GZ,HG10BZ,HG10UN httpheader=1024
> +  lookup changegroupsubset branchmap pushkey known getbundle unbundlehash
> batch bundle2=HG20%0Achangegroup%3D01%2C02%0Adigests%3Dmd5%
> 2Csha1%2Csha512%0Aerror%3Dabort%2Cunsupportedcontent%
> 2Cpushraced%2Cpushkey%0Ahgtagsfnodes%0Alistkeys%0Apushkey%0Aremote-changegroup%3Dhttp%2Chttps
> unbundle=HG10GZ,HG10BZ,HG10UN httpheader=1024 compression=*ZL* (glob)
>
>  heads
>
> @@ -2154,6 +2154,7 @@ capabilities
>    bundle2=HG20%0Achangegroup%3D01%2C02%0Adigests%3Dmd5%
> 2Csha1%2Csha512%0Aerror%3Dabort%2Cunsupportedcontent%
> 2Cpushraced%2Cpushkey%0Ahgtagsfnodes%0Alistkeys%0Apushkey%0Aremote-
> changegroup%3Dhttp%2Chttps
>    unbundle=HG10GZ,HG10BZ,HG10UN
>    httpheader=1024
> +  compression=*ZL* (glob)
>
>  heads
>
> diff --git a/tests/test-http-protocol.t b/tests/test-http-protocol.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-http-protocol.t
> @@ -0,0 +1,45 @@
> +  $ cat >> $HGRCPATH << EOF
> +  > [web]
> +  > push_ssl = false
> +  > allow_push = *
> +  > EOF
> +
> +  $ hg init server
> +  $ cd server
> +  $ touch a
> +  $ hg -q commit -A -m initial
> +  $ cd ..
> +
> +  $ hg -R server serve -p $HGPORT -d --pid-file hg.pid
> +  $ cat hg.pid >> $DAEMON_PIDS
> +
> +compression formats are advertised in compression capability
> +
> +#if zstd
> +  $ get-with-headers.py 127.0.0.1:$HGPORT '?cmd=capabilities' | tr ' '
> '\n' | grep compression
> +  compression=ZS,ZL
> +#else
> +  $ get-with-headers.py 127.0.0.1:$HGPORT '?cmd=capabilities' | tr ' '
> '\n' | grep compression
> +  compression=ZL
> +#endif
> +
> +  $ killdaemons.py
> +
> +
> +server.compressionengines can replace engines list wholesale
> +
> +  $ hg --config server.compressionengines=none -R server serve -p $HGPORT
> -d --pid-file hg.pid
> +  $ cat hg.pid > $DAEMON_PIDS
> +  $ get-with-headers.py 127.0.0.1:$HGPORT '?cmd=capabilities' | tr ' '
> '\n' | grep compression
> +  compression=UN
> +
> +  $ killdaemons.py
> +
> +Order of engines can also change
> +
> +  $ hg --config server.compressionengines=none,zlib -R server serve -p
> $HGPORT -d --pid-file hg.pid
> +  $ cat hg.pid > $DAEMON_PIDS
> +  $ get-with-headers.py 127.0.0.1:$HGPORT '?cmd=capabilities' | tr ' '
> '\n' | grep compression
> +  compression=UN,ZL
> +
> +  $ killdaemons.py
> diff --git a/tests/test-install.t b/tests/test-install.t
> --- a/tests/test-install.t
> +++ b/tests/test-install.t
> @@ -13,6 +13,7 @@ hg debuginstall
>    checking installed modules (*mercurial)... (glob)
>    checking registered compression engines (*zlib*) (glob)
>    checking available compression engines (*zlib*) (glob)
> +  checking available compression engines for wire protocol (*zlib*) (glob)
>    checking templates (*mercurial?templates)... (glob)
>    checking default template (*mercurial?templates?map-cmdline.default)
> (glob)
>    checking commit editor... (* -c "import sys; sys.exit(0)") (glob)
> @@ -25,6 +26,7 @@ hg debuginstall JSON
>     {
>      "compengines": ["bz2", "bz2truncated", "none", "zlib"*], (glob)
>      "compenginesavail": ["bz2", "bz2truncated", "none", "zlib"*], (glob)
> +    "compenginesserver": [*"zlib"*], (glob)
>      "defaulttemplate": "*mercurial?templates?map-cmdline.default", (glob)
>      "defaulttemplateerror": null,
>      "defaulttemplatenotfound": "default",
> @@ -64,6 +66,7 @@ hg debuginstall with no username
>    checking installed modules (*mercurial)... (glob)
>    checking registered compression engines (*zlib*) (glob)
>    checking available compression engines (*zlib*) (glob)
> +  checking available compression engines for wire protocol (*zlib*) (glob)
>    checking templates (*mercurial?templates)... (glob)
>    checking default template (*mercurial?templates?map-cmdline.default)
> (glob)
>    checking commit editor... (* -c "import sys; sys.exit(0)") (glob)
> @@ -93,6 +96,7 @@ path variables are expanded (~ is the sa
>    checking installed modules (*mercurial)... (glob)
>    checking registered compression engines (*zlib*) (glob)
>    checking available compression engines (*zlib*) (glob)
> +  checking available compression engines for wire protocol (*zlib*) (glob)
>    checking templates (*mercurial?templates)... (glob)
>    checking default template (*mercurial?templates?map-cmdline.default)
> (glob)
>    checking commit editor... (* -c "import sys; sys.exit(0)") (glob)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20161121/e433b75e/attachment.html>


More information about the Mercurial-devel mailing list