feat: add infoCmd section:server stats cpu commandstats

[hahahashen]

添加infoCmd的部分选项

[panlei-coder]

cmd_count_
cmd_time_consuming_

[hahahashen]

done

[panlei-coder]

对TimeStat的相关操作封装一下，time_stat_设置成私有的

[hahahashen]

done

[panlei-coder]

这里有的大写有的小写 统一成大写吧

[panlei-coder]

类似于这样，放到info类里面

[hahahashen]

这里有的大写有的小写 统一成大写吧

redis 官方info cmd的section是小写，我觉得得小写，info RAFT section只是暂用，不知道braft哪里会用到，所以我觉得要么就都先暂留着？（// @todo The info raft command is only supported for the time being）

[panlei-coder]

这个方便增加 go_test 么

[Issues-translate-bot]

Bot detected the issue body's language is not English, translate it automatically.

---

Is this convenient to add go_test?

[hahahashen]

这个方便增加 go_test 么

这个方便增加 go_test 么

我觉得不是很好添加，因为info只是用于显示命令输出

[Issues-translate-bot]

Bot detected the issue body's language is not English, translate it automatically.

---

Is this convenient to add go_test?

Is this convenient to add go_test?

I don't think it's very good to add, because info is only used to display command output

[Issues-translate-bot]

Bot detected the issue body's language is not English, translate it automatically.

---

I have been very busy in the past three weeks. This PR has been modified according to the comments, but it cannot be compiled locally. I will take a closer look when I am finished.

[AlexStocks]

@CodiumAI-Agent /improve

