Skip to content

Commit

Permalink
fix: incorrect initialization sequence about pika role and cmdstat_map(
Browse files Browse the repository at this point in the history
  • Loading branch information
liuchengyu committed Jan 6, 2024
1 parent a51405d commit 90c58bf
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 75 deletions.
10 changes: 10 additions & 0 deletions include/pika_cmd_table_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ class PikaCmdTableManager {
bool CmdExist(const std::string& cmd) const;
CmdTable* GetCmdTable();

/*
* Info Commandstats used
*/
std::unordered_map<std::string, CommandStatistics>* GetCommandStatMap();

private:
std::shared_ptr<Cmd> NewCommand(const std::string& opt);

Expand All @@ -41,5 +46,10 @@ class PikaCmdTableManager {

std::shared_mutex map_protector_;
std::unordered_map<std::thread::id, std::unique_ptr<PikaDataDistribution>> thread_distribution_map_;

/*
* Info Commandstats used
*/
std::unordered_map<std::string, CommandStatistics> cmdstat_map_;
};
#endif
10 changes: 0 additions & 10 deletions include/pika_conf.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,6 @@ class PikaConf : public pstd::BaseConf {
std::shared_lock l(rwlock_);
return run_id_;
}
std::string master_run_id() {
std::shared_lock l(rwlock_);
return master_run_id_;
}
std::string replication_id() {
std::shared_lock l(rwlock_);
return replication_id_;
Expand Down Expand Up @@ -403,11 +399,6 @@ class PikaConf : public pstd::BaseConf {
TryPushDiffCommands("slaveof", value);
slaveof_ = value;
}
void SetMasterRunID(const std::string& value) {
std::lock_guard l(rwlock_);
TryPushDiffCommands("master-run-id", value);
master_run_id_ = value;
}
void SetReplicationID(const std::string& value) {
std::lock_guard l(rwlock_);
TryPushDiffCommands("replication-id", value);
Expand Down Expand Up @@ -672,7 +663,6 @@ class PikaConf : public pstd::BaseConf {
int timeout_ = 0;
std::string server_id_;
std::string run_id_;
std::string master_run_id_;
std::string replication_id_;
std::string requirepass_;
std::string masterauth_;
Expand Down
13 changes: 0 additions & 13 deletions include/pika_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,6 @@ class PikaServer : public pstd::noncopyable {
int port();
time_t start_time_s();
std::string master_ip();
std::string master_run_id();
void set_master_run_id(const std::string& master_run_id);
int master_port();
int role();
bool leader_protected_mode();
Expand Down Expand Up @@ -518,11 +516,6 @@ class PikaServer : public pstd::noncopyable {
*/
storage::Status RewriteStorageOptions(const storage::OptionType& option_type,
const std::unordered_map<std::string, std::string>& options);
/*
* Info Commandstats used
*/
std::unordered_map<std::string, CommandStatistics>* GetCommandStatMap();


/*
* Instantaneous Metric used
Expand Down Expand Up @@ -627,7 +620,6 @@ class PikaServer : public pstd::noncopyable {
*/
std::string master_ip_;
int master_port_ = 0;
std::string master_run_id_;
int repl_state_ = PIKA_REPL_NO_CONNECT;
int role_ = PIKA_ROLE_SINGLE;
int last_meta_sync_timestamp_ = 0;
Expand Down Expand Up @@ -698,11 +690,6 @@ class PikaServer : public pstd::noncopyable {
*/
Statistic statistic_;

/*
* Info Commandstats used
*/
std::unordered_map<std::string, CommandStatistics> cmdstat_map_;

net::BGThread common_bg_thread_;

/*
Expand Down
11 changes: 3 additions & 8 deletions src/pika_admin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ using pstd::Status;

extern PikaServer* g_pika_server;
extern std::unique_ptr<PikaReplicaManager> g_pika_rm;
extern std::unique_ptr<PikaCmdTableManager> g_pika_cmd_table_manager;

static std::string ConstructPinginPubSubResp(const PikaCmdArgsType& argv) {
if (argv.size() > 2) {
Expand Down Expand Up @@ -160,7 +161,6 @@ void SlaveofCmd::Do(std::shared_ptr<Slot> slot) {
res_.SetRes(CmdRes::kOk);
g_pika_server->ClearCacheDbAsync(slot);
g_pika_conf->SetSlaveof(master_ip_ + ":" + std::to_string(master_port_));
g_pika_conf->SetMasterRunID("");
g_pika_server->SetFirstMetaSync(true);
} else {
res_.SetRes(CmdRes::kErrOther, "Server is not in correct state for slaveof");
Expand Down Expand Up @@ -1393,7 +1393,8 @@ void InfoCmd::InfoCommandStats(std::string& info) {
tmp_stream.precision(2);
tmp_stream.setf(std::ios::fixed);
tmp_stream << "# Commandstats" << "\r\n";
for (auto iter : *g_pika_server->GetCommandStatMap()) {
auto cmdstat_map = g_pika_cmd_table_manager->GetCommandStatMap();
for (auto iter : *cmdstat_map) {
if (iter.second.cmd_count != 0) {
tmp_stream << iter.first << ":"
<< "calls=" << iter.second.cmd_count << ", usec="
Expand Down Expand Up @@ -1944,12 +1945,6 @@ void ConfigCmd::ConfigGet(std::string& ret) {
EncodeString(&config_body, g_pika_conf->run_id());
}

if (pstd::stringmatch(pattern.data(), "master-run-id", 1) != 0) {
elements += 2;
EncodeString(&config_body, "master-run-id");
EncodeString(&config_body, g_pika_conf->master_run_id());
}

if (pstd::stringmatch(pattern.data(), "blob-cache", 1) != 0) {
elements += 2;
EncodeString(&config_body, "blob-cache");
Expand Down
2 changes: 1 addition & 1 deletion src/pika_client_conn.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ std::shared_ptr<Cmd> PikaClientConn::DoCmd(const PikaCmdArgsType& argv, const st
// Process Command
c_ptr->Execute();
time_stat_->process_done_ts_ = pstd::NowMicros();
auto cmdstat_map = g_pika_server->GetCommandStatMap();
auto cmdstat_map = g_pika_cmd_table_manager->GetCommandStatMap();
(*cmdstat_map)[opt].cmd_count.fetch_add(1);
(*cmdstat_map)[opt].cmd_time_consuming.fetch_add(time_stat_->total_time());

Expand Down
9 changes: 9 additions & 0 deletions src/pika_cmd_table_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ PikaCmdTableManager::PikaCmdTableManager() {
cmds_ = std::make_unique<CmdTable>();
cmds_->reserve(300);
InitCmdTable(cmds_.get());

CommandStatistics statistics;
for (auto& iter : *cmds_) {
cmdstat_map_.emplace(iter.first, statistics);
}
}

std::unordered_map<std::string, CommandStatistics>* PikaCmdTableManager::GetCommandStatMap() {
return &cmdstat_map_;
}

std::shared_ptr<Cmd> PikaCmdTableManager::GetCmd(const std::string& opt) {
Expand Down
9 changes: 1 addition & 8 deletions src/pika_conf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -572,13 +572,7 @@ int PikaConf::Load() {
// slaveof
slaveof_ = "";
GetConfStr("slaveof", &slaveof_);
if (slaveof_ != "") {
std::string master_run_id;
GetConfStr("master-run-id", &master_run_id);
if (master_run_id.length() == configRunIDSize) {
master_run_id_ = master_run_id;
}
}

int cache_num = 16;
GetConfInt("cache-num", &cache_num);
cache_num_ = (0 >= cache_num || 48 < cache_num) ? 16 : cache_num;
Expand Down Expand Up @@ -732,7 +726,6 @@ int PikaConf::ConfigRewrite() {
SetConfInt("slowlog-max-len", slowlog_max_len_);
SetConfStr("write-binlog", write_binlog_ ? "yes" : "no");
SetConfStr("run-id", run_id_);
SetConfStr("master-run-id", master_run_id_);
SetConfStr("replication-id", replication_id_);
SetConfInt("max-cache-statistic-keys", max_cache_statistic_keys_);
SetConfInt("small-compaction-threshold", small_compaction_threshold_);
Expand Down
48 changes: 13 additions & 35 deletions src/pika_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,19 @@ PikaServer::PikaServer()
exit_mutex_.lock();
int64_t lastsave = GetLastSaveTime(g_pika_conf->bgsave_path());
UpdateLastSave(lastsave);

// init role
std::string slaveof = g_pika_conf->slaveof();
if (!slaveof.empty()) {
auto sep = static_cast<int32_t>(slaveof.find(':'));
std::string master_ip = slaveof.substr(0, sep);
int32_t master_port = std::stoi(slaveof.substr(sep + 1));
if ((master_ip == "127.0.0.1" || master_ip == host_) && master_port == port_) {
LOG(FATAL) << "you will slaveof yourself as the config file, please check";
} else {
SetMaster(master_ip, master_port);
}
}
}

PikaServer::~PikaServer() {
Expand Down Expand Up @@ -180,26 +193,6 @@ void PikaServer::Start() {
}

time(&start_time_s_);

std::string master_run_id = g_pika_conf->master_run_id();
set_master_run_id(master_run_id);
std::string slaveof = g_pika_conf->slaveof();
if (!slaveof.empty()) {
auto sep = static_cast<int32_t>(slaveof.find(':'));
std::string master_ip = slaveof.substr(0, sep);
int32_t master_port = std::stoi(slaveof.substr(sep + 1));
if ((master_ip == "127.0.0.1" || master_ip == host_) && master_port == port_) {
LOG(FATAL) << "you will slaveof yourself as the config file, please check";
} else {
g_pika_server->set_master_run_id(g_pika_conf->master_run_id());
SetMaster(master_ip, master_port);
}
}
CommandStatistics statistics;
auto cmdstat_map = g_pika_server->GetCommandStatMap();
for (auto& iter : *g_pika_cmd_table_manager->GetCmdTable()) {
cmdstat_map->emplace(iter.first, statistics);
}
LOG(INFO) << "Pika Server going to start";
rsync_server_->Start();
while (!exit_) {
Expand Down Expand Up @@ -233,16 +226,6 @@ int PikaServer::master_port() {
return master_port_;
}

std::string PikaServer::master_run_id() {
std::shared_lock l(state_protector_);
return master_run_id_;
}

void PikaServer::set_master_run_id(const std::string& master_run_id) {
std::lock_guard l(state_protector_);
master_run_id_ = master_run_id;
}

int PikaServer::role() {
std::shared_lock l(state_protector_);
return role_;
Expand Down Expand Up @@ -769,7 +752,6 @@ void PikaServer::RemoveMaster() {

master_ip_ = "";
master_port_ = -1;
master_run_id_ = "";
DoSameThingEverySlot(TaskType::kResetReplState);
}
}
Expand Down Expand Up @@ -1716,10 +1698,6 @@ void PikaServer::Bgslotsreload(const std::shared_ptr<Slot>& slot) {
bgsave_thread_.Schedule(&DoBgslotsreload, static_cast<void*>(this));
}

std::unordered_map<std::string, CommandStatistics>* PikaServer::GetCommandStatMap() {
return &cmdstat_map_;
}

void DoBgslotsreload(void* arg) {
auto p = static_cast<PikaServer*>(arg);
PikaServer::BGSlotsReload reload = p->bgslots_reload();
Expand Down

0 comments on commit 90c58bf

Please sign in to comment.