You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kvrocks.apache.org by hu...@apache.org on 2022/10/28 08:40:59 UTC
[incubator-kvrocks] branch unstable updated: Fix rename-command only the first one works(#1047)
This is an automated email from the ASF dual-hosted git repository.
hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/incubator-kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 29d5a952 Fix rename-command only the first one works(#1047)
29d5a952 is described below
commit 29d5a9528baa0db14ac6715e64e8f1bd1a08d5db
Author: Ruixiang Tan <81...@qq.com>
AuthorDate: Fri Oct 28 16:40:54 2022 +0800
Fix rename-command only the first one works(#1047)
---
src/config/config.cc | 50 ++++++++++++++++++++-------------
src/config/config.h | 2 +-
src/config/config_type.h | 30 ++++++++++++++++++++
tests/cppunit/config_test.cc | 42 +++++++++++++++++++++------
tests/gocase/unit/config/config_test.go | 21 ++++++++++----
5 files changed, 110 insertions(+), 35 deletions(-)
diff --git a/src/config/config.cc b/src/config/config.cc
index 6dd24856..08332610 100644
--- a/src/config/config.cc
+++ b/src/config/config.cc
@@ -149,7 +149,7 @@ Config::Config() {
{"profiling-sample-commands", false, new StringField(&profiling_sample_commands_, "")},
{"slowlog-max-len", false, new IntField(&slowlog_max_len, 128, 0, INT_MAX)},
{"purge-backup-on-fullsync", false, new YesNoField(&purge_backup_on_fullsync, false)},
- {"rename-command", true, new StringField(&rename_command_, "")},
+ {"rename-command", true, new MultiStringField(&rename_command_, std::vector<std::string>{})},
{"auto-resize-block-and-sst", false, new YesNoField(&auto_resize_block_and_sst, true)},
{"fullsync-recv-file-delay", false, new IntField(&fullsync_recv_file_delay, 0, 0, INT_MAX)},
{"cluster-enabled", true, new YesNoField(&cluster_enabled, false)},
@@ -273,23 +273,26 @@ void Config::initFieldValidator() {
}},
{"rename-command",
[](const std::string &k, const std::string &v) -> Status {
- std::vector<std::string> args = Util::Split(v, " \t");
- if (args.size() != 2) {
- return Status(Status::NotOK, "Invalid rename-command format");
- }
- auto commands = Redis::GetCommands();
- auto cmd_iter = commands->find(Util::ToLower(args[0]));
- if (cmd_iter == commands->end()) {
- return Status(Status::NotOK, "No such command in rename-command");
- }
- if (args[1] != "\"\"") {
- auto new_command_name = Util::ToLower(args[1]);
- if (commands->find(new_command_name) != commands->end()) {
- return Status(Status::NotOK, "Target command name already exists");
+ std::vector<std::string> all_args = Util::Split(v, "\n");
+ for (auto &p : all_args) {
+ std::vector<std::string> args = Util::Split(p, " \t");
+ if (args.size() != 2) {
+ return Status(Status::NotOK, "Invalid rename-command format");
}
- (*commands)[new_command_name] = cmd_iter->second;
+ auto commands = Redis::GetCommands();
+ auto cmd_iter = commands->find(Util::ToLower(args[0]));
+ if (cmd_iter == commands->end()) {
+ return Status(Status::NotOK, "No such command in rename-command");
+ }
+ if (args[1] != "\"\"") {
+ auto new_command_name = Util::ToLower(args[1]);
+ if (commands->find(new_command_name) != commands->end()) {
+ return Status(Status::NotOK, "Target command name already exists");
+ }
+ (*commands)[new_command_name] = cmd_iter->second;
+ }
+ commands->erase(cmd_iter);
}
- commands->erase(cmd_iter);
return Status::OK();
}},
};
@@ -722,8 +725,15 @@ void Config::Get(std::string key, std::vector<std::string> *values) {
values->clear();
for (const auto &iter : fields_) {
if (key == "*" || Util::ToLower(key) == iter.first) {
- values->emplace_back(iter.first);
- values->emplace_back(iter.second->ToString());
+ if (iter.second->IsMultiConfig()) {
+ for (const auto &p : Util::Split(iter.second->ToString(), "\n")) {
+ values->emplace_back(iter.first);
+ values->emplace_back(p);
+ }
+ } else {
+ values->emplace_back(iter.first);
+ values->emplace_back(iter.second->ToString());
+ }
}
}
}
@@ -754,8 +764,8 @@ Status Config::Rewrite() {
std::vector<std::string> lines;
std::map<std::string, std::string> new_config;
for (const auto &iter : fields_) {
- if (iter.first == "rename-command") {
- // We should NOT overwrite the rename command since it cannot be rewritten in-flight,
+ if (iter.second->IsMultiConfig()) {
+ // We should NOT overwrite the commands which are MultiConfig since it cannot be rewritten in-flight,
// so skip it here to avoid rewriting it as new item.
continue;
}
diff --git a/src/config/config.h b/src/config/config.h
index 6e3556fd..12d2a9d1 100644
--- a/src/config/config.h
+++ b/src/config/config.h
@@ -213,7 +213,7 @@ struct Config {
std::string compaction_checker_range_;
std::string profiling_sample_commands_;
std::map<std::string, std::unique_ptr<ConfigField>> fields_;
- std::string rename_command_;
+ std::vector<std::string> rename_command_;
void initFieldValidator();
void initFieldCallback();
diff --git a/src/config/config_type.h b/src/config/config_type.h
index 998a3bc4..27410115 100644
--- a/src/config/config_type.h
+++ b/src/config/config_type.h
@@ -36,6 +36,9 @@ struct configEnum {
const char *name;
const int val;
};
+
+enum configType { SingleConfig, MultiConfig };
+
int configEnumGetValue(configEnum *ce, const char *name);
const char *configEnumGetName(configEnum *ce, int val);
@@ -47,12 +50,16 @@ class ConfigField {
virtual Status Set(const std::string &v) = 0;
virtual Status ToNumber(int64_t *n) { return Status(Status::NotOK, "not supported"); }
virtual Status ToBool(bool *b) { return Status(Status::NotOK, "not supported"); }
+ virtual configType GetConfigType() { return config_type; }
+ virtual bool IsMultiConfig() { return config_type == configType::MultiConfig; }
+ virtual bool IsSingleConfig() { return config_type == configType::SingleConfig; }
public:
int line_number = 0;
bool readonly = true;
validate_fn validate = nullptr;
callback_fn callback = nullptr;
+ configType config_type = configType::SingleConfig;
};
class StringField : public ConfigField {
@@ -69,6 +76,29 @@ class StringField : public ConfigField {
std::string *receiver_;
};
+class MultiStringField : public ConfigField {
+ public:
+ MultiStringField(std::vector<std::string> *receiver, std::vector<std::string> input) : receiver_(receiver) {
+ *receiver_ = std::move(input);
+ this->config_type = configType::MultiConfig;
+ }
+ ~MultiStringField() override = default;
+ std::string ToString() override {
+ std::string tmp;
+ for (auto &p : *receiver_) {
+ tmp += p + "\n";
+ }
+ return tmp;
+ }
+ Status Set(const std::string &v) override {
+ receiver_->emplace_back(v);
+ return Status::OK();
+ }
+
+ private:
+ std::vector<std::string> *receiver_;
+};
+
class IntField : public ConfigField {
public:
IntField(int *receiver, int n, int min, int max) : receiver_(receiver), min_(min), max_(max) { *receiver_ = n; }
diff --git a/tests/cppunit/config_test.cc b/tests/cppunit/config_test.cc
index 4d3812f9..df3cf812 100644
--- a/tests/cppunit/config_test.cc
+++ b/tests/cppunit/config_test.cc
@@ -133,19 +133,42 @@ TEST(Config, GetAndSet) {
}
}
+TEST(Config, GetRenameCommand) {
+ const char *path = "test.conf";
+ unlink(path);
+
+ std::ofstream output_file(path, std::ios::out);
+ output_file << "rename-command KEYS KEYS_NEW"
+ << "\n";
+ output_file << "rename-command GET GET_NEW"
+ << "\n";
+ output_file << "rename-command SET SET_NEW"
+ << "\n";
+ output_file.close();
+ Config config;
+ Redis::PopulateCommands();
+ ASSERT_TRUE(config.Load(CLIOptions(path)).IsOK());
+ std::vector<std::string> values;
+ config.Get("rename-command", &values);
+ ASSERT_EQ(values[1], "KEYS KEYS_NEW");
+ ASSERT_EQ(values[3], "GET GET_NEW");
+ ASSERT_EQ(values[5], "SET SET_NEW");
+ ASSERT_EQ(values[0], "rename-command");
+ ASSERT_EQ(values[2], "rename-command");
+ ASSERT_EQ(values[4], "rename-command");
+}
+
TEST(Config, Rewrite) {
const char *path = "test.conf";
unlink(path);
- std::ostringstream string_stream;
- string_stream << "rename-command KEYS KEYS_NEW"
- << "\n";
- string_stream << "rename-command GET GET_NEW"
- << "\n";
- string_stream << "rename-command SET SET_NEW"
- << "\n";
std::ofstream output_file(path, std::ios::out);
- output_file.write(string_stream.str().c_str(), string_stream.str().size());
+ output_file << "rename-command KEYS KEYS_NEW"
+ << "\n";
+ output_file << "rename-command GET GET_NEW"
+ << "\n";
+ output_file << "rename-command SET SET_NEW"
+ << "\n";
output_file.close();
Config config;
@@ -153,8 +176,9 @@ TEST(Config, Rewrite) {
ASSERT_TRUE(config.Load(CLIOptions(path)).IsOK());
ASSERT_TRUE(config.Rewrite().IsOK());
// Need to re-populate the command table since it has renamed by the previous
+ Config new_config;
Redis::PopulateCommands();
- ASSERT_TRUE(config.Load(CLIOptions(path)).IsOK());
+ ASSERT_TRUE(new_config.Load(CLIOptions(path)).IsOK());
unlink(path);
}
diff --git a/tests/gocase/unit/config/config_test.go b/tests/gocase/unit/config/config_test.go
index 8d337464..e44e5b1b 100644
--- a/tests/gocase/unit/config/config_test.go
+++ b/tests/gocase/unit/config/config_test.go
@@ -23,6 +23,7 @@ import (
"context"
"log"
"os"
+ "sort"
"testing"
"time"
@@ -34,17 +35,27 @@ import (
func TestRenameCommand(t *testing.T) {
srv := util.StartServer(t, map[string]string{
- "rename-command": "KEYS KEYSNEW",
+ "rename-command KEYS": "KEYSNEW",
+ "rename-command GET": "GETNEW",
+ "rename-command SET": "SETNEW",
})
defer srv.Close()
ctx := context.Background()
rdb := srv.NewClient()
defer func() { require.NoError(t, rdb.Close()) }()
- err := rdb.Keys(ctx, "*").Err()
- require.ErrorContains(t, err, "unknown command")
- r := rdb.Do(ctx, "KEYSNEW", "*")
- require.Equal(t, []interface{}{}, r.Val())
+ require.ErrorContains(t, rdb.Keys(ctx, "*").Err(), "unknown command")
+ require.ErrorContains(t, rdb.Get(ctx, "key").Err(), "unknown command")
+ require.ErrorContains(t, rdb.Set(ctx, "key", "1", 0).Err(), "unknown command")
+ require.Equal(t, []interface{}{}, rdb.Do(ctx, "KEYSNEW", "*").Val())
+ require.NoError(t, rdb.Do(ctx, "SETNEW", "key", "1").Err())
+ require.Equal(t, "1", rdb.Do(ctx, "GETNEW", "key").Val())
+ val := []string{}
+ for _, v := range rdb.Do(ctx, "config", "get", "rename-command").Val().([]interface{}) {
+ val = append(val, v.(string))
+ }
+ sort.Strings(val)
+ require.EqualValues(t, []string{"GET GETNEW", "KEYS KEYSNEW", "SET SETNEW", "rename-command", "rename-command", "rename-command"}, val)
}
func TestSetConfigBackupDir(t *testing.T) {