Patchwork [5,of,9] util: validate and extract compression-related bundlespec parameters

login
register
mail settings
Submitter Gregory Szorc
Date April 1, 2017, 10:31 p.m.
Message ID <62e377f673f3d9e10701.1491085916@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/19897/
State Changes Requested
Headers show

Comments

Gregory Szorc - April 1, 2017, 10:31 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1491079470 25200
#      Sat Apr 01 13:44:30 2017 -0700
# Node ID 62e377f673f3d9e10701d373b82f995085b54363
# Parent  5cc2f25b803a0184fdc4e67142b65df752e40284
util: validate and extract compression-related bundlespec parameters

There is currently an experimental config option used by
`hg bundle` to declare the compression level to be used by
the compression engine. I implemented this just before the 4.1
freeze so there would be a way to adjust the zstd compression
level. (Because zstd is highly flexible and one may want to optimize
for extreme speed or extreme compression.) My intent was always
to formalize compression engine settings later.

Now that we have formalized the *bundlespec* string and exposed
it to users via documentation, we can start leaning on it - and
its extensibility via parameters - to declare compression options.

This patch introduces a compression engine method for validating
and extracting relevant bundlespec parameters used by the
compression engine. This allows each compression engine to define
its own set of tunable settings.

For the gzip and zstd engines, we have defined a method that
turns the "compressionlevel" parameter into a "level" option
for compression. I have plans to add more parameters to the zstd
engine. But I'll add those in a follow-up.

To prove this works, we call the new API from bundlespec parsing
and add tests demonstrating failures.

The added API returns a set of parameters relevant to the engine.
While unused, my eventual intent is for bundlespec parsing to know
when there are "unknown" parameters so it can warn about their
presence. Since a compression engine could consume *any* parameter,
it needs to tell the bundlespec parser which parameters are relevant
to it. While this is a YAGNI violation, it will prevent future BC
in the compression engine API.
Augie Fackler - April 3, 2017, 7:58 p.m.
On Sat, Apr 01, 2017 at 03:31:56PM -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1491079470 25200
> #      Sat Apr 01 13:44:30 2017 -0700
> # Node ID 62e377f673f3d9e10701d373b82f995085b54363
> # Parent  5cc2f25b803a0184fdc4e67142b65df752e40284
> util: validate and extract compression-related bundlespec parameters
>
> There is currently an experimental config option used by
> `hg bundle` to declare the compression level to be used by
> the compression engine. I implemented this just before the 4.1
> freeze so there would be a way to adjust the zstd compression
> level. (Because zstd is highly flexible and one may want to optimize
> for extreme speed or extreme compression.) My intent was always
> to formalize compression engine settings later.
>
> Now that we have formalized the *bundlespec* string and exposed
> it to users via documentation, we can start leaning on it - and
> its extensibility via parameters - to declare compression options.
>
> This patch introduces a compression engine method for validating
> and extracting relevant bundlespec parameters used by the
> compression engine. This allows each compression engine to define
> its own set of tunable settings.
>
> For the gzip and zstd engines, we have defined a method that
> turns the "compressionlevel" parameter into a "level" option
> for compression. I have plans to add more parameters to the zstd
> engine. But I'll add those in a follow-up.
>
> To prove this works, we call the new API from bundlespec parsing
> and add tests demonstrating failures.
>
> The added API returns a set of parameters relevant to the engine.
> While unused, my eventual intent is for bundlespec parsing to know
> when there are "unknown" parameters so it can warn about their
> presence. Since a compression engine could consume *any* parameter,
> it needs to tell the bundlespec parser which parameters are relevant
> to it. While this is a YAGNI violation, it will prevent future BC
> in the compression engine API.
>
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -163,8 +163,15 @@ def parsebundlespec(repo, spec, strict=T
>                      _('missing support for repository features: %s') %
>                        ', '.join(sorted(missingreqs)))
>
> +    engine = util.compengines.forbundlename(compression)
> +
> +    # Verify compression-related parameters with compression engine.
> +    try:
> +        engine.parsebundlespecparams(params)
> +    except Exception as e:

Yikes. Can we make the API contract more narrow here and not do a
catch-all except?

