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/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new a9259a79 Fix the pidfile and backup_dir will be rewritten even though
they weren't modified (#2186)
a9259a79 is described below
commit a9259a79f5b57501959a072e1a5629fd524d9f10
Author: hulk <[email protected]>
AuthorDate: Thu Mar 21 12:24:23 2024 +0800
Fix the pidfile and backup_dir will be rewritten even though they weren't
modified (#2186)
---
src/cli/main.cc | 4 ++--
src/config/config.cc | 17 ++++++++++-------
src/config/config.h | 6 ++----
src/storage/storage.cc | 4 ++--
tests/cppunit/config_test.cc | 2 ++
5 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/src/cli/main.cc b/src/cli/main.cc
index 6e9b0b14..32c957a4 100644
--- a/src/cli/main.cc
+++ b/src/cli/main.cc
@@ -154,12 +154,12 @@ int main(int argc, char *argv[]) {
}
bool is_supervised = IsSupervisedMode(config.supervised_mode);
if (config.daemonize && !is_supervised) Daemonize();
- s = CreatePidFile(config.GetPidFile());
+ s = CreatePidFile(config.pidfile);
if (!s.IsOK()) {
LOG(ERROR) << "Failed to create pidfile: " << s.Msg();
return 1;
}
- auto pidfile_exit = MakeScopeExit([&config] {
RemovePidFile(config.GetPidFile()); });
+ auto pidfile_exit = MakeScopeExit([&config] { RemovePidFile(config.pidfile);
});
#ifdef ENABLE_OPENSSL
// initialize OpenSSL
diff --git a/src/config/config.cc b/src/config/config.cc
index 38de6595..41408e8b 100644
--- a/src/config/config.cc
+++ b/src/config/config.cc
@@ -42,6 +42,9 @@
#include "status.h"
#include "storage/redis_metadata.h"
+constexpr const char *kDefaultDir = "/tmp/kvrocks";
+constexpr const char *kDefaultBackupDir = "/tmp/kvrocks/backup";
+constexpr const char *kDefaultPidfile = "/tmp/kvrocks/kvrocks.pid";
constexpr const char *kDefaultBindAddress = "127.0.0.1";
constexpr const char *errBlobDbNotEnabled = "Must set
rocksdb.enable_blob_files to yes first.";
@@ -141,11 +144,11 @@ Config::Config() {
{"force-compact-file-min-deleted-percentage", false,
new IntField(&force_compact_file_min_deleted_percentage, 10, 1, 100)},
{"db-name", true, new StringField(&db_name, "change.me.db")},
- {"dir", true, new StringField(&dir, "/tmp/kvrocks")},
- {"backup-dir", false, new StringField(&backup_dir_, "")},
+ {"dir", true, new StringField(&dir, kDefaultDir)},
+ {"backup-dir", false, new StringField(&backup_dir, kDefaultBackupDir)},
{"log-dir", true, new StringField(&log_dir, "")},
{"log-level", false, new EnumField<int>(&log_level, log_levels,
google::INFO)},
- {"pidfile", true, new StringField(&pidfile_, "")},
+ {"pidfile", true, new StringField(&pidfile, kDefaultPidfile)},
{"max-io-mb", false, new IntField(&max_io_mb, 0, 0, INT_MAX)},
{"max-bitmap-to-string-mb", false, new
IntField(&max_bitmap_to_string_mb, 16, 0, INT_MAX)},
{"max-db-size", false, new IntField(&max_db_size, 0, 0, INT_MAX)},
@@ -403,6 +406,8 @@ void Config::initFieldCallback() {
checkpoint_dir = dir + "/checkpoint";
sync_checkpoint_dir = dir + "/sync_checkpoint";
backup_sync_dir = dir + "/backup_for_sync";
+ if (backup_dir == kDefaultBackupDir) backup_dir = dir + "/backup";
+ if (pidfile == kDefaultPidfile) pidfile = dir + "/kvrocks.pid";
return Status::OK();
}},
{"backup-dir",
@@ -412,8 +417,8 @@ void Config::initFieldCallback() {
// Note: currently, backup_mu_ may block by backing up or
purging,
// the command may wait for seconds.
std::lock_guard<std::mutex> lg(this->backup_mu);
- previous_backup = std::move(backup_dir_);
- backup_dir_ = v;
+ previous_backup = std::move(backup_dir);
+ backup_dir = v;
}
if (!previous_backup.empty() && srv != nullptr &&
!srv->IsLoading()) {
// LOG(INFO) should be called after log is initialized and
server is loaded.
@@ -778,9 +783,7 @@ Status Config::finish() {
if (master_port != 0 && binds.size() == 0) {
return {Status::NotOK, "replication doesn't support unix socket"};
}
- if (backup_dir_.empty()) backup_dir_ = dir + "/backup";
if (db_dir.empty()) db_dir = dir + "/db";
- if (pidfile_.empty()) pidfile_ = dir + "/kvrocks.pid";
if (log_dir.empty()) log_dir = dir;
std::vector<std::string> create_dirs = {dir};
for (const auto &name : create_dirs) {
diff --git a/src/config/config.h b/src/config/config.h
index 2ea2f553..fc62b517 100644
--- a/src/config/config.h
+++ b/src/config/config.h
@@ -122,6 +122,8 @@ struct Config {
std::vector<std::string> binds;
std::string dir;
std::string db_dir;
+ std::string backup_dir; // GUARD_BY(backup_mu_)
+ std::string pidfile;
std::string backup_sync_dir;
std::string checkpoint_dir;
std::string sync_checkpoint_dir;
@@ -237,13 +239,9 @@ struct Config {
void ClearMaster();
bool IsSlave() const { return !master_host.empty(); }
bool HasConfigFile() const { return !path_.empty(); }
- std::string GetBackupDir() const { return backup_dir_.empty() ? dir +
"/backup" : backup_dir_; }
- std::string GetPidFile() const { return pidfile_.empty() ? dir +
"/kvrocks.pid" : pidfile_; }
private:
std::string path_;
- std::string backup_dir_; // GUARD_BY(backup_mu_)
- std::string pidfile_;
std::string binds_str_;
std::string slaveof_;
std::string compact_cron_str_;
diff --git a/src/storage/storage.cc b/src/storage/storage.cc
index f42886b9..c21b8469 100644
--- a/src/storage/storage.cc
+++ b/src/storage/storage.cc
@@ -384,7 +384,7 @@ Status Storage::Open(DBOpenMode mode) {
Status Storage::CreateBackup(uint64_t *sequence_number) {
LOG(INFO) << "[storage] Start to create new backup";
std::lock_guard<std::mutex> lg(config_->backup_mu);
- std::string task_backup_dir = config_->GetBackupDir();
+ std::string task_backup_dir = config_->backup_dir;
std::string tmpdir = task_backup_dir + ".tmp";
// Maybe there is a dirty tmp checkpoint, try to clean it
@@ -535,7 +535,7 @@ void Storage::EmptyDB() {
void Storage::PurgeOldBackups(uint32_t num_backups_to_keep, uint32_t
backup_max_keep_hours) {
time_t now = util::GetTimeStamp();
std::lock_guard<std::mutex> lg(config_->backup_mu);
- std::string task_backup_dir = config_->GetBackupDir();
+ std::string task_backup_dir = config_->backup_dir;
// Return if there is no backup
auto s = env_->FileExists(task_backup_dir);
diff --git a/tests/cppunit/config_test.cc b/tests/cppunit/config_test.cc
index 1c28f0bf..cca20bf1 100644
--- a/tests/cppunit/config_test.cc
+++ b/tests/cppunit/config_test.cc
@@ -175,6 +175,8 @@ TEST(Config, Rewrite) {
redis::CommandTable::Reset();
Config config;
ASSERT_TRUE(config.Load(CLIOptions(path)).IsOK());
+ ASSERT_EQ(config.dir + "/backup", config.backup_dir);
+ ASSERT_EQ(config.dir + "/kvrocks.pid", config.pidfile);
ASSERT_TRUE(config.Rewrite({}).IsOK());
// Need to re-populate the command table since it has renamed by the previous
redis::CommandTable::Reset();
