kot...@apache.org wrote on Tue, 01 Aug 2017 12:18 +0000:
> Author: kotkov
> Date: Tue Aug  1 12:18:23 2017
> New Revision: 1803639
> 
> URL: http://svn.apache.org/viewvc?rev=1803639&view=rev
> Log:
> fsfs: Introduce new 'compression' config option.
> 
> This option allows explicitly specifying the compression algorithm for
> format 8 repositories.  It deprecates the previously used 'compression-level'
> option.  The syntax of the new option is:
> 
>   compression = none | lz4 | zlib | zlib-1 ... zlib-9

Thanks for implementing this, Evgeny.  One comment:

> +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Tue Aug  1 12:18:23 2017
> @@ -947,23 +1052,25 @@ write_config(svn_fs_t *fs,
>  "# " CONFIG_OPTION_MAX_LINEAR_DELTIFICATION " = 16"                          
> NL
>  "###"                                                                        
> NL
>  "### After deltification, we compress the data to minimize on-disk size."    
> NL
> +"### This setting controls the compression algorithm, which will be used in" 
> NL
> +"### future revisions.  It can be used to either disable compression or to"  
> NL
> +"### select between available algorithms (zlib, lz4).  zlib is a general-"   
> NL
> +"### purpose compression algorithm.  lz4 is a fast compression algorithm"    
> NL
> +"### which should be preferred for repositories with large and, possibly,"   
> NL
> +"### incompressible files.  Note that the compression ratio of lz4 is"       
> NL
> +"### usually lower than the one provided by zlib, but using it can"          
> NL
> +"### significantly speed up commits as well as reading the data."            
> NL
> +"### The syntax of this option is:"                                          
> NL
> +"###   " CONFIG_OPTION_COMPRESSION " = none | lz4 | zlib | zlib-1 ... 
> zlib-9" NL
> +"### The default value is 'zlib', which is currently equivalent to 
> 'zlib-5'." NL
> +"# " CONFIG_OPTION_COMPRESSION " = zlib"                                     
> NL
> +"###"                                                                        
> NL
> +"### DEPRECATED: The new '" CONFIG_OPTION_COMPRESSION "' option deprecates 
> previously used" NL
> +"### '" CONFIG_OPTION_COMPRESSION_LEVEL "' option, which was used to 
> configure zlib compression." NL
> +"### For compatibility with previous versions of Subversion, this option 
> can"NL
> +"### still be used (and it will result in zlib compression with the"         
> NL
> +"### corresponding compression level)."                                      
> NL
> +"###   " CONFIG_OPTION_COMPRESSION_LEVEL " = 0 ... 9 (default is 5)"         
> NL

The documentation implies that CONFIG_OPTION_COMPRESSION can be used
regardless of the filesystem format, …

> @@ -683,6 +683,60 @@ verify_block_size(apr_int64_t block_size
> +static svn_error_t *
> +parse_compression_option(compression_type_t *compression_type_p,
> +                         int *compression_level_p,
> +                         const char *value)
> +{
> @@ -816,6 +859,68 @@ read_config(fs_fs_data_t *ffd,
>      {
>        ffd->pack_after_commit = FALSE;
>      }
> +
> +  /* Initialize compression settings in ffd. */
> +  if (ffd->format >= SVN_FS_FS__MIN_SVNDIFF2_FORMAT)
> +    {
> +      svn_config_get(config, &compression_val,
> +                     CONFIG_SECTION_DELTIFICATION,
> +                     CONFIG_OPTION_COMPRESSION, NULL);
> +      svn_config_get(config, &compression_level_val,
> +                     CONFIG_SECTION_DELTIFICATION,
> +                     CONFIG_OPTION_COMPRESSION_LEVEL, NULL);
> +    }
> +  else if (ffd->format >= SVN_FS_FS__MIN_DELTIFICATION_FORMAT)
> +    {
> +      SVN_ERR(svn_config_get_int64(config, &compression_level,
> +                                   CONFIG_SECTION_DELTIFICATION,
> +                                   CONFIG_OPTION_COMPRESSION_LEVEL,
> +                                   SVN_DELTA_COMPRESSION_LEVEL_DEFAULT));

… 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").

Cheers,

Daniel

> +    }
> +  else if (ffd->format >= SVN_FS_FS__MIN_SVNDIFF1_FORMAT)
> +    {
> +      ffd->delta_compression_type = compression_type_zlib;
> +      ffd->delta_compression_level = SVN_DELTA_COMPRESSION_LEVEL_DEFAULT;
> +    }
> +  else
> +    {
> +      ffd->delta_compression_type = compression_type_none;
> +      ffd->delta_compression_level = SVN_DELTA_COMPRESSION_LEVEL_NONE;
> +    }
> +
>  #ifdef SVN_DEBUG
>    SVN_ERR(svn_config_get_bool(config, &ffd->verify_before_commit,
>                                CONFIG_SECTION_DEBUG,

Reply via email to