> +        raise error.UnsupportedBundleSpecification(str(e))
> +
>      if not externalnames:
> -        engine = util.compengines.forbundlename(compression)
>          compression = engine.bundletype()[1]
>          version = _bundlespeccgversions[version]
>      return compression, version, params
> diff --git a/mercurial/help/bundlespec.txt b/mercurial/help/bundlespec.txt
> --- a/mercurial/help/bundlespec.txt
> +++ b/mercurial/help/bundlespec.txt
> @@ -82,3 +82,7 @@ Examples
>
>  ``zstd-v1``
>      This errors because ``zstd`` is not supported for ``v1`` types.
> +
> +``zstd-v2;compressionlevel=11``
> +   Produce a ``v2`` bundle with zstandard compression using compression level
> +   ``11`` (which will take longer to produce a smaller bundle).
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -3276,6 +3276,20 @@ class compressionengine(object):
>          """
>          return None
>
> +    def parsebundlespecparams(self, params):
> +        """Parse *bundlespec* parameters to a dict of options.
> +
> +        The *bundlespec* string may define key-value parameters. This method
> +        is used to validate and extract parameters relevant to compression.
> +
> +        Returns a 2-tuple of a set of parameter names that are relevant to this
> +        engine and a dict of options that will possibly be fed as the ``opts``
> +        argument into a subsequent compression operation.
> +
> +        Exceptions can be raised to indicate an invalid parameter value.
> +        """
> +        return set(), {}
> +
>      def wireprotosupport(self):
>          """Declare support for this compression format on the wire protocol.
>
> @@ -3362,9 +3376,33 @@ class _zlibengine(compressionengine):
>          All Mercurial clients should support this format. The compression
>          algorithm strikes a reasonable balance between compression ratio
>          and size.
> +
> +        The ``compressionlevel`` parameter defines the integer compression
> +        level to use. The default (``-1``) is likely equivalent to ``6``.
> +        ``0`` means no compression. ``9`` means maximum compression.
>          """
>          return 'gzip', 'GZ'
>
> +    def parsebundlespecparams(self, params):
> +        relevant = set()
> +        opts = {}
> +
> +        if 'compressionlevel' in params:
> +            relevant.add('compressionlevel')
> +            try:
> +                level = int(params['compressionlevel'])
> +            except ValueError:
> +                raise ValueError(_('compressionlevel "%s" is not an integer') %
> +                                 params['compressionlevel'])
> +
> +            if level < -1 or level > 9:
> +                raise ValueError(_('zlib compression level must be between -1 '
> +                                   'and 9'))
> +
> +            opts['level'] = level
> +
> +        return relevant, opts
> +
>      def wireprotosupport(self):
>          return compewireprotosupport('zlib', 20, 20)
>
> @@ -3568,9 +3606,40 @@ class _zstdengine(compressionengine):
>
>          If this engine is available and backwards compatibility is not a
>          concern, it is likely the best available engine.
> +
> +        The ``compressionlevel`` parameter defines the integer compression
> +        level to use. The default (``-1``) is equivalent to ``3``, which
> +        should deliver better-than-gzip compression ratios while being
> +        faster. Compression levels over ``10`` can yield significantly
> +        smaller bundles at a cost of much slower execution. Higher compression
> +        levels also require more memory for both compression and
> +        decompression. Compression levels higher than ``19`` can require
> +        hundreds of megabytes of memory and may exhaust memory in 32-bit
> +        processes.
>          """
>          return 'zstd', 'ZS'
>
> +    def parsebundlespecparams(self, params):
> +        relevant = set()
> +        opts = {}
> +
> +        if 'compressionlevel' in params:
> +            relevant.add('compressionlevel')
> +            try:
> +                level = int(params['compressionlevel'])
> +            except ValueError:
> +                raise ValueError(_('compressionlevel "%s" is not an integer') %
> +                                 params['compressionlevel'])
> +
> +            max_level = self._module.MAX_COMPRESSION_LEVEL
> +            if level < -1 or level > max_level:
> +                raise ValueError(_('zstd compression level must be between -1 '
> +                                   'and %d') % max_level)
> +
> +            opts['level'] = level
> +
> +        return relevant, opts
> +
>      def wireprotosupport(self):
>          return compewireprotosupport('zstd', 50, 50)
>
> diff --git a/tests/test-bundle-type.t b/tests/test-bundle-type.t
> --- a/tests/test-bundle-type.t
> +++ b/tests/test-bundle-type.t
> @@ -48,6 +48,31 @@ Unknown compression type is rejected
>    (see 'hg help bundlespec' for supported values for --type)
>    [255]
>
> +Invalid compression levels are rejected
> +
> +  $ hg bundle -a -t 'gzip-v2;compressionlevel=foo' out.hg
> +  abort: compressionlevel "foo" is not an integer
> +  (see 'hg help bundlespec' for supported values for --type)
> +  [255]
> +
> +  $ hg bundle -a -t 'gzip-v2;compressionlevel=10' out.hg
> +  abort: zlib compression level must be between -1 and 9
> +  (see 'hg help bundlespec' for supported values for --type)
> +  [255]
> +
> +#if zstd
> +  $ hg bundle -a -t 'zstd-v2;compressionlevel=foo' out.hg
> +  abort: compressionlevel "foo" is not an integer
> +  (see 'hg help bundlespec' for supported values for --type)
> +  [255]
> +
> +  $ hg bundle -a -t 'zstd-v2;compressionlevel=42' out.hg
> +  abort: zstd compression level must be between -1 and 22
> +  (see 'hg help bundlespec' for supported values for --type)
> +  [255]
> +
> +#endif
> +
>    $ cd ..
>
>  test bundle types
> diff --git a/tests/test-help.t b/tests/test-help.t
> --- a/tests/test-help.t
> +++ b/tests/test-help.t
> @@ -1849,6 +1849,7 @@ Compression engines listed in `hg help b
>          This engine will likely produce smaller bundles than "gzip" but will be
>        "gzip"
>          better compression than "gzip". It also frequently yields better
> +        better-than-gzip compression ratios while being faster. Compression
>
>  Test usage of section marks in help documents
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Gregory Szorc - April 3, 2017, 9:55 p.m.
On Mon, Apr 3, 2017 at 12:58 PM, Augie Fackler <raf@durin42.com> wrote:

> On Sat, Apr 01, 2017 at 03:31:56PM -0700, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc@gmail.com>
> > # Date 1491079470 25200
> > #      Sat Apr 01 13:44:30 2017 -0700
> > # Node ID 62e377f673f3d9e10701d373b82f995085b54363
> > # Parent  5cc2f25b803a0184fdc4e67142b65df752e40284
> > util: validate and extract compression-related bundlespec parameters
> >
> > There is currently an experimental config option used by
> > `hg bundle` to declare the compression level to be used by
> > the compression engine. I implemented this just before the 4.1
> > freeze so there would be a way to adjust the zstd compression
> > level. (Because zstd is highly flexible and one may want to optimize
> > for extreme speed or extreme compression.) My intent was always
> > to formalize compression engine settings later.
> >
> > Now that we have formalized the *bundlespec* string and exposed
> > it to users via documentation, we can start leaning on it - and
> > its extensibility via parameters - to declare compression options.
> >
> > This patch introduces a compression engine method for validating
> > and extracting relevant bundlespec parameters used by the
> > compression engine. This allows each compression engine to define
> > its own set of tunable settings.
> >
> > For the gzip and zstd engines, we have defined a method that
> > turns the "compressionlevel" parameter into a "level" option
> > for compression. I have plans to add more parameters to the zstd
> > engine. But I'll add those in a follow-up.
> >
> > To prove this works, we call the new API from bundlespec parsing
> > and add tests demonstrating failures.
> >
> > The added API returns a set of parameters relevant to the engine.
> > While unused, my eventual intent is for bundlespec parsing to know
> > when there are "unknown" parameters so it can warn about their
> > presence. Since a compression engine could consume *any* parameter,
> > it needs to tell the bundlespec parser which parameters are relevant
> > to it. While this is a YAGNI violation, it will prevent future BC
> > in the compression engine API.
> >
> > diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> > --- a/mercurial/exchange.py
> > +++ b/mercurial/exchange.py
> > @@ -163,8 +163,15 @@ def parsebundlespec(repo, spec, strict=T
> >                      _('missing support for repository features: %s') %
> >                        ', '.join(sorted(missingreqs)))
> >
> > +    engine = util.compengines.forbundlename(compression)
> > +
> > +    # Verify compression-related parameters with compression engine.
> > +    try:
> > +        engine.parsebundlespecparams(params)
> > +    except Exception as e:
>
> Yikes. Can we make the API contract more narrow here and not do a
> catch-all except?
>

Yes (at the risk of not catching all exceptions and converting to the
expected type). I figured the function is sufficiently narrow in scope that
an "except Exception" is tolerable. It's not like we're swallowing
KeyboardInterrupt and SystemExit here ;)


>
> > +        raise error.UnsupportedBundleSpecification(str(e))
> > +
> >      if not externalnames:
> > -        engine = util.compengines.forbundlename(compression)
> >          compression = engine.bundletype()[1]
> >          version = _bundlespeccgversions[version]
> >      return compression, version, params
> > diff --git a/mercurial/help/bundlespec.txt b/mercurial/help/bundlespec.
> txt
> > --- a/mercurial/help/bundlespec.txt
> > +++ b/mercurial/help/bundlespec.txt
> > @@ -82,3 +82,7 @@ Examples
> >
> >  ``zstd-v1``
> >      This errors because ``zstd`` is not supported for ``v1`` types.
> > +
> > +``zstd-v2;compressionlevel=11``
> > +   Produce a ``v2`` bundle with zstandard compression using compression
> level
> > +   ``11`` (which will take longer to produce a smaller bundle).
> > diff --git a/mercurial/util.py b/mercurial/util.py
> > --- a/mercurial/util.py
> > +++ b/mercurial/util.py
> > @@ -3276,6 +3276,20 @@ class compressionengine(object):
> >          """
> >          return None
> >
> > +    def parsebundlespecparams(self, params):
> > +        """Parse *bundlespec* parameters to a dict of options.
> > +
> > +        The *bundlespec* string may define key-value parameters. This
> method
> > +        is used to validate and extract parameters relevant to
> compression.
> > +
> > +        Returns a 2-tuple of a set of parameter names that are relevant
> to this
> > +        engine and a dict of options that will possibly be fed as the
> ``opts``
> > +        argument into a subsequent compression operation.
> > +
> > +        Exceptions can be raised to indicate an invalid parameter value.
> > +        """
> > +        return set(), {}
> > +
> >      def wireprotosupport(self):
> >          """Declare support for this compression format on the wire
> protocol.
> >
> > @@ -3362,9 +3376,33 @@ class _zlibengine(compressionengine):
> >          All Mercurial clients should support this format. The
> compression
> >          algorithm strikes a reasonable balance between compression ratio
> >          and size.
> > +
> > +        The ``compressionlevel`` parameter defines the integer
> compression
> > +        level to use. The default (``-1``) is likely equivalent to
> ``6``.
> > +        ``0`` means no compression. ``9`` means maximum compression.
> >          """
> >          return 'gzip', 'GZ'
> >
> > +    def parsebundlespecparams(self, params):
> > +        relevant = set()
> > +        opts = {}
> > +
> > +        if 'compressionlevel' in params:
> > +            relevant.add('compressionlevel')
> > +            try:
> > +                level = int(params['compressionlevel'])
> > +            except ValueError:
> > +                raise ValueError(_('compressionlevel "%s" is not an
> integer') %
> > +                                 params['compressionlevel'])
> > +
> > +            if level < -1 or level > 9:
> > +                raise ValueError(_('zlib compression level must be
> between -1 '
> > +                                   'and 9'))
> > +
> > +            opts['level'] = level
> > +
> > +        return relevant, opts
> > +
> >      def wireprotosupport(self):
> >          return compewireprotosupport('zlib', 20, 20)
> >
> > @@ -3568,9 +3606,40 @@ class _zstdengine(compressionengine):
> >
> >          If this engine is available and backwards compatibility is not a
> >          concern, it is likely the best available engine.
> > +
> > +        The ``compressionlevel`` parameter defines the integer
> compression
> > +        level to use. The default (``-1``) is equivalent to ``3``, which
> > +        should deliver better-than-gzip compression ratios while being
> > +        faster. Compression levels over ``10`` can yield significantly
> > +        smaller bundles at a cost of much slower execution. Higher
> compression
> > +        levels also require more memory for both compression and
> > +        decompression. Compression levels higher than ``19`` can require
> > +        hundreds of megabytes of memory and may exhaust memory in 32-bit
> > +        processes.
> >          """
> >          return 'zstd', 'ZS'
> >
> > +    def parsebundlespecparams(self, params):
> > +        relevant = set()
> > +        opts = {}
> > +
> > +        if 'compressionlevel' in params:
> > +            relevant.add('compressionlevel')
> > +            try:
> > +                level = int(params['compressionlevel'])
> > +            except ValueError:
> > +                raise ValueError(_('compressionlevel "%s" is not an
> integer') %
> > +                                 params['compressionlevel'])
> > +
> > +            max_level = self._module.MAX_COMPRESSION_LEVEL
> > +            if level < -1 or level > max_level:
> > +                raise ValueError(_('zstd compression level must be
> between -1 '
> > +                                   'and %d') % max_level)
> > +
> > +            opts['level'] = level
> > +
> > +        return relevant, opts
> > +
> >      def wireprotosupport(self):
> >          return compewireprotosupport('zstd', 50, 50)
> >
> > diff --git a/tests/test-bundle-type.t b/tests/test-bundle-type.t
> > --- a/tests/test-bundle-type.t
> > +++ b/tests/test-bundle-type.t
> > @@ -48,6 +48,31 @@ Unknown compression type is rejected
> >    (see 'hg help bundlespec' for supported values for --type)
> >    [255]
> >
> > +Invalid compression levels are rejected
> > +
> > +  $ hg bundle -a -t 'gzip-v2;compressionlevel=foo' out.hg
> > +  abort: compressionlevel "foo" is not an integer
> > +  (see 'hg help bundlespec' for supported values for --type)
> > +  [255]
> > +
> > +  $ hg bundle -a -t 'gzip-v2;compressionlevel=10' out.hg
> > +  abort: zlib compression level must be between -1 and 9
> > +  (see 'hg help bundlespec' for supported values for --type)
> > +  [255]
> > +
> > +#if zstd
> > +  $ hg bundle -a -t 'zstd-v2;compressionlevel=foo' out.hg
> > +  abort: compressionlevel "foo" is not an integer
> > +  (see 'hg help bundlespec' for supported values for --type)
> > +  [255]
> > +
> > +  $ hg bundle -a -t 'zstd-v2;compressionlevel=42' out.hg
> > +  abort: zstd compression level must be between -1 and 22
> > +  (see 'hg help bundlespec' for supported values for --type)
> > +  [255]
> > +
> > +#endif
> > +
> >    $ cd ..
> >
> >  test bundle types
> > diff --git a/tests/test-help.t b/tests/test-help.t
> > --- a/tests/test-help.t
> > +++ b/tests/test-help.t
> > @@ -1849,6 +1849,7 @@ Compression engines listed in `hg help b
> >          This engine will likely produce smaller bundles than "gzip" but
> will be
> >        "gzip"
> >          better compression than "gzip". It also frequently yields better
> > +        better-than-gzip compression ratios while being faster.
> Compression
> >
> >  Test usage of section marks in help documents
> >
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Augie Fackler - April 3, 2017, 11:56 p.m.
> On Apr 3, 2017, at 5:55 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote:
> 
> Yes (at the risk of not catching all exceptions and converting to the expected type). I figured the function is sufficiently narrow in scope that an "except Exception" is tolerable. It's not like we're swallowing KeyboardInterrupt and SystemExit here ;)

