Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the support of ANALYZE command to inspect the performance of RocksDB #1943

Merged
merged 2 commits into from
Jan 2, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 70 additions & 1 deletion src/commands/cmd_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
*
*/

#include <rocksdb/iostats_context.h>
#include <rocksdb/perf_context.h>

#include "command_parser.h"
#include "commander.h"
#include "commands/scan_base.h"
Expand Down Expand Up @@ -1123,6 +1126,71 @@ class CommandRdb : public Commander {
uint32_t db_index_ = 0;
};

class CommandAnalyze : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
if (args.size() <= 1) return {Status::RedisExecErr, errInvalidSyntax};
for (int i = 1; i < args.size(); ++i) {
command_args_.push_back(args[i]);
}
Comment on lines +1133 to +1135
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we can directly insert rather than push back

return Status::OK();
}
Status Execute(Server *srv, Connection *conn, std::string *output) override {
auto commands = redis::CommandTable::Get();
auto cmd_iter = commands->find(util::ToLower(command_args_[0]));
if (cmd_iter == commands->end()) {
// unsupported redis command
return {Status::RedisExecErr, errInvalidSyntax};
}
auto redis_cmd = cmd_iter->second;
auto cmd = redis_cmd->factory();
cmd->SetAttributes(redis_cmd);
cmd->SetArgs(command_args_);

int arity = cmd->GetAttributes()->arity;
if ((arity > 0 && command_args_.size() != arity) || (arity < 0 && command_args_.size() < -arity)) {
*output = redis::Error("ERR wrong number of arguments");
return {Status::RedisExecErr, errWrongNumOfArguments};
}

auto s = cmd->Parse(command_args_);
Comment on lines +1155 to +1156
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check the arity before Parse? Since wrong command_args_ might make our program crash

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to reuse the code in ExecuteCommand rather than write them again here.

Copy link
Contributor Author

@chrisxu333 chrisxu333 Dec 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pushed a newer version which added the arity checking logic. As for reusing / refactoring the code in ExecuteCommand, maybe it's better to do it in a separate pr? As the ExecuteCommand code needs to be dissected into several chunks for reusing purpose. @mapleFU @PragmaTwice

if (!s.IsOK()) {
return s;
}

auto prev_perf_level = rocksdb::GetPerfLevel();
rocksdb::SetPerfLevel(rocksdb::PerfLevel::kEnableTimeExceptForMutex);
rocksdb::get_perf_context()->Reset();
rocksdb::get_iostats_context()->Reset();

std::string command_output;
s = cmd->Execute(srv, conn, &command_output);
if (!s.IsOK()) {
return s;
}

if (command_output[0] == '-') {
*output = command_output;
return s;
}

std::string perf_context = rocksdb::get_perf_context()->ToString(true);
std::string iostats_context = rocksdb::get_iostats_context()->ToString(true);
rocksdb::get_perf_context()->Reset();
rocksdb::get_iostats_context()->Reset();
rocksdb::SetPerfLevel(prev_perf_level);

*output = redis::MultiLen(3); // command output + perf context + iostats context
*output += command_output;
*output += redis::BulkString(perf_context);
*output += redis::BulkString(iostats_context);
return Status::OK();
}

private:
std::vector<std::string> command_args_;
};

REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandAuth>("auth", 2, "read-only ok-loading", 0, 0, 0),
MakeCmdAttr<CommandPing>("ping", -1, "read-only", 0, 0, 0),
MakeCmdAttr<CommandSelect>("select", 2, "read-only", 0, 0, 0),
Expand Down Expand Up @@ -1157,6 +1225,7 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandAuth>("auth", 2, "read-only ok-loadin
MakeCmdAttr<CommandFlushBackup>("flushbackup", 1, "read-only no-script", 0, 0, 0),
MakeCmdAttr<CommandSlaveOf>("slaveof", 3, "read-only exclusive no-script", 0, 0, 0),
MakeCmdAttr<CommandStats>("stats", 1, "read-only", 0, 0, 0),
MakeCmdAttr<CommandRdb>("rdb", -3, "write exclusive", 0, 0, 0), )
MakeCmdAttr<CommandRdb>("rdb", -3, "write exclusive", 0, 0, 0),
MakeCmdAttr<CommandAnalyze>("analyze", -1, "", 0, 0, 0), )
chrisxu333 marked this conversation as resolved.
Show resolved Hide resolved

} // namespace redis