Skip to content

Use module config API#360

Merged
tezc merged 3 commits intoRedisLabs:masterfrom
tezc:config-api
Jun 30, 2022
Merged

Use module config API#360
tezc merged 3 commits intoRedisLabs:masterfrom
tezc:config-api

Conversation

@tezc
Copy link
Collaborator

@tezc tezc commented Jun 4, 2022

RedisRaft adopts new configuration API: redis/redis#10285

After this commit, RAFT.CONFIG will not exist anymore. CONFIG command will be used instead.

Usage:

  • Command: config set raft.addr 127.0.0.1:8000 and config get raft.id
  • CLI: ./redis-server --port 5001 --loadmodule redisraft.so --raft.id 3 --raft.addr 127.0.0.1:8000
  • Config file:
    ...
    port 5000
    dbfilename redis.rdb
    loadmodule /home/ozan/Desktop/redisraft/redisraft.so
    raft.addr 10.0.3.69:5022
    raft.slot-config 0:5460
    

Breaking changes

  1. CONFIG command will be used instead of RAFT.CONFIG

  2. Configuration parameters will require raft. or --raft. prefix.

  3. Renamed some configuration parameters:

    Old name New name
    raft-log-filename log-filename
    raft-interval periodic-interval
    raft-response-timeout response-timeout
    raft-log-max-cache-size log-max-cache-size
    raft-log-max-file-size log-max-file-size
    raft-log-fsync log-fsync

    Reasoning: As all parameters now have raft. prefix, parameters already starting with raft would include too many "raft"s :). With this change, instead of raft.raft-log-fsync, we'll have raft.log-fsync.

  4. join-timeout config was in seconds. Now, it is in milliseconds.
    connection-timeout config was in microseconds. Now, it is in milliseconds.

    Reasoning: Now, all timeout configs are in milliseconds. I suggest keeping it that way for future configuration parameters as well. This is easy to remember and it prevents confusion.


Jepsen part: RedisLabs/jepsen-redisraft#3

Closes: #306

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2022

Codecov Report

Attention: Patch coverage is 85.46099% with 82 lines in your changes missing coverage. Please review.

Project coverage is 54.70%. Comparing base (24f2cb3) to head (04a12d0).
Report is 244 commits behind head on master.

Files with missing lines Patch % Lines
src/config.c 84.50% 64 Missing ⚠️
src/commands.c 75.60% 10 Missing ⚠️
src/raft.c 91.66% 3 Missing ⚠️
src/redisraft.c 90.47% 2 Missing ⚠️
src/cluster.c 88.88% 1 Missing ⚠️
src/common.c 50.00% 1 Missing ⚠️
src/snapshot.c 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #360      +/-   ##
==========================================
+ Coverage   53.14%   54.70%   +1.55%     
==========================================
  Files          37       37              
  Lines       13513    13284     -229     
  Branches     1627     1532      -95     
==========================================
+ Hits         7182     7267      +85     
+ Misses       6331     6017     -314     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fadidahanna
Copy link
Contributor

RedisRaft works only on Redis 6.0 and above.- with your current changes changing raft configuration on the fly is supported only for Redis 7.0 - do we want to update the doc too?

@tezc
Copy link
Collaborator Author

tezc commented Jun 19, 2022

Updated PR, not using module args anymore.

fadidahanna
fadidahanna previously approved these changes Jun 19, 2022
@tezc tezc merged commit 0715618 into RedisLabs:master Jun 30, 2022
tezc added a commit that referenced this pull request Jul 11, 2022
Delete duplicate command filter registration

Introduced by #360, the command filter will be registered twice as part
of a rebase mistake. It is harmless but unnecessary. Also, as we are not
using the command filter object itself, we don't need to keep 
a reference to it.
@tezc tezc deleted the config-api branch August 14, 2022 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adopt new Module Configuration mechanism

4 participants