On the one hand, you’re probably right, but on the other, it’ll make debugging unintentional defects in the author’s decompression code super-annoying. I’d rather we were more narrow about the kinds of exceptions that are legal to throw from the called code here.

(If you think I’m being too orthodox about this, it’s possible, since I know I’m biasing hard towards long-term maintenance here.)

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -163,8 +163,15 @@  def parsebundlespec(repo, spec, strict=T
                     _('missing support for repository features: %s') %
                       ', '.join(sorted(missingreqs)))
 
+    engine = util.compengines.forbundlename(compression)
+
+    # Verify compression-related parameters with compression engine.
+    try:
+        engine.parsebundlespecparams(params)
+    except Exception as e:
+        raise error.UnsupportedBundleSpecification(str(e))
+
     if not externalnames:
-        engine = util.compengines.forbundlename(compression)
         compression = engine.bundletype()[1]
         version = _bundlespeccgversions[version]
     return compression, version, params
diff --git a/mercurial/help/bundlespec.txt b/mercurial/help/bundlespec.txt
--- a/mercurial/help/bundlespec.txt
+++ b/mercurial/help/bundlespec.txt
@@ -82,3 +82,7 @@  Examples
 
 ``zstd-v1``
     This errors because ``zstd`` is not supported for ``v1`` types.
