[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1801940 - in /subversion/trunk: ./ notes/ subversion/include/ subversion/include/private/ subversion/libsvn_delta/ subversion/libsvn_fs_fs/ subversion/libsvn_subr/ subversion/tests/libsvn_delta/ subversion/tests/libsvn_subr/

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Tue, 25 Jul 2017 11:39:03 +0000

Good morning Evgeny,

First of all, thanks for the well written response.

Evgeny Kotkov wrote on Mon, 24 Jul 2017 19:19 +0300:
> Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
> > I'm a bit uncomfortable with this logic.
> >
> > 1. It violates the principle of least surprise: compression-level=9
> > means 'gzip -9', compression-level=5 means 'gzip -5', but
> > compression-level=1 means LZ4 (with the default acceleration_factor)
> > rather than 'gzip -1'.
> >
> > 2. It leaves no way to use zlib level 1 in f8 filesystems. This seems
> > like a decision that should be left to the admin, rather than hardcoded
> > into the library.
> >
> > 3. What if somebody wanted to add a backend with, say, xz compression.
> > (xz compression also takes levels like gzip.) Would it make sense to have
> > two tunables:
> > .
> > compression-backend = { lz4 | zlib }
> > compression-level = {1..N for lz4, 1..9 for zlib}
> > .
> > and then other compression backends could be easily added?
> >
> > This would also allow admins to set the 'acceleration_factor' of lz4.
>
> First of all, I agree that this logic has drawbacks if observed from the
> idealistic point of view:
> - the choice of the compression algorithm happens implicitly, and
> - it doesn't allow users to use stop using LZ4 for any reason with
> the compression level set to 1.
>
> In the meanwhile, I think that the current approach is quite pragmatic,
> as LZ4 is a suitable alternative for zlib1, and considering the big picture
> with a similar configuration knob in mod_dav_svn, where the choice of the
> compression algorithm is tied to the negotiation of the wire format for
> older clients (see below).

I'm afraid I disagree with both of these points.

You keep stating "lz4 is better than zlib(level=1)" as an absolute fact.
I do not see it this way. Yes, there is a benchmark you cited that
scored lz4 higher than zlib, but that does not imply that lz4 is
universally preferable to zlib (and that it will remain so for the
lifetime of the 1.10.x series).

Secondly, the compatibility requirements of mod_dav_svn and FSFS are
different. mod_dav_svn is supposed to support multiple client minor
versions simultaneously; FSFS did a format bump explicitly because
supporting 1.9 and 1.10 simultaneously was not possible.

> When I've been thinking about allowing an explicit choice of the algorithm,
> I had a slightly different line of thought, opposed to "compression-backend
> + compression-level", with a single option:
>
> compression = none | lz4 | zlib | zlib-1 ... zlib-9
>
> (The rationale is to avoid having two dependent options; as well as that,
> currently, I don't have the data showing that being able to tune the
> acceleration factor of LZ4 can noticeably improve performance.)

+1

> However, there are a couple of difficulties with porting this approach to
> mod_dav_svn, i.e., if we introduce the SVNCompression directive. There
> are clients that don't use LZ4, so, presumably, this options would require
> specifying all formats that a server can use, in the preferred order:
>
> SVNCompression "lz4, zlib"
>
> While such approach is explicit, it also has a couple of drawbacks, as it:
> - leaves a window for mistakes (say, if the user sets "SVNCompression lz4"
> and inadvertently disables compression for older clients),
> - is not forward-compatible, as new compression algorithms require the
> server to be reconfigured, and
> - adds complexity.
>

- We can detect the configuration "SVNCompression lz4" and error out on it.

- Forward compatibility: I'm not convinced that we even _need_ forward
  compatibility for this; we can just tell people who set this knob that
  they may miss out on new compression algorithms if they don't review
  the setting of that knob when they upgrade to a new minor version.
  (That's exactly how the client-side config:auth:password-stores
  works.) There are other options to solve the compatibility issue, but
  as you say, they would add complexity.
  
- I don't see how the fact that ┬źSVNCompression "lz4, zlib"┬╗ might be
  considered too complex to add, affects my arguments about fsfs.conf.
  As I said earlier, FSFS f8 does not need to support 1.9 and 1.10
  clients in parallel, so it has no need for a compression negotiation
  configuration. Perhaps you could clarify the fsfs part of your
  argument?

> As we are lucky that LZ4 is a suitable alternative for zlib1, and that our
> current configuration knobs are not tightly bound to zlib, I propose that we
> keep the current logic for now and postpone the generic solution up to the
> moment when we add another compression algorithm that does not fit this
> scheme or requires additional configuration.
>
> In other words, we can always do this separately, when it's absolutely needed
> (say, if we find ourselves adding a compression algorithm such as zstd).

Yes, there are further changes we could make if we find ourselves adding
more compression algorithms, but it is premature to consider them. At
this point, I simply suggest to add a fsfs.conf "compression" knob, with
the syntax you proposed, that overrides compression-level if both are set.
We can make further changes as and when.

Cheers,

Daniel
Received on 2017-07-25 13:39:14 CEST

This is an archived mail posted to the Subversion Dev mailing list.