From 3d0d59a5bfa7429a7f5548d37cfdcb5289dc77f0 Mon Sep 17 00:00:00 2001 From: Wang Yuan Date: Fri, 16 Jul 2021 10:27:49 +0800 Subject: [PATCH] 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. --- kvrocks.conf | 26 +++++++++---------------- src/config.cc | 7 +++++-- src/config.h | 5 +++-- src/redis_cmd.cc | 4 ++-- src/redis_connection.cc | 4 ++-- tests/tcl/tests/assets/default.conf | 2 -- tests/tcl/tests/integration/cluster.tcl | 14 ++++++------- tools/try_cluster/default.conf | 2 +- 8 files changed, 29 insertions(+), 35 deletions(-) diff --git a/kvrocks.conf b/kvrocks.conf index b2f7ff0dcb1..69b3e6585fa 100644 --- 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 index 3f322990e57..50f30351baa 100644 --- 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 args; diff --git a/src/config.h b/src/config.h index a31ebd5da48..a5db76155e3 100644 --- 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 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 index 8d19ed5b578..e8e1a103cee 100644 --- 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 index 050a8dc8480..7d95d0ff0dd 100644 --- a/src/redis_connection.cc +++ b/src/redis_connection.cc @@ -316,7 +316,7 @@ void Connection::ExecuteCommands(const std::vector &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 &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 index 321152a3879..c6ff002b0cd 100644 --- 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 index fbb9da56d2d..d2e2522c98f 100644 --- 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 index e4fc57aacd0..a8fa66f839a 100644 --- 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 \ No newline at end of file +cluster-enabled yes