• yoav-steinberg's avatar
    Multiparam config set (#9748) · 0e5b813e
    yoav-steinberg authored
    We can now do: `config set maxmemory 10m repl-backlog-size 5m`
    
    ## Basic algorithm to support "transaction like" config sets:
    
    1. Backup all relevant current values (via get).
    2. Run "verify" and "set" on everything, if we fail run "restore".
    3. Run "apply" on everything (optional optimization: skip functions already run). If we fail run "restore".
    4. Return success.
    
    ### restore
    1. Run set on everything in backup. If we fail log it and continue (this puts us in an undefined
       state but we decided it's better than the alternative of panicking). This indicates either a bug
       or some unsupported external state.
    2. Run apply on everything in backup (optimization: skip functions already run). If we fail log
       it (see comment above).
    3. Return error.
    
    ## Implementation/design changes:
    * Apply function are idempotent (have no effect if they are run more than once for the same config).
    * No indication in set functions if we're reading the config or running from the `CONFIG SET` command
       (removed `update` argument).
    * Set function should set some config variable and assume an (optional) apply function will use that
       later to apply. If we know this setting can be safely applied immediately and can always be reverted
       and doesn't depend on any other configuration we can apply immediately from within the set function
       (and not store the setting anywhere). This is the case of this `dir` config, for example, which has no
       apply function. No apply function is need also in the case that setting the variable in the `server` struct
       is all that needs to be done to make the configuration take effect. Note that the original concept of `update_fn`,
       which received the old and new values was removed and replaced by the optional apply function.
    * Apply functions use settings written to the `server` struct and don't receive any inputs.
    * I take care that for the generic (non-special) configs if there's no change I avoid calling the setter (possible
       optimization: avoid calling the apply function as well).
    * Passing the same config parameter more than once to `config set` will fail. You can't do `config set my-setting
       value1 my-setting value2`.
    
    Note that getting `save` in the context of the conf file parsing to work here as before was a pain.
    The conf file supports an aggregate `save` definition, where each `save` line is added to the server's
    save params. This is unlike any other line in the config file where each line overwrites any previous
    configuration. Since we now support passing multiple save params in a single line (see top comments
    about `save` in https://github.com/redis/redis/pull/9644) we should deprecate the aggregate nature of
    this config line and perhaps reduce this ugly code in the future.
    0e5b813e
replication.c 161 KB