On Tue, Jul 25, 2017 at 6:39 AM, Daniel Shahaf <d...@daniel.shahaf.name> wrote:
> 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...@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
+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.
+1
>
> - 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.
+1
>
> Cheers,
>
> Daniel