[CodiumAI-Agent]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Prevent potential null pointer dereference by checking if time_stat_ is initialized Ensure that the time_stat_ object is initialized before using it in PClient::handlePacket. This prevents potential dereferencing of a null pointer which can lead to segmentation faults. src/client.cc [354] -time_stat_->enqueue_ts_ = pstd::NowMicros(); +if (time_stat_) { + time_stat_->enqueue_ts_ = pstd::NowMicros(); +} Suggestion importance[1-10]: 9 Why: This suggestion addresses a potential null pointer dereference, which is a critical issue that can lead to segmentation faults. Ensuring time_stat_ is initialized before use is a necessary safety check.	9
Possible issue	Improve safety by deleting the custom copy constructor in CommandStatistics The CommandStatistics copy constructor uses atomic operations which are not exception-safe. Consider using the implicitly generated copy constructor or explicitly deleting it to avoid unsafe copy operations. src/client.h [25-27] -CommandStatistics(const CommandStatistics& other) { - cmd_time_consuming.store(other.cmd_time_consuming.load()); - cmd_count.store(other.cmd_count.load()); -} +CommandStatistics(const CommandStatistics& other) = delete; Suggestion importance[1-10]: 9 Why: The suggestion addresses a potential safety issue with atomic operations in the copy constructor, which is crucial for preventing unsafe copy operations. Deleting the copy constructor is a valid approach to ensure exception safety.	9
Enhancement	Add error handling for map insertion to ensure data integrity Add error handling for the emplace operation to ensure that the command statistics are correctly added to the map. src/cmd_thread_pool_worker.cc [44] -cmdstat_map->emplace(task->CmdName(), statistics); +auto result = cmdstat_map->emplace(task->CmdName(), statistics); +if (!result.second) { + // Handle the error, for example, log it or throw an exception +} Suggestion importance[1-10]: 8 Why: Adding error handling for the emplace operation enhances the robustness of the code by ensuring that the insertion into the map is successful. This is important for maintaining data integrity and preventing potential runtime errors.	8
	Encapsulate raw timestamp types within TimeStat using std::chrono::time_point for safer type handling The TimeStat struct uses raw types for timestamps which can lead to unsafe direct manipulations. Consider encapsulating these timestamps within a class or using a safer type like std::chrono::time_point. src/client.h [42-44] -uint64_t enqueue_ts_; -uint64_t dequeue_ts_; -uint64_t process_done_ts_; +std::chrono::time_point<std::chrono::steady_clock> enqueue_ts_; +std::chrono::time_point<std::chrono::steady_clock> dequeue_ts_; +std::chrono::time_point<std::chrono::steady_clock> process_done_ts_; Suggestion importance[1-10]: 7 Why: This suggestion improves type safety and readability by using std::chrono::time_point instead of raw uint64_t for timestamps. While beneficial, it is not critical and involves a significant change in how timestamps are managed.	7
Best practice	Declare and define static string constants outside the InfoCmd class to prevent multiple definitions The InfoCmd class uses multiple static string constants for section names. These should be declared and defined outside the class to avoid multiple definitions across translation units. src/cmd_admin.h [146-153] -const static std::string kInfoSection; -const static std::string kAllSection; -const static std::string kServerSection; -const static std::string kStatsSection; -const static std::string kCPUSection; -const static std::string kDataSection; -const static std::string kCommandStatsSection; -const static std::string kRaftSection; +static const std::string kInfoSection = "info"; +static const std::string kAllSection = "all"; +static const std::string kServerSection = "server"; +static const std::string kStatsSection = "stats"; +static const std::string kCPUSection = "cpu"; +static const std::string kDataSection = "data"; +static const std::string kCommandStatsSection = "commandstats"; +static const std::string kRaftSection = "raft"; Suggestion importance[1-10]: 8 Why: This suggestion follows best practices for defining static constants, which can prevent potential issues with multiple definitions across translation units. It enhances code maintainability and clarity.	8
Maintainability	Simplify the InfoCmd::DoInitial method by using a helper function or map for command section handling Refactor the InfoCmd::DoInitial method to reduce complexity by splitting the large conditional block into a separate function or using a map for command section handlers. src/cmd_admin.cc [170-182] -if (strcasecmp(argv_[1].data(), kAllSection.data()) == 0) { - info_section_ = kInfoAll; -} else if (strcasecmp(argv_[1].data(), kServerSection.data()) == 0) { - info_section_ = kInfoServer; -} else if (strcasecmp(argv_[1].data(), kStatsSection.data()) == 0) { - info_section_ = kInfoStats; -} else if (strcasecmp(argv_[1].data(), kCPUSection.data()) == 0) { - info_section_ = kInfoCPU; -} else if (strcasecmp(argv_[1].data(), kDataSection.data()) == 0) { - info_section_ = kInfoData; -} else if (strcasecmp(argv_[1].data(), kRaftSection.data()) == 0) { - info_section_ = kInfoRaft; -} else if (strcasecmp(argv_[1].data(), kCommandStatsSection.data()) == 0) { - info_section_ = kInfoCommandStats; -} +info_section_ = GetInfoSectionFromString(argv_[1].data()); Suggestion importance[1-10]: 7 Why: Refactoring the method to reduce complexity improves code readability and maintainability. However, this suggestion is more about code style and maintainability rather than fixing a critical issue.	7
Performance	Optimize string concatenation in InfoCmd::InfoServer for better performance and readability Use a more efficient method for string concatenation in InfoCmd::InfoServer by using std::ostringstream instead of multiple std::stringstream and std::string operations. src/cmd_admin.cc [310-322] -std::stringstream tmp_stream; -tmp_stream << "# Server\r\n"; -tmp_stream << "PikiwiDB_version:" << version << "\r\n"; -tmp_stream << "PikiwiDB_build_git_sha:" << KPIKIWIDB_GIT_COMMIT_ID << "\r\n"; -tmp_stream << "Pikiwidb_build_compile_date: " << KPIKIWIDB_BUILD_DATE << "\r\n"; -tmp_stream << "os:" << host_info.sysname << " " << host_info.release << " " << host_info.machine << "\r\n"; -tmp_stream << "arch_bits:" << (reinterpret_cast<char*>(&host_info.machine) + strlen(host_info.machine) - 2) << "\r\n"; -tmp_stream << "process_id:" << getpid() << "\r\n"; -tmp_stream << "tcp_port:" << g_config.port << "\r\n"; -tmp_stream << "uptime_in_seconds:" << (current_time_s - g_pikiwidb->Start_time_s()) << "\r\n"; -tmp_stream << "uptime_in_days:" << (current_time_s / (24 * 3600) - g_pikiwidb->Start_time_s() / (24 * 3600) + 1) << "\r\n"; -tmp_stream << "config_file:" << g_pikiwidb->GetConfigName() << "\r\n"; +std::ostringstream info_stream; +info_stream << "# Server\r\n" + << "PikiwiDB_version:" << version << "\r\n" + << "PikiwiDB_build_git_sha:" << KPIKIWIDB_GIT_COMMIT_ID << "\r\n" + << "Pikiwidb_build_compile_date: " << KPIKIWIDB_BUILD_DATE << "\r\n" + << "os:" << host_info.sysname << " " << host_info.release << " " << host_info.machine << "\r\n" + << "arch_bits:" << (reinterpret_cast<char*>(&host_info.machine) + strlen(host_info.machine) - 2) << "\r\n" + << "process_id:" << getpid() << "\r\n" + << "tcp_port:" << g_config.port << "\r\n" + << "uptime_in_seconds:" << (current_time_s - g_pikiwidb->Start_time_s()) << "\r\n" + << "uptime_in_days:" << (current_time_s / (24 * 3600) - g_pikiwidb->Start_time_s() / (24 * 3600) + 1) << "\r\n" + << "config_file:" << g_pikiwidb->GetConfigName() << "\r\n"; Suggestion importance[1-10]: 6 Why: Using std::ostringstream for string concatenation can improve performance and readability. This is a minor optimization and does not address any critical issues, hence a lower score.	6

[AlexStocks]

@CodiumAI-Agent /review

[CodiumAI-Agent]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	4, due to the extensive changes across multiple files, the introduction of new functionalities, and the complexity of the modifications in command handling and statistics management. The PR affects core components such as command processing and server information reporting, requiring careful review to ensure correctness and performance.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The InfoCmd::DoCmd method has a default case in the switch statement that does not handle unknown sections, potentially leading to unhandled cases or incorrect server responses.
	Performance Concern: The InfoCmd::InfoCommandStats method iterates over all command statistics and performs calculations, which could be costly if there are many commands. Consider optimizing this by caching results or reducing frequency of updates.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/cmd_thread_pool_worker.cc
suggestion	Consider initializing the CommandStatistics structure directly in the map to avoid potential issues with missing entries. This can be done using the std::unordered_map::emplace method with piecewise construction to construct the CommandStatistics in place if it does not already exist. [important]
relevant line	if (cmdstat_map->find(task->CmdName()) == cmdstat_map->end()) {

---

relevant file	src/cmd_admin.cc
suggestion	To improve the maintainability and readability of the InfoCmd::DoCmd method, consider refactoring the large switch-case block into separate functions for each case. This will make the code easier to understand and modify in the future. [important]
relevant line	switch (info_section_) {

---

relevant file	src/client.h
suggestion	For the TimeStat struct, consider using more descriptive names for the timestamp variables to clarify their purposes, such as enqueue_time_, dequeue_time_, and process_done_time_. This will improve code readability and maintainability. [medium]
relevant line	uint64_t enqueue_ts_;

---

relevant file	src/cmd_admin.cc
suggestion	Optimize the InfoCmd::InfoCPU method by caching the results of getrusage calls if the data does not need to be updated in real-time. This can reduce system calls and improve performance, especially under heavy load. [medium]
relevant line	getrusage(RUSAGE_SELF, &self_ru);

---