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

chore: remove ToUpper calls in main_service #3947

Merged
merged 2 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
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
28 changes: 27 additions & 1 deletion src/server/command_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ void CommandRegistry::StartFamily() {
}

std::string_view CommandRegistry::RenamedOrOriginal(std::string_view orig) const {
if (cmd_rename_map_.contains(orig)) {
if (!cmd_rename_map_.empty() && cmd_rename_map_.contains(orig)) {
return cmd_rename_map_.find(orig)->second;
}
return orig;
Expand All @@ -173,6 +173,32 @@ CommandRegistry::FamiliesVec CommandRegistry::GetFamilies() {
return std::move(family_of_commands_);
}

std::pair<const CommandId*, CmdArgList> CommandRegistry::FindExtended(string_view cmd,
CmdArgList tail_args) const {
if (cmd == RenamedOrOriginal("ACL"sv)) {
if (tail_args.empty()) {
return {Find(cmd), {}};
}

auto second_cmd = absl::AsciiStrToUpper(ArgS(tail_args, 0));
string full_cmd = absl::StrCat(cmd, " ", second_cmd);

return {Find(full_cmd), tail_args.subspan(1)};
}

const CommandId* res = Find(cmd);
if (!res)
return {nullptr, {}};

// A workaround for XGROUP HELP that does not fit our static taxonomy of commands.
if (tail_args.size() == 1 && res->name() == "XGROUP") {
if (absl::EqualsIgnoreCase(ArgS(tail_args, 0), "HELP")) {
res = Find("_XGROUP_HELP");
}
}
return {res, tail_args};
}

namespace CO {

const char* OptName(CO::CommandOpt fl) {
Expand Down
3 changes: 3 additions & 0 deletions src/server/command_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,9 @@ class CommandRegistry {
using FamiliesVec = std::vector<std::vector<std::string>>;
FamiliesVec GetFamilies();

std::pair<const CommandId*, facade::CmdArgList> FindExtended(std::string_view cmd,
facade::CmdArgList tail_args) const;

private:
absl::flat_hash_map<std::string, CommandId> cmd_map_;
absl::flat_hash_map<std::string, std::string> cmd_rename_map_;
Expand Down
4 changes: 2 additions & 2 deletions src/server/hset_family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1125,8 +1125,8 @@ void HSetFamily::HRandField(CmdArgList args, ConnectionContext* cntx) {
}

if (args.size() == 3) {
ToUpper(&args[2]);
if (ArgS(args, 2) != "WITHVALUES")
string arg = absl::AsciiStrToUpper(ArgS(args, 2));
if (arg != "WITHVALUES")
return cntx->SendError(kSyntaxErr);
else
with_values = true;
Expand Down
59 changes: 13 additions & 46 deletions src/server/main_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1241,16 +1241,16 @@ std::optional<ErrorReply> Service::VerifyCommandState(const CommandId* cid, CmdA
void Service::DispatchCommand(CmdArgList args, facade::ConnectionContext* cntx) {
absl::Cleanup clear_last_error(
[cntx]() { std::ignore = cntx->reply_builder()->ConsumeLastError(); });
CHECK(!args.empty());
DCHECK(!args.empty());
DCHECK_NE(0u, shard_set->size()) << "Init was not called";

ServerState& etl = *ServerState::tlocal();

ToUpper(&args[0]);
const auto [cid, args_no_cmd] = FindCmd(args);
string cmd = absl::AsciiStrToUpper(ArgS(args, 0));
const auto [cid, args_no_cmd] = registry_.FindExtended(cmd, args.subspan(1));

if (cid == nullptr) {
return cntx->SendError(ReportUnknownCmd(ArgS(args, 0)));
return cntx->SendError(ReportUnknownCmd(cmd));
}

ConnectionContext* dfly_cntx = static_cast<ConnectionContext*>(cntx);
Expand All @@ -1264,7 +1264,7 @@ void Service::DispatchCommand(CmdArgList args, facade::ConnectionContext* cntx)
}

// Don't interrupt running multi commands or admin connections.
if (!dispatching_in_multi && cntx->conn() && !cntx->conn()->IsPrivileged()) {
if (etl.IsPaused() && !dispatching_in_multi && cntx->conn() && !cntx->conn()->IsPrivileged()) {
bool is_write = cid->IsWriteOnly();
is_write |= cid->name() == "PUBLISH" || cid->name() == "EVAL" || cid->name() == "EVALSHA";
is_write |= cid->name() == "EXEC" && dfly_cntx->conn_state.exec_info.is_write;
Expand Down Expand Up @@ -1535,8 +1535,8 @@ size_t Service::DispatchManyCommands(absl::Span<CmdArgList> args_list,
return 0;

for (auto args : args_list) {
ToUpper(&args[0]);
const auto [cid, tail_args] = FindCmd(args);
string cmd = absl::AsciiStrToUpper(ArgS(args, 0));
const auto [cid, tail_args] = registry_.FindExtended(cmd, args.subspan(1));

// MULTI...EXEC commands need to be collected into a single context, so squashing is not
// possible
Expand Down Expand Up @@ -1850,10 +1850,10 @@ void Service::CallFromScript(ConnectionContext* cntx, Interpreter::CallArgs& ca)

if (ca.async) {
auto& info = cntx->conn_state.script_info;
ToUpper(&ca.args[0]);
string cmd = absl::AsciiStrToUpper(ArgS(ca.args, 0));

// Full command verification happens during squashed execution
if (auto* cid = registry_.Find(ArgS(ca.args, 0)); cid != nullptr) {
if (auto* cid = registry_.Find(cmd); cid != nullptr) {
auto replies = ca.error_abort ? ReplyMode::ONLY_ERR : ReplyMode::NONE;
info->async_cmds.emplace_back(std::move(*ca.buffer), cid, ca.args.subspan(1), replies);
info->async_cmds_heap_mem += info->async_cmds.back().UsedMemory();
Expand Down Expand Up @@ -1995,35 +1995,6 @@ optional<bool> StartMultiEval(DbIndex dbid, CmdArgList keys, ScriptMgr::ScriptPa
return false;
}

static std::string FullAclCommandFromArgs(CmdArgList args, std::string_view name) {
ToUpper(&args[1]);
auto res = absl::StrCat(name, " ", ArgS(args, 1));
return res;
}

std::pair<const CommandId*, CmdArgList> Service::FindCmd(CmdArgList args) const {
const std::string_view command = facade::ToSV(args[0]);
std::string_view acl = "ACL";
if (command == registry_.RenamedOrOriginal(acl)) {
if (args.size() == 1) {
return {registry_.Find(ArgS(args, 0)), args};
}
return {registry_.Find(FullAclCommandFromArgs(args, command)), args.subspan(2)};
}

const CommandId* res = registry_.Find(ArgS(args, 0));
if (!res)
return {nullptr, args};

// A workaround for XGROUP HELP that does not fit our static taxonomy of commands.
if (args.size() == 2 && res->name() == "XGROUP") {
if (absl::EqualsIgnoreCase(ArgS(args, 1), "HELP")) {
res = registry_.Find("_XGROUP_HELP");
}
}
return {res, args.subspan(1)};
}

static bool CanRunSingleShardMulti(optional<ShardId> sid, const ScriptMgr::ScriptParams& params,
const Transaction& tx) {
if (!sid.has_value()) {
Expand Down Expand Up @@ -2426,8 +2397,7 @@ void Service::PUnsubscribe(CmdArgList args, ConnectionContext* cntx) {
// Not a real implementation. Serves as a decorator to accept some function commands
// for testing.
void Service::Function(CmdArgList args, ConnectionContext* cntx) {
ToUpper(&args[0]);
string_view sub_cmd = ArgS(args, 0);
string sub_cmd = absl::AsciiStrToUpper(ArgS(args, 0));

if (sub_cmd == "FLUSH") {
return cntx->SendOk();
Expand Down Expand Up @@ -2475,8 +2445,7 @@ void Service::Pubsub(CmdArgList args, ConnectionContext* cntx) {
return;
}

ToUpper(&args[0]);
string_view subcmd = ArgS(args, 0);
string subcmd = absl::AsciiStrToUpper(ArgS(args, 0));

if (subcmd == "HELP") {
string_view help_arr[] = {
Expand Down Expand Up @@ -2552,8 +2521,7 @@ void Service::Command(CmdArgList args, ConnectionContext* cntx) {
return;
}

ToUpper(&args[0]);
string_view subcmd = ArgS(args, 0);
string subcmd = absl::AsciiStrToUpper(ArgS(args, 0));

// COUNT
if (subcmd == "COUNT") {
Expand All @@ -2562,8 +2530,7 @@ void Service::Command(CmdArgList args, ConnectionContext* cntx) {

// INFO [cmd]
if (subcmd == "INFO" && args.size() == 2) {
ToUpper(&args[1]);
string_view cmd = ArgS(args, 1);
string cmd = absl::AsciiStrToUpper(ArgS(args, 1));

if (const auto* cid = registry_.Find(cmd); cid) {
rb->StartArray(1);
Expand Down
1 change: 0 additions & 1 deletion src/server/main_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ class Service : public facade::ServiceInterface {
facade::ConnectionContext* CreateContext(util::FiberSocketBase* peer,
facade::Connection* owner) final;

std::pair<const CommandId*, CmdArgList> FindCmd(CmdArgList args) const;
const CommandId* FindCmd(std::string_view) const;

CommandRegistry* mutable_registry() {
Expand Down
4 changes: 0 additions & 4 deletions src/server/server_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,6 @@ void ServerState::AwaitPauseState(bool is_write) {
});
}

bool ServerState::IsPaused() const {
return (client_pauses_[0] + client_pauses_[1]) > 0;
}

void ServerState::DecommitMemory(uint8_t flags) {
if (flags & kDataHeap) {
mi_heap_collect(data_heap(), true);
Expand Down
4 changes: 3 additions & 1 deletion src/server/server_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,9 @@ class ServerState { // public struct - to allow initialization.
// @is_write controls whether the command is a write command or not.
void AwaitPauseState(bool is_write);

bool IsPaused() const;
bool IsPaused() const {
return (client_pauses_[0] + client_pauses_[1]) > 0;
}

SlowLogShard& GetSlowLog() {
return slow_log_shard_;
Expand Down
Loading