diff --git a/src/tendisplus/commands/command_test.cpp b/src/tendisplus/commands/command_test.cpp index 962dbc20..0e8c4420 100644 --- a/src/tendisplus/commands/command_test.cpp +++ b/src/tendisplus/commands/command_test.cpp @@ -991,96 +991,109 @@ void testGlobStylePattern(std::shared_ptr svr) { EXPECT_TRUE(!expect.ok()); } +// Get content of config file +std::string getConfFileContent(const std::string& confile) { + std::ifstream file(confile); + if (!file.is_open()) { + EXPECT_TRUE(0); + return ""; + } + std::stringstream buffer; + buffer << file.rdbuf(); + file.close(); + return buffer.str(); +} + +// Test config rewrite covers all scenarios void testConfigRewrite(std::shared_ptr svr) { asio::io_context ioContext; asio::ip::tcp::socket socket(ioContext), socket1(ioContext); NetSession sess(svr, std::move(socket), 1, false, nullptr, nullptr); - - sess.setArgs({"config", "set", "maxbinlogkeepnum", "1500000"}); + // Test Case 1: Add new rocks option (rocks.min_blob_size) + // Option not in config file, should be appended after rewrite + sess.setArgs({"config", "set", "rocks.min_blob_size", "2048"}); auto expect = Command::runSessionCmd(&sess); EXPECT_TRUE(expect.ok()); - - sess.setArgs({"config", "rewrite"}); + // Test Case 2: Modify existing rocks option + // rocks.min_blob_size already exists, should update in place + sess.setArgs({"config", "set", "rocks.min_blob_size", "4096"}); + expect = Command::runSessionCmd(&sess); + EXPECT_TRUE(expect.ok()); + // Test Case 3: Modify original config option (rocks.disable_wal) + // Option exists in original config, should update in place + sess.setArgs({"config", "set", "rocks.disable_wal", "1"}); + expect = Command::runSessionCmd(&sess); + EXPECT_TRUE(expect.ok()); + // Test Case 4: Add new rocks CF option (rocks.defaultcf.xxx) + sess.setArgs({"config", "set", "rocks.defaultcf.enable_blob_files", "1"}); + expect = Command::runSessionCmd(&sess); + EXPECT_TRUE(expect.ok()); + // Test Case 5: Modify existing rocks CF option + sess.setArgs({"config", "set", "rocks.binlogcf.enable_blob_files", "0"}); + expect = Command::runSessionCmd(&sess); + EXPECT_TRUE(expect.ok()); + // Test Case 6: Add another CF option (rocks.binlogcf.xxx) + // Multiple CF options should coexist + sess.setArgs({"config", + "set", + "rocks.binlogcf.blob_garbage_collection_age_cutoff", + "0.25"}); + expect = Command::runSessionCmd(&sess); + EXPECT_TRUE(expect.ok()); + // Test Case 7: Add multiple options for same CF + sess.setArgs( + {"config", "set", "rocks.defaultcf.blob_file_size", "268435456"}); + expect = Command::runSessionCmd(&sess); + EXPECT_TRUE(expect.ok()); + // Test Case 8: Modify blob garbage collection age cutoff + sess.setArgs( + {"config", "set", "rocks.blob_garbage_collection_age_cutoff", "0.36"}); + expect = Command::runSessionCmd(&sess); + EXPECT_TRUE(expect.ok()); + // Test Case 9: Modify defaultcf blob compression type + sess.setArgs( + {"config", "set", "rocks.defaultcf.blob_compression_type", "snappy"}); expect = Command::runSessionCmd(&sess); EXPECT_TRUE(expect.ok()); - - auto confile = svr->getParams()->getConfFile(); - std::ifstream file(confile); - if (!file.is_open()) { - EXPECT_TRUE(0); - } - std::string line; - std::string text; - std::vector tokens; - - bool find = false; - try { - line.clear(); - while (std::getline(file, line)) { - line = trim(line); - if (line.size() == 0 || line[0] == '#') { - continue; - } - std::stringstream ss(line); - tokens.clear(); - std::string tmp; - while (std::getline(ss, tmp, ' ')) { - tokens.emplace_back(tmp); - } - if (tokens.size() == 2) { - if (tokens[0] == "maxbinlogkeepnum" && tokens[1] == "1500000") { - find = true; - } - } - } - } catch (const std::exception& ex) { - EXPECT_TRUE(0); - return; - } - EXPECT_TRUE(find); - file.close(); - - std::ofstream out; - out.open(confile); - out.flush(); - out << text; - out.close(); sess.setArgs({"config", "rewrite"}); expect = Command::runSessionCmd(&sess); EXPECT_TRUE(expect.ok()); - bool correct = false; - std::ifstream check(confile); - if (!check.is_open()) { - EXPECT_TRUE(0); - } - try { - line.clear(); - while (std::getline(check, line)) { - line = trim(line); - if (line.size() == 0 || line[0] == '#') { - continue; - } - std::stringstream ss(line); - tokens.clear(); - std::string tmp; - while (std::getline(ss, tmp, ' ')) { - tokens.emplace_back(tmp); - } - if (tokens.size() == 2) { - if (tokens[0] == "maxbinlogkeepnum" && tokens[1] == "1500000") { - correct = true; - break; - } - } - } - } catch (const std::exception& ex) { - EXPECT_TRUE(0); - return; - } - EXPECT_TRUE(correct); - check.close(); + std::string expectedContent = + "bind 127.0.0.1\n" + "port 8811\n" + "loglevel debug\n" + "logdir ./log\n" + "dir ./db\n" + "dumpdir ./dump\n" + "pidfile ./tendisplus.pid\n" + "slowlog ./log/slowlog\n" + "storage rocks\n" + "rocks.blockcachemb 4096\n" + "rocks.write_buffer_size 4096000\n" + "generallog yes\n" + "checkkeytypeforsetcmd yes\n" + "kvStoreCount 1\n" + "maxbinlogkeepnum 1000000\n" + "slavebinlogkeepnum 1000000\n" + "executorthreadnum 8\n" + "rocks.binlogcf.enable_blob_files 0\n" + "rocks.blob_garbage_collection_age_cutoff 0.36\n" + "rocks.defaultcf.blob_compression_type snappy\n" + "rocks.enable_blob_files 1\n" + "# Generated by CONFIG REWRITE\n" + "fullpushthreadnum 1\n" + "fullreceivethreadnum 1\n" + "incrpushthreadnum 1\n" + "logrecyclethreadnum 1\n" + "rocks.disable_wal yes\n" + "rocks.min_blob_size 4096\n" + "rocks.defaultcf.blob_file_size 268435456\n" + "rocks.defaultcf.enable_blob_files 1\n" + "rocks.binlogcf.blob_garbage_collection_age_cutoff 0.25\n"; + auto content = getConfFileContent(svr->getParams()->getConfFile()); + EXPECT_EQ(content, expectedContent); } void testCommand(std::shared_ptr svr) { @@ -1340,16 +1353,22 @@ TEST(Command, testGlobStylePattern) { TEST(Command, testConfigRewrite) { const auto guard = MakeGuard([] { destroyEnv(); }); - EXPECT_TRUE(setupEnv()); - auto cfg = makeServerParam(); - auto server = makeServerEntry(cfg); + std::map configMap = { + {"executorthreadnum", "8"}, + {"rocks.enable_blob_files", "1"}, + {"rocks.binlogcf.enable_blob_files", "1"}, + {"rocks.blob_garbage_collection_age_cutoff", "0.12"}, + {"rocks.defaultcf.blob_compression_type", "lz4"}}; + auto cfg = makeServerParam(8811, 1, "", true, configMap); + auto server = makeServerEntry(cfg); + getGlobalServer() = server; testConfigRewrite(server); - + getGlobalServer() = nullptr; + // Clean up temp config file remove(cfg->getConfFile().c_str()); - #ifndef _WIN32 server->stop(); EXPECT_EQ(server.use_count(), 1); diff --git a/src/tendisplus/commands/version.h b/src/tendisplus/commands/version.h index 14a68808..396373cf 100644 --- a/src/tendisplus/commands/version.h +++ b/src/tendisplus/commands/version.h @@ -6,7 +6,7 @@ #define SRC_TENDISPLUS_COMMANDS_VERSION_H_ #include -#define TENDISPLUS_VERSION_PRE "2.8.2-rocksdb-v" +#define TENDISPLUS_VERSION_PRE "2.8.3-rocksdb-v" std::string getTendisPlusVersion(); diff --git a/src/tendisplus/server/server_params.cpp b/src/tendisplus/server/server_params.cpp index b6b12544..13e3faf8 100644 --- a/src/tendisplus/server/server_params.cpp +++ b/src/tendisplus/server/server_params.cpp @@ -931,9 +931,9 @@ Status ServerParams::rewriteConfig() const { return s; } - for (auto it = _mapServerParams.begin(); it != _mapServerParams.end(); it++) { - std::string name = it->first; - BaseVar* var = it->second; + for (const auto& param : _mapServerParams) { + std::string name = param.first; + BaseVar* var = param.second; if (!var->isallowDynamicSet()) continue; @@ -941,6 +941,22 @@ Status ServerParams::rewriteConfig() const { rw->rewriteConfigOption(name, var->show(), var->default_show()); } + for (const auto& option : _rocksdbOptions) { + std::string name = "rocks." + option.first; + auto var = option.second; + + rw->rewriteConfigOption(name, var, ""); + } + + for (const auto& it : _rocksdbCFOptions) { + auto cfName = it.first; + auto cfOptions = it.second; + for (const auto& option : cfOptions) { + std::string name = "rocks." + cfName + "." + option.first; + rw->rewriteConfigOption(name, option.second, ""); + } + } + rw->rewriteConfigRemoveOrphaned(); std::string content = rw->rewriteConfigGetContentFromState(); diff --git a/src/tendisplus/utils/test_util.cpp b/src/tendisplus/utils/test_util.cpp index e49e53e2..fcd60f72 100644 --- a/src/tendisplus/utils/test_util.cpp +++ b/src/tendisplus/utils/test_util.cpp @@ -36,8 +36,8 @@ std::shared_ptr makeServerParam( ss_tmp_conf_file << port << "_test.cfg"; std::string tmp_conf_file = ss_tmp_conf_file.str(); - const auto guard = - MakeGuard([tmp_conf_file] { remove(tmp_conf_file.c_str()); }); + // remove file if exist + remove(tmp_conf_file.c_str()); std::ofstream myfile; myfile.open(tmp_conf_file);