Skip to content

Commit

Permalink
Fix config rewrite overwrite the first rename-command with the last o…
Browse files Browse the repository at this point in the history
…ne (#429)

For example, we have below rename-commands in the config file:
```
rename-command KEYS KEYS_NEW
rename-command GET  GET_NEW
rename-command SET  SET_NEW
```
then `rename-command KEYS KEYS_NEW` would be overwriten to
`rename-command SET  SET_NEW` since we didn't skip the `rename-command`
correctly in config rewrite.
  • Loading branch information
git-hulk authored and ShooterIT committed Jan 28, 2022
1 parent a21c3f6 commit aa57b14
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,11 @@ 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,
// so skip it here to avoid rewriting it as new item.
continue;
}
new_config[iter.first] = iter.second->ToString();
}

Expand Down
24 changes: 24 additions & 0 deletions tests/config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#include "server.h"
#include <map>
#include <vector>
#include <fstream>
#include <iostream>
#include <gtest/gtest.h>

TEST(Config, GetAndSet) {
Expand Down Expand Up @@ -104,6 +106,28 @@ TEST(Config, GetAndSet) {
}
}

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.close();

Config config;
Redis::PopulateCommands();
ASSERT_TRUE(config.Load(path).IsOK());
ASSERT_TRUE(config.Rewrite().IsOK());
// Need to re-populate the command table since it has renamed by the previous
Redis::PopulateCommands();
ASSERT_TRUE(config.Load(path).IsOK());
unlink(path);
}

TEST(Namespace, Add) {
const char *path = "test.conf";
unlink(path);
Expand Down

0 comments on commit aa57b14

Please sign in to comment.