+
+``zstd-v2;compressionlevel=11``
+   Produce a ``v2`` bundle with zstandard compression using compression level
+   ``11`` (which will take longer to produce a smaller bundle).
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -3276,6 +3276,20 @@  class compressionengine(object):
         """
         return None
 
+    def parsebundlespecparams(self, params):
+        """Parse *bundlespec* parameters to a dict of options.
+
+        The *bundlespec* string may define key-value parameters. This method
+        is used to validate and extract parameters relevant to compression.
+
+        Returns a 2-tuple of a set of parameter names that are relevant to this
+        engine and a dict of options that will possibly be fed as the ``opts``
+        argument into a subsequent compression operation.
+
+        Exceptions can be raised to indicate an invalid parameter value.
+        """
+        return set(), {}
+
     def wireprotosupport(self):
         """Declare support for this compression format on the wire protocol.
 
@@ -3362,9 +3376,33 @@  class _zlibengine(compressionengine):
         All Mercurial clients should support this format. The compression
         algorithm strikes a reasonable balance between compression ratio
         and size.
+
+        The ``compressionlevel`` parameter defines the integer compression
+        level to use. The default (``-1``) is likely equivalent to ``6``.
+        ``0`` means no compression. ``9`` means maximum compression.
         """
         return 'gzip', 'GZ'
 
+    def parsebundlespecparams(self, params):
+        relevant = set()
+        opts = {}
+
+        if 'compressionlevel' in params:
+            relevant.add('compressionlevel')
+            try:
+                level = int(params['compressionlevel'])
+            except ValueError:
+                raise ValueError(_('compressionlevel "%s" is not an integer') %
+                                 params['compressionlevel'])
+
+            if level < -1 or level > 9:
+                raise ValueError(_('zlib compression level must be between -1 '
+                                   'and 9'))
+
+            opts['level'] = level
+
+        return relevant, opts
+
     def wireprotosupport(self):
         return compewireprotosupport('zlib', 20, 20)
 
@@ -3568,9 +3606,40 @@  class _zstdengine(compressionengine):
 
         If this engine is available and backwards compatibility is not a
         concern, it is likely the best available engine.
+
+        The ``compressionlevel`` parameter defines the integer compression
+        level to use. The default (``-1``) is equivalent to ``3``, which
+        should deliver better-than-gzip compression ratios while being
+        faster. Compression levels over ``10`` can yield significantly
+        smaller bundles at a cost of much slower execution. Higher compression
+        levels also require more memory for both compression and
+        decompression. Compression levels higher than ``19`` can require
+        hundreds of megabytes of memory and may exhaust memory in 32-bit
+        processes.
         """
         return 'zstd', 'ZS'
 
