Evgeny Kotkov wrote on Wed, Aug 02, 2017 at 15:25:33 +0300:
> Daniel Shahaf <d...@daniel.shahaf.name> writes:
>
> > The documentation implies that CONFIG_OPTION_COMPRESSION can be used
> > regardless of the filesystem format, …
>
> > … but the code only reads CONFIG_OPTION_COMPRESSION in f8 filesystems.
> >
> > Given the docs as written, I would expect to be able to edit fsfs.conf
> > and replace 'compression-level = 4' with 'compression = zlib-4' without
> > doing an 'svnadmin upgrade'; so I think the
> > SVN_FS_FS__MIN_DELTIFICATION_FORMAT
> > codepath should honour CONFIG_OPTION_COMPRESSION (and error out if it is
> > set to "lz4").
>
> Unless I am missing something subtle, it would be an incompatible change.
>
> (For example, then the same fsfs.conf file in format 7 repository with
> "compression=zlib-4" would result in different behavior, depending on
> whether the repository is opened with Subversion 1.9 or 1.10.)
>
> That's the reason why the option is currently implemented only for format 8.
Fair point.
> But in the meanwhile, I think that I see the point that the documentation
> doesn't mention this. Perhaps, we could say something along the following
> lines to avoid the potential confusion?
>
> [[[
> ### In format 8 repositories created by Subversion 1.10, compression can
> ### be configured using the following option:
> ### compression = none | lz4 | zlib | zlib-1 ... zlib-9
> ### For new formats, 'compression' option DEPRECATES the previously used
> ### 'compression-level' option (see below).
> ⋮
> ### In format 4-7 repositories, only zlib compression algorithm is
> ### supported. Its compression level can be configured with the
> ### following option:
> ### compression-level = 0 ... 9 (default is 5)
Hmm. My first instinct would be to make the availability of the
'compression' knob coupled, not with the format number but with
SVN_VER_MINOR, so the rule would be "1.10 recognises the 'compression'
knob if set; 1.9 ignores it".
That _would_ cause the knob to be silently ignored on downgrades, but
the failure mode wouldn't be too bad (some performance loss; that's
expected when downgrading), and it would only affect people who edited
fsfs.conf from the default. Moreover, we can even teach fsfs to warn
whenever it sees an unrecognised option in the config file, similar to
the cmdline client:
% svn info --config-option=config:foo:bar=1
svn: warning: apr_err=SVN_ERR_CL_ARG_PARSING_ERROR
svn: warning: W205000: Ignoring unknown value 'foo'
Path: .
On the other hand, recognising the knob only in f8+ would require
administrators to learn two different ways to do one thing: "In one kind
of repositories, you disable compression by doing X, and in another kind
of repositories, you disable compression by doing Y" (where the two
kinds are "≥f8" and "≤f7"). This fails PEP 20.
All in all, I think I'm leaning towards making the knob conditional on
SVN_VER_MINOR rather than ffd->format, but not strongly. Let's say I'm
+0.5 to SVN_VER_MINOR.
Supposing the knob is coupled with the format number, should we issue
warnings about that? For example, a warning in 1.10 about 'compression'
being set when opening a f7-or-older repository, or a warning when
'compression' is set and upgrading f7-or-older to f8-or-newer? These
situations, too, would change the observed behaviour.
Cheers,
Daniel
