Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Date: Wed, 26 Jul 2017 13:19:20 +0200
From: Stefan Sperling <stsp@elego.de>
To: dev@subversion.apache.org
Cc: Daniel Shahaf <d.s@daniel.shahaf.name>, commits@subversion.apache.org
Subject: 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/
Message-ID: <20170726111920.GU90677@ted.stsp.name>
Mail-Followup-To: dev@subversion.apache.org,
	Daniel Shahaf <d.s@daniel.shahaf.name>,
	commits@subversion.apache.org
References: <20170714111350.B07063A0F99@svn01-us-west.apache.org>
 <1500300013.520913.1043351984.67FF62A9@webmail.messagingengine.com>
 <CAP_GPNjwm1Aj34WcmwfhNCZKUKgK2yxYAfpUUyZoiAi1t+YbCA@mail.gmail.com>
 <20170726095410.GR90677@ted.stsp.name>
 <CAP_GPNibuoij6VahwgssBRHp0u+4wusv4rzCgfXHhen+iJGxRw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAP_GPNibuoij6VahwgssBRHp0u+4wusv4rzCgfXHhen+iJGxRw@mail.gmail.com>
User-Agent: Mutt/1.8.0 (2017-02-23)
archived-at: Wed, 26 Jul 2017 11:21:22 -0000

On Wed, Jul 26, 2017 at 01:18:00PM +0300, Evgeny Kotkov wrote:
> Stefan Sperling <stsp@elego.de> writes:
> 
> > Does mod_dav_svn really need an option for this?
> >
> > Can't lz4 (svndiff2) be negotiated as a mutual protocol capability of
> > client and server? Why won't a simple logic such as the following work:
> >
> >   If the client announces lz4 compression level 1 support, use it.
> >   Else, use zlib.
> 
> The current state is that there are no additional options in mod_dav_svn,
> and the "SVNCompression" option has just been discussed as an explicit
> alternative to what we have now (but it also has a couple of potential
> drawbacks, and I don't plan to implement it at this time).
> 
> Currently, the negotiation scheme works similarly to what you have described.
> If the server has "SVNCompressionLevel 1", and the client supports LZ4, then
> it will be used.  Otherwise, in case of older clients, they will be using zlib
> compression with the corresponding compression level.
> 
> Apart from this, I was willing to propose that we switch to LZ4 compression
> by default for both mod_dav_svn and newly created FSFS repositories.  That
> is, by making "1" the default value of SVNCompressionLevel directive and
> by making "lz4" the default value of the discussed "compression=" knob in
> fsfs.conf.
> 
> (I will take this to a separate thread once all the necessary things are
> in place.)
> 
> 
> Regards,
> Evgeny Kotkov

Sounds like we're headed in a good direction :-)

I agree that 'compression-level = 1' -> lz4 makes sense if you know
what to expect from lz4 vs. zlib. But honestly imagining myself giving
a workshop for administrators and explaining it like that, I see people
shake their heads in disbelief. It looks too much like an implementation
detail leaked into the config interface. A compression=lz4/zlib knob would
make a lot more sense to most people, with a corresponding subordinate
default for compression-level (lz4: 1, zlib: 5).

I strongly believe we should not require an admin to even look at fsfs.conf
to enable lz4. Because if admins look at this file today, they will easily get
overwhelmed with choices. I think we've been a bit too eager with adding
knobs to fsfs.conf in general. Most people never tweak them, and if anyone
asks I generally recommend to just leave them all at their defaults.
In hindsight, I would say all these options have a questionable reason
for existence and we should not have exposed them:

fail-stop
enable-rep-sharing
enable-dir-deltification
enable-props-deltification
max-deltification-walk
max-linear-deltification
compression-level
revprop-pack-size
compress-packed-revprops
block-size
l2p-page-size
p2l-page-size

I mean, apart from Stefan2, which person on this planet *really* knows what
*all* of these knobs do, why anyone would want to tweak which knob, and how
they potentially interact?

It would have been better to automatically adjust some of these values
at run-time where possible, or decide on doing something one way instead
of supporting several ways.