+    def parsebundlespecparams(self, params):
+        relevant = set()
+        opts = {}
+
+        if 'compressionlevel' in params:
+            relevant.add('compressionlevel')
+            try:
+                level = int(params['compressionlevel'])
+            except ValueError:
+                raise ValueError(_('compressionlevel "%s" is not an integer') %
+                                 params['compressionlevel'])
+
+            max_level = self._module.MAX_COMPRESSION_LEVEL
+            if level < -1 or level > max_level:
+                raise ValueError(_('zstd compression level must be between -1 '
+                                   'and %d') % max_level)
+
+            opts['level'] = level
+
+        return relevant, opts
+
     def wireprotosupport(self):
         return compewireprotosupport('zstd', 50, 50)
 
diff --git a/tests/test-bundle-type.t b/tests/test-bundle-type.t
--- a/tests/test-bundle-type.t
+++ b/tests/test-bundle-type.t
@@ -48,6 +48,31 @@  Unknown compression type is rejected
   (see 'hg help bundlespec' for supported values for --type)
   [255]
 
+Invalid compression levels are rejected
+
+  $ hg bundle -a -t 'gzip-v2;compressionlevel=foo' out.hg
+  abort: compressionlevel "foo" is not an integer
+  (see 'hg help bundlespec' for supported values for --type)
+  [255]
+
+  $ hg bundle -a -t 'gzip-v2;compressionlevel=10' out.hg
+  abort: zlib compression level must be between -1 and 9
+  (see 'hg help bundlespec' for supported values for --type)
+  [255]
+
+#if zstd
+  $ hg bundle -a -t 'zstd-v2;compressionlevel=foo' out.hg
+  abort: compressionlevel "foo" is not an integer
+  (see 'hg help bundlespec' for supported values for --type)
+  [255]
+
+  $ hg bundle -a -t 'zstd-v2;compressionlevel=42' out.hg
+  abort: zstd compression level must be between -1 and 22
+  (see 'hg help bundlespec' for supported values for --type)
+  [255]
+
+#endif
+
   $ cd ..
 
 test bundle types
diff --git a/tests/test-help.t b/tests/test-help.t
--- a/tests/test-help.t
+++ b/tests/test-help.t
@@ -1849,6 +1849,7 @@  Compression engines listed in `hg help b
         This engine will likely produce smaller bundles than "gzip" but will be
       "gzip"
         better compression than "gzip". It also frequently yields better
+        better-than-gzip compression ratios while being faster. Compression
 
 Test usage of section marks in help documents