Remove exposing slot-id-encoded config (#330)

Before, we expose slot-id-encoded in config file, that is useful to migrate keys based on slot when cluster is enabled, actually, we should only enable slot-id-encoded when we want to use cluster mode. So exposing this config may make users confused, and wrong config also make data corrupt. Now we remove this config, and set it automatically.
apache · Jul 19, 2021 · 3d0d59a · 3d0d59a
1 parent 30798e1
commit 3d0d59a
Show file tree

Hide file tree

Showing 8 changed files with 29 additions and 35 deletions.
diff --git a/kvrocks.conf b/kvrocks.conf
@@ -34,8 +34,16 @@ daemonize no
 # But kvrocks doesn't support to communicate with each others, so you must set
 # cluster topology by CLUSTER SETNODES|SETNODEID commands, more details: #219.
 #
+# PLEASE NOTE:
+# If you enable cluster, kvrocks will encode key with its slot id calculated by
+# CRC16 and modulo 16384, endoding key with its slot id makes it efficient to
+# migrate keys based on slot. So if you enabled at first time, cluster mode must
+# not be disabled after restarting, and vice versa. That is to say, data is not
+# compatible between standalone mode with cluster mode, you must migrate data
+# if you want to change mode, otherwise, kvrocks will make data corrupt.
+#
 # Default: no
-cluster-enable no
+cluster-enabled no
 
 # Set the max number of connected clients at the same time. By default
 # this limit is set to 10000 clients, however if the server is not
@@ -209,22 +217,6 @@ max-backup-to-keep 1
 # default: 1 day
 max-backup-keep-hours 24
 
-# Encode key with its slot id calculated by CRC16 and modulo 16384, if you enabled
-# at first time, this option must not be disabled after restarted, and vice versa.
-# Endoding key with its slot id make it efficient to migrate keys based on slot.
-#
-# PLEASE NOTE:
-# If you just start to use kvrocks firstly and also want to use cluster mode, please
-# enable this config, because cluster mode is dependent with enabling it for migrating
-# slot.
-#
-# If you want to upgrade your old kvrocks version(without encoding slot it into key)
-# whatever by master-replicas replication or restarting using new binary, you must
-# disable this option, otherwise, kvrocks will make data corrupt.
-#
-# Default: no
-slot-id-encoded no
-
 # Ratio of the samples would be recorded when the profiling was enabled. 
 # we simply use the rand to determine whether to record the sample or not.
 # 

diff --git a/src/config.cc b/src/config.cc
@@ -71,7 +71,6 @@ Config::Config() {
       {"maxclients", false, new IntField(&maxclients, 10240, 0, INT_MAX)},
       {"max-backup-to-keep", false, new IntField(&max_backup_to_keep, 1, 0, 1)},
       {"max-backup-keep-hours", false, new IntField(&max_backup_keep_hours, 0, 0, INT_MAX)},
-      {"slot-id-encoded", true, new YesNoField(&slot_id_encoded, false)},
       {"master-use-repl-port", false, new YesNoField(&master_use_repl_port, false)},
       {"requirepass", false, new StringField(&requirepass, "")},
       {"masterauth", false, new StringField(&masterauth, "")},
@@ -102,7 +101,7 @@ Config::Config() {
       {"purge-backup-on-fullsync", false, new YesNoField(&purge_backup_on_fullsync, false)},
       {"rename-command", true, new StringField(&rename_command_, "")},
       {"auto-resize-block-and-sst", false, new YesNoField(&auto_resize_block_and_sst, true)},
-      {"cluster-enable", true, new YesNoField(&cluster_enable_, false)},
+      {"cluster-enabled", true, new YesNoField(&cluster_enabled, false)},
       /* rocksdb options */
       {"rocksdb.compression", false, new EnumField(&RocksDB.compression, compression_type_enum, 0)},
       {"rocksdb.block_size", true, new IntField(&RocksDB.block_size, 4096, 0, INT_MAX)},
@@ -231,6 +230,10 @@ void Config::initFieldCallback() {
         backup_sync_dir = dir + "/backup_for_sync";
         return Status::OK();
       }},
+      {"cluster-enabled", [this](Server* srv, const std::string &k, const std::string& v)->Status {
+        if (cluster_enabled) slot_id_encoded = true;
+        return Status::OK();
+      }},
       {"bind", [this](Server* srv,  const std::string &k,  const std::string& v)->Status {
         trimRocksDBPrefix(k);
         std::vector<std::string> args;

diff --git a/src/config.h b/src/config.h
@@ -63,7 +63,6 @@ struct Config{
   int max_db_size = 0;
   int max_replication_mb = 0;
   int max_io_mb = 0;
-  bool slot_id_encoded = false;
   bool master_use_repl_port = false;
   bool purge_backup_on_fullsync = false;
   bool auto_resize_block_and_sst = true;
@@ -85,7 +84,9 @@ struct Config{
   Cron bgsave_cron;
   CompactionCheckerRange compaction_checker_range{-1, -1};
   std::map<std::string, std::string> tokens;
-  bool cluster_enable_ = false;
+
+  bool slot_id_encoded = false;
+  bool cluster_enabled = false;
 
   // profiling
   int profiling_sample_ratio = 0;

diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc
@@ -4148,7 +4148,7 @@ class CommandCluster : public Commander {
   }
 
   Status Execute(Server *svr, Connection *conn, std::string *output) override {
-    if (svr->GetConfig()->cluster_enable_ == false) {
+    if (svr->GetConfig()->cluster_enabled == false) {
       *output = Redis::Error("Cluster mode is not enabled");
       return Status::OK();
     }
@@ -4230,7 +4230,7 @@ class CommandClusterX : public Commander {
   }
 
   Status Execute(Server *svr, Connection *conn, std::string *output) override {
-    if (svr->GetConfig()->cluster_enable_ == false) {
+    if (svr->GetConfig()->cluster_enabled == false) {
       *output = Redis::Error("Cluster mode is not enabled");
       return Status::OK();
     }

diff --git a/src/redis_connection.cc b/src/redis_connection.cc
@@ -316,7 +316,7 @@ void Connection::ExecuteCommands(const std::vector<Redis::CommandTokens> &to_pro
       // No lock guard, because 'exec' command has acquired 'WorkExclusivityGuard'
     } else if (attributes->is_exclusive() ||
         (cmd_name == "config" && cmd_tokens.size() == 2 && !strcasecmp(cmd_tokens[1].c_str(), "set")) ||
-        (config->cluster_enable_ && cmd_name == "clusterx" && cmd_tokens.size() >= 2
+        (config->cluster_enabled && cmd_name == "clusterx" && cmd_tokens.size() >= 2
          && Cluster::SubCommandIsExecExclusive(cmd_tokens[1]))) {
       exclusivity = svr_->WorkExclusivityGuard();
     } else {
@@ -351,7 +351,7 @@ void Connection::ExecuteCommands(const std::vector<Redis::CommandTokens> &to_pro
       continue;
     }
 
-    if (config->cluster_enable_) {
+    if (config->cluster_enabled) {
       s = svr_->cluster_->CanExecByMySelf(attributes, cmd_tokens);
       if (!s.IsOK()) {
         if (IsFlagEnabled(Connection::kMultiExec)) multi_error_ = true;

diff --git a/tests/tcl/tests/assets/default.conf b/tests/tcl/tests/assets/default.conf
@@ -13,5 +13,3 @@ slave-serve-stale-data yes
 max-replication-mb 0
 max-io-mb 500
 max-db-size 0
-cluster-enable no
-slot-id-encoded yes
diff --git a/tests/tcl/tests/integration/cluster.tcl b/tests/tcl/tests/integration/cluster.tcl
@@ -7,7 +7,7 @@ start_server {tags {"disable-cluster"}} {
     }
 }
 
-start_server {tags {"cluster"} overrides {cluster-enable yes}} {
+start_server {tags {"cluster"} overrides {cluster-enabled yes}} {
     test {CLUSTER KEYSLOT} {
         set slot_table_len [llength $::CRC16_SLOT_TABLE]
         for {set i 0} {$i < $slot_table_len} {incr i} {
@@ -16,7 +16,7 @@ start_server {tags {"cluster"} overrides {cluster-enable yes}} {
     }
 }
 
-start_server {tags {"cluster"} overrides {cluster-enable yes}} {
+start_server {tags {"cluster"} overrides {cluster-enabled yes}} {
     set nodeid "07c37dfeb235213a872192d90877d0cd55635b91"
     r clusterx SETNODEID $nodeid
 
@@ -76,7 +76,7 @@ start_server {tags {"cluster"} overrides {cluster-enable yes}} {
     }
 }
 
-start_server {tags {"cluster"} overrides {cluster-enable yes}} {
+start_server {tags {"cluster"} overrides {cluster-enabled yes}} {
     test {cluster slots and nodes about complex topology} {
         set nodeid "07c37dfeb235213a872192d90877d0cd55635b91"
         set host [srv host]
@@ -96,22 +96,22 @@ start_server {tags {"cluster"} overrides {cluster-enable yes}} {
     }
 }
 
-start_server {tags {"cluster"} overrides {cluster-enable yes}} {
+start_server {tags {"cluster"} overrides {cluster-enabled yes}} {
     set r0 [srv 0 client]
     set node0_host [srv host]
     set node0_port [srv port]
     set node0_id "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx00"
-    start_server {tags {"cluster"} overrides {cluster-enable yes}} {
+    start_server {tags {"cluster"} overrides {cluster-enabled yes}} {
         set r1 [srv 0 client]
         set node1_host [srv host]
         set node1_port [srv port]
         set node1_id "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx01"
-        start_server {tags {"cluster"} overrides {cluster-enable yes}} {
+        start_server {tags {"cluster"} overrides {cluster-enabled yes}} {
             set r2 [srv 0 client]
             set node2_host [srv host]
             set node2_port [srv port]
             set node2_id "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx02"
-            start_server {tags {"cluster"} overrides {cluster-enable yes}} {
+            start_server {tags {"cluster"} overrides {cluster-enabled yes}} {
                 set r3 [srv 0 client]
                 set node3_host [srv host]
                 set node3_port [srv port]

diff --git a/tools/try_cluster/default.conf b/tools/try_cluster/default.conf
@@ -13,4 +13,4 @@ slave-serve-stale-data yes
 max-replication-mb 0
 max-io-mb 500
 max-db-size 0
-cluster-enable yes
+cluster-enabled yes