[tarantool-patches] Re: [PATCH 1/2] wal: deprecate rows_per_wal in favour of wal_max_size

[Alexander Turenko]

I think it would look simpler if we would not use translation table to
mark options for which we want just give a warning w/o any additional
actions.

I mean, leave 'translate_cfg' as is and add another table:

 | deprecate_cfg = {
 |     'rows_per_wal',
 | }

But the patch was already pushed and this is not much important. So
okay.

WBR, Alexander Turenko.

On Sat, Sep 07, 2019 at 06:05:36PM +0200, Vladislav Shpilevoy wrote:
> rows_per_wal does not allow to properly limit size of WAL logs.
> A user would need to estimate size of each WAL row to do that.
> Now WAL supports option wal_max_size allowing to limit WAL size
> much more precise.
> 
> Part of #3762
> ---
>  src/box/lua/load_cfg.lua | 39 ++++++++++++++++++++++++++++-----------
>  1 file changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index d402468ab..4432e0d08 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -283,19 +283,32 @@ local function convert_gb(size)
>      return math.floor(size * 1024 * 1024 * 1024)
>  end
>  
> --- Old to new config translation tables
> +-- Old to new config translation tables. In case a translation is
> +-- not 1-to-1, then a function can be used. It takes 2 parameters:
> +-- value of the old option, value of the new if present. It
> +-- returns two values - value to replace the old option and to
> +-- replace the new one.
>  local translate_cfg = {
>      snapshot_count = {'checkpoint_count'},
>      snapshot_period = {'checkpoint_interval'},
> -    slab_alloc_arena = {'memtx_memory', convert_gb},
> +    slab_alloc_arena = {'memtx_memory', function(old)
> +        return nil, convert_gb(old)
> +    end},
>      slab_alloc_minimal = {'memtx_min_tuple_size'},
>      slab_alloc_maximal = {'memtx_max_tuple_size'},
>      snap_dir = {'memtx_dir'},
>      logger = {'log'},
>      logger_nonblock = {'log_nonblock'},
> -    panic_on_snap_error = {'force_recovery', function (p) return not p end},
> -    panic_on_wal_error = {'force_recovery', function (p) return not p end},
> +    panic_on_snap_error = {'force_recovery', function(old)
> +        return nil, not old end
> +    },
> +    panic_on_wal_error = {'force_recovery', function(old)
> +        return nil, not old end
> +    },
>      replication_source = {'replication'},
> +    rows_per_wal = {'wal_max_size', function(old, new)
> +        return old, new
> +    end},
>  }
>  
>  -- Upgrade old config
> @@ -307,19 +320,23 @@ local function upgrade_cfg(cfg, translate_cfg)
>      for k, v in pairs(cfg) do
>          local translation = translate_cfg[k]
>          if translation ~= nil then
> -            log.warn('Deprecated option %s, please use %s instead', k, translation[1])
> -            local new_val
> -            if translation[2] == nil then
> +            local new_key = translation[1]
> +            local transofm = translation[2]
> +            log.warn('Deprecated option %s, please use %s instead', k, new_key)
> +            local new_val_orig = cfg[new_key]
> +            local old_val, new_val
> +            if transofm == nil then
>                  new_val = v
>              else
> -                new_val = translation[2](v)
> +                old_val, new_val = transofm(v, new_val_orig)
>              end
> -            if cfg[translation[1]] ~= nil and
> -               cfg[translation[1]] ~= new_val then
> +            if new_val_orig ~= nil and
> +               new_val_orig ~= new_val then
>                  box.error(box.error.CFG, k,
>                            'can not override a value for a deprecated option')
>              end
> -            result_cfg[translation[1]] = new_val
> +            result_cfg[k] = old_val
> +            result_cfg[new_key] = new_val
>          else
>              result_cfg[k] = v
>          end
> -- 
> 2.20.1 (Apple Git-117)
>