diff --git a/src/facade/cmd_arg_parser.h b/src/facade/cmd_arg_parser.h index 9a5ea30f5240..17fe1962f1e3 100644 --- a/src/facade/cmd_arg_parser.h +++ b/src/facade/cmd_arg_parser.h @@ -206,9 +206,6 @@ struct CmdArgParser { }; public: - explicit CmdArgParser(ArgSlice args) : args_{args} { - } - explicit CmdArgParser(const cmn::BackedArguments& bargs, uint32_t offset = 0) : args_{bargs, offset} { } diff --git a/src/facade/cmd_arg_parser_test.cc b/src/facade/cmd_arg_parser_test.cc index 2d7129bcb187..ad70d0231288 100644 --- a/src/facade/cmd_arg_parser_test.cc +++ b/src/facade/cmd_arg_parser_test.cc @@ -20,16 +20,13 @@ namespace facade { class CmdArgParserTest : public testing::Test { public: CmdArgParser Make(absl::Span args) { - storage_.assign(args.begin(), args.end()); - arg_vec_.clear(); - for (auto& s : storage_) - arg_vec_.push_back(MutableSlice{s.data(), s.size()}); - return CmdArgParser{absl::MakeSpan(arg_vec_)}; + storage_.Assign(args.begin(), args.end(), args.size()); + return CmdArgParser{storage_}; } private: CmdArgVec arg_vec_; - std::vector storage_; + cmn::BackedArguments storage_; }; TEST_F(CmdArgParserTest, BasicTypes) { diff --git a/src/facade/facade_types.h b/src/facade/facade_types.h index d71624cedd78..3936cfda0719 100644 --- a/src/facade/facade_types.h +++ b/src/facade/facade_types.h @@ -127,14 +127,6 @@ class ParsedArgs { return const_iterator{this, size()}; } - ArgSlice ToSlice(CmdArgVec* scratch) const { - return std::visit([scratch](const auto& args) { return args.ToSlice(scratch); }, args_); - } - - void ToVec(CmdArgVec* vec) const { - std::visit([vec](const auto& args) { return args.ToVec(vec); }, args_); - } - private: struct WrapperBacked { WrapperBacked(const cmn::BackedArguments* args, uint32_t index = 0) // NOLINT @@ -163,18 +155,6 @@ class ParsedArgs { std::string_view at(size_t i) const { return args_->at(index_ + i); } - - ArgSlice ToSlice(CmdArgVec* scratch) const { - ToVec(scratch); - return *scratch; - } - - void ToVec(CmdArgVec* vec) const { - vec->clear(); - vec->reserve(args_->size() - index_); - for (auto arg : args_->view(index_)) - vec->emplace_back(arg); - } }; struct Slice : public ArgSlice { @@ -189,14 +169,6 @@ class ParsedArgs { ParsedArgs Tail(unsigned offset = 1) const { return ParsedArgs{subspan(offset)}; } - - ArgSlice ToSlice(void* /*scratch*/) const { - return *this; - } - - void ToVec(CmdArgVec* vec) const { - vec->assign(begin(), end()); - } }; std::variant args_; }; diff --git a/src/server/command_registry.h b/src/server/command_registry.h index e219b8d5e16b..74eba149d1b7 100644 --- a/src/server/command_registry.h +++ b/src/server/command_registry.h @@ -115,7 +115,7 @@ class CommandId : public facade::CommandId { std::optional(const facade::ParsedArgs&) const>; // Returns the invoke time in usec. - void Invoke(CmdArgList args, CommandContext* cmd_cntx) const { + void Invoke(const facade::ParsedArgs& args, CommandContext* cmd_cntx) const { handler_(facade::CmdArgParser{args}, cmd_cntx); } @@ -148,10 +148,10 @@ class CommandId : public facade::CommandId { return interleave_step_; } + // Adapts a free-function handler (including coroutine handlers returning cmd::CmdR) into the + // void Handler, discarding the unused return object. template CommandId&& SetHandler(RT f(facade::CmdArgParser, CommandContext*)) && { - handler_ = [f](facade::CmdArgParser parser, CommandContext* cntx) { - f(std::move(parser), cntx); - }; + handler_ = [f](facade::CmdArgParser parser, CommandContext* cntx) { f(parser, cntx); }; return std::move(*this); } @@ -161,9 +161,7 @@ class CommandId : public facade::CommandId { return std::move(*this).SetHandler(f); } - CommandId&& SetHandler(Handler f, bool async_support = false) && { - if (async_support) - kind_mask_ |= SUPPORT_ASYNC; + CommandId&& SetHandler(Handler f) && { handler_ = std::move(f); return std::move(*this); } diff --git a/src/server/conn_context.cc b/src/server/conn_context.cc index 8fa7ff284ee0..bfa9232c1921 100644 --- a/src/server/conn_context.cc +++ b/src/server/conn_context.cc @@ -59,7 +59,7 @@ vector FormatEvalSlowlog(const ConnectionState& state) { } // namespace -StoredCmd::StoredCmd(const CommandId* cid, facade::ArgSlice args, facade::ReplyMode mode) +StoredCmd::StoredCmd(const CommandId* cid, const facade::ParsedArgs& args, facade::ReplyMode mode) : cid_{cid}, reply_mode_{mode} { backed_ = std::make_unique(args.begin(), args.end(), args.size()); args_ = facade::ParsedArgs{*backed_}; @@ -73,10 +73,6 @@ StoredCmd::StoredCmd(const CommandId* cid, cmn::BackedArguments* src, uint8_t ta args_ = facade::ParsedArgs{*backed_, tail_index}; } -CmdArgList StoredCmd::Slice(CmdArgVec* scratch) const { - return args_.ToSlice(scratch); -} - std::string StoredCmd::FirstArg() const { if (NumArgs() == 0) { return {}; @@ -313,11 +309,10 @@ void CommandContext::ReuseInternal() { cid_ = nullptr; tx_ = nullptr; tail_args_ = {}; - arg_slice_backing.clear(); start_time_usec = 0; } -void CommandContext::RecordLatency(facade::ArgSlice tail_args) const { +void CommandContext::RecordLatency(const facade::ParsedArgs& tail_args) const { DCHECK_GT(start_time_usec, 0u); int64_t after = base::CycleClock::ToUsec(base::CycleClock::Now()); @@ -344,6 +339,7 @@ void CommandContext::RecordLatency(facade::ArgSlice tail_args) const { vector aux_params; CmdArgVec aux_slice; + facade::ParsedArgs slowlog_args = tail_args; // Rewrite arguments for exec/eval with stats if (cid_->IsMultiTransactional()) { @@ -351,16 +347,18 @@ void CommandContext::RecordLatency(facade::ArgSlice tail_args) const { aux_params = FormatExecSlowlog(cntx->conn_state); else if (cid_->IsEvalGroup()) aux_params = FormatEvalSlowlog(cntx->conn_state); + aux_slice = {aux_params.begin(), aux_params.end()}; if (tail_args.size() > 0) { + facade::ParsedArgs args_suffix = tail_args; if (!aux_params.empty()) - tail_args.remove_prefix(1); // remove script/sha from eval/evalsha - aux_slice.insert(aux_slice.end(), tail_args.begin(), tail_args.end()); + args_suffix = args_suffix.Tail(1); // remove script/sha from eval/evalsha + aux_slice.insert(aux_slice.end(), args_suffix.begin(), args_suffix.end()); } - tail_args = aux_slice; + slowlog_args = facade::ArgSlice{aux_slice}; } - ServerState::SafeTLocal()->GetSlowLog().Add(cid_->name(), tail_args, conn->GetName(), + ServerState::SafeTLocal()->GetSlowLog().Add(cid_->name(), slowlog_args, conn->GetName(), conn->RemoteEndpointStr(), execution_time_usec, absl::GetCurrentTimeNanos() / 1000); } diff --git a/src/server/conn_context.h b/src/server/conn_context.h index 3e3f1fdf6e93..23bd793e071c 100644 --- a/src/server/conn_context.h +++ b/src/server/conn_context.h @@ -32,10 +32,6 @@ struct CmdRef { bool IsValid() const { return cid != nullptr; } - - facade::ArgSlice Slice(CmdArgVec* scratch) const { - return args.ToSlice(scratch); - } }; // Stores command id and arguments for delayed invocation. @@ -43,7 +39,8 @@ struct CmdRef { class StoredCmd { public: // Deep copy of args, creates backing storage internally. - StoredCmd(const CommandId* cid, ArgSlice args, facade::ReplyMode mode = facade::ReplyMode::FULL); + StoredCmd(const CommandId* cid, const facade::ParsedArgs& args, + facade::ReplyMode mode = facade::ReplyMode::FULL); // Moves args from src via swap. src's BackedArguments will be empty after this. // tail_index specifies how many leading args to skip (e.g., 1 to skip the command name). @@ -58,7 +55,6 @@ class StoredCmd { return backed_ ? backed_->HeapMemory() + sizeof(*backed_) : 0; } - facade::ArgSlice Slice(CmdArgVec* scratch) const; const facade::ParsedArgs& Args() const { return args_; } @@ -419,7 +415,7 @@ class CommandContext : public facade::ParsedCommand { return static_cast(conn_cntx_); } - void RecordLatency(facade::ArgSlice tail_args) const; + void RecordLatency(const facade::ParsedArgs& tail_args) const; facade::Connection* conn() const { return conn_cntx_->conn(); @@ -447,15 +443,11 @@ class CommandContext : public facade::ParsedCommand { uint64_t start_time_usec = 0; - // Stores backing array for tail args slice - CmdArgVec arg_slice_backing; - protected: void ReuseInternal() final; - // Command arguments without the command name. Replaces arg_slice_backing as the - // canonical arg source once all handlers are migrated. Points into BackedArguments - // owned by this CommandContext (or StoredCmd for EXEC), so it survives async execution. + // Command arguments without the command name. Points into BackedArguments owned by this + // CommandContext (or StoredCmd for EXEC), so it survives async execution. facade::ParsedArgs tail_args_; Transaction* tx_ = nullptr; diff --git a/src/server/debugcmd.cc b/src/server/debugcmd.cc index ed08a0e7da4a..d884f94dc6b8 100644 --- a/src/server/debugcmd.cc +++ b/src/server/debugcmd.cc @@ -613,9 +613,11 @@ DebugCmd::DebugCmd(ServerFamily* owner, cluster::ClusterFamily* cf, ConnectionCo } void DebugCmd::Run(facade::CmdArgParser parser, CommandContext* cmd_cntx) { - CmdArgVec scratch; - CmdArgList args = parser.UnparsedArgs().ToSlice(&scratch); - string subcmd = absl::AsciiStrToUpper(ArgS(args, 0)); + string subcmd = absl::AsciiStrToUpper(parser.Next()); + if (auto err = parser.TakeError(); err) { + return cmd_cntx->SendError(err.MakeReply()); + } + if (subcmd == "HELP") { string_view help_arr[] = { "DEBUG [ [value] [opt] ...]. Subcommands are:", @@ -708,29 +710,28 @@ void DebugCmd::Run(facade::CmdArgParser parser, CommandContext* cmd_cntx) { VLOG(1) << "subcmd " << subcmd; if (subcmd == "POPULATE") { - return Populate(args, cmd_cntx); + return Populate(parser, cmd_cntx); } if (subcmd == "RELOAD") { - return Reload(args, cmd_cntx); + return Reload(parser, cmd_cntx); } - if (subcmd == "REPLICA" && args.size() == 2) { - return Replica(args, cmd_cntx); + if (subcmd == "REPLICA" && parser.UnparsedArgs().size() == 1) { + return Replica(parser, cmd_cntx); } - if (subcmd == "MIGRATION" && args.size() == 2) { - return Migration(args, cmd_cntx); + if (subcmd == "MIGRATION" && parser.UnparsedArgs().size() == 1) { + return Migration(parser, cmd_cntx); } if (subcmd == "WATCHED") { return Watched(cmd_cntx); } - if (subcmd == "OBJECT" && args.size() >= 2) { - string_view key = ArgS(args, 1); - args.remove_prefix(2); - return Inspect(key, args, cmd_cntx); + if (subcmd == "OBJECT" && parser.HasNext()) { + string_view key = parser.Next(); + return Inspect(key, parser, cmd_cntx); } if (subcmd == "TX") { @@ -754,37 +755,37 @@ void DebugCmd::Run(facade::CmdArgParser parser, CommandContext* cmd_cntx) { } if (subcmd == "TRAFFIC") { - return LogTraffic(CmdArgParser{args.subspan(1)}, cmd_cntx); + return LogTraffic(parser, cmd_cntx); } - if (subcmd == "RECVSIZE" && args.size() == 2) { - return RecvSize(ArgS(args, 1), cmd_cntx); + if (subcmd == "RECVSIZE" && parser.UnparsedArgs().size() == 1) { + return RecvSize(parser.Next(), cmd_cntx); } - if (subcmd == "TOPK" && args.size() >= 2) { - return Topk(args.subspan(1), cmd_cntx); + if (subcmd == "TOPK" && parser.HasNext()) { + return Topk(parser, cmd_cntx); } - if (subcmd == "KEYS" && args.size() >= 2) { - return Keys(args.subspan(1), cmd_cntx); + if (subcmd == "KEYS" && parser.HasNext()) { + return Keys(parser, cmd_cntx); } - if (subcmd == "VALUES" && args.size() >= 2) { - return Values(args.subspan(1), cmd_cntx); + if (subcmd == "VALUES" && parser.HasNext()) { + return Values(parser, cmd_cntx); } if (subcmd == "COMPRESSION") { - return Compression(CmdArgParser{args.subspan(1)}, cmd_cntx); + return Compression(parser, cmd_cntx); } if (subcmd == "IOSTATS") { - return IOStats(args.subspan(1), cmd_cntx); + return IOStats(parser, cmd_cntx); } if (subcmd == "SEGMENTS") { - return Segments(args.subspan(1), cmd_cntx); + return Segments(parser, cmd_cntx); } if (subcmd == "COMPACT-TABLE") { - return CompactTable(args.subspan(1), cmd_cntx); + return CompactTable(parser, cmd_cntx); } if (subcmd == "UNIQ-STRS") { @@ -800,12 +801,12 @@ void DebugCmd::Shutdown() { shard_set->pool()->AwaitFiberOnAll([](auto*) { facade::Connection::StopTrafficLogging(); }); } -void DebugCmd::Reload(CmdArgList args, CommandContext* cmd_cntx) { +void DebugCmd::Reload(facade::CmdArgParser parser, CommandContext* cmd_cntx) { bool save = true; auto* rb = static_cast(cmd_cntx->rb()); - for (size_t i = 1; i < args.size(); ++i) { - string opt = absl::AsciiStrToUpper(ArgS(args, i)); + while (parser.HasNext()) { + string opt = absl::AsciiStrToUpper(parser.Next()); VLOG(1) << "opt " << opt; if (opt == "NOSAVE") { @@ -841,10 +842,8 @@ void DebugCmd::Reload(CmdArgList args, CommandContext* cmd_cntx) { rb->SendOk(); } -void DebugCmd::Replica(CmdArgList args, CommandContext* cmd_cntx) { - args.remove_prefix(1); - - string opt = absl::AsciiStrToUpper(ArgS(args, 0)); +void DebugCmd::Replica(facade::CmdArgParser parser, CommandContext* cmd_cntx) { + string opt = absl::AsciiStrToUpper(parser.Next()); auto* rb = static_cast(cmd_cntx->rb()); if (opt == "PAUSE" || opt == "RESUME") { @@ -867,10 +866,8 @@ void DebugCmd::Replica(CmdArgList args, CommandContext* cmd_cntx) { return cmd_cntx->SendError(UnknownSubCmd("replica", "DEBUG")); } -void DebugCmd::Migration(CmdArgList args, CommandContext* cmd_cntx) { - args.remove_prefix(1); - - string opt = absl::AsciiStrToUpper(ArgS(args, 0)); +void DebugCmd::Migration(facade::CmdArgParser parser, CommandContext* cmd_cntx) { + string opt = absl::AsciiStrToUpper(parser.Next()); auto* rb = static_cast(cmd_cntx->rb()); if (opt == "PAUSE" || opt == "RESUME") { cf_.PauseAllIncomingMigrations(opt == "PAUSE"); @@ -886,7 +883,6 @@ enum PopulateFlag { FLAG_RAND, FLAG_TYPE, FLAG_ELEMENTS, FLAG_SLOT, FLAG_EXPIRE, // optional: [RAND | TYPE typename | ELEMENTS element num | SLOTS (key value)+ | EXPIRE start end] optional DebugCmd::ParsePopulateArgs(CmdArgParser parser, CommandContext* cmd_cntx) { - parser.Skip(1); PopulateOptions options; options.total_count = parser.Next(); @@ -936,8 +932,8 @@ optional DebugCmd::ParsePopulateArgs(CmdArgParser par return options; } -void DebugCmd::Populate(CmdArgList args, CommandContext* cmd_cntx) { - optional options = ParsePopulateArgs(CmdArgParser{args}, cmd_cntx); +void DebugCmd::Populate(facade::CmdArgParser parser, CommandContext* cmd_cntx) { + optional options = ParsePopulateArgs(parser, cmd_cntx); if (!options.has_value()) { return; } @@ -1160,14 +1156,14 @@ void DebugCmd::LogTraffic(CmdArgParser parser, CommandContext* cmd_cntx) { rb->SendOk(); } -void DebugCmd::Inspect(string_view key, CmdArgList args, CommandContext* cmd_cntx) { +void DebugCmd::Inspect(string_view key, facade::CmdArgParser parser, CommandContext* cmd_cntx) { EngineShardSet& ess = *shard_set; ShardId sid = Shard(key, ess.size()); VLOG(1) << "DebugCmd::Inspect " << key; bool check_compression = false; - if (args.size() == 1) { - check_compression = absl::AsciiStrToUpper(ArgS(args, 0)) == "COMPRESS"; + if (parser.UnparsedArgs().size() == 1) { + check_compression = absl::EqualsIgnoreCase(parser.Peek(), "COMPRESS"); } string resp; auto* rb = static_cast(cmd_cntx->rb()); @@ -1429,17 +1425,15 @@ void DebugCmd::RecvSize(string_view param, CommandContext* cmd_cntx) { rb->SendVerbatimString(hist); } -void DebugCmd::Topk(CmdArgList args, CommandContext* cmd_cntx) { +void DebugCmd::Topk(facade::CmdArgParser parser, CommandContext* cmd_cntx) { auto* rb = static_cast(cmd_cntx->rb()); - DCHECK_GE(args.size(), 1u); - string_view subcmd = ArgS(args, 0); + string_view subcmd = parser.Next(); if (absl::EqualsIgnoreCase(subcmd, "ON")) { - uint32_t min_freq = 100; - if (args.size() > 1) { - if (!absl::SimpleAtoi(ArgS(args, 1), &min_freq)) - return cmd_cntx->SendError(kUintErr); - } + uint32_t min_freq = parser.NextOrDefault(100); + if (parser.TakeError()) + return cmd_cntx->SendError(kUintErr); + shard_set->RunBriefInParallel([&](EngineShard* es) { cntx_->ns->GetDbSlice(es->shard_id()).StartSampleTopK(cntx_->db_index(), min_freq); }); @@ -1448,12 +1442,10 @@ void DebugCmd::Topk(CmdArgList args, CommandContext* cmd_cntx) { if (absl::EqualsIgnoreCase(subcmd, "OFF")) { vector results(shard_set->size()); - uint32_t max_keys = 50; + uint32_t max_keys = parser.NextOrDefault(50); - if (args.size() > 1) { - if (!absl::SimpleAtoi(ArgS(args, 1), &max_keys)) - return cmd_cntx->SendError(kUintErr); - } + if (parser.TakeError()) + return cmd_cntx->SendError(kUintErr); shard_set->RunBriefInParallel([&](EngineShard* es) { results[es->shard_id()] = @@ -1486,8 +1478,8 @@ void DebugCmd::Topk(CmdArgList args, CommandContext* cmd_cntx) { return cmd_cntx->SendError(kSyntaxErr); } -void DebugCmd::Keys(CmdArgList args, CommandContext* cmd_cntx) { - string_view subcmd = ArgS(args, 0); +void DebugCmd::Keys(facade::CmdArgParser parser, CommandContext* cmd_cntx) { + string_view subcmd = parser.Next(); auto* rb = static_cast(cmd_cntx->rb()); if (absl::EqualsIgnoreCase(subcmd, "ON")) { shard_set->RunBriefInParallel([&](EngineShard* es) { @@ -1512,8 +1504,8 @@ void DebugCmd::Keys(CmdArgList args, CommandContext* cmd_cntx) { return cmd_cntx->SendError(kSyntaxErr); } -void DebugCmd::Values(CmdArgList args, CommandContext* cmd_cntx) { - string_view subcmd = ArgS(args, 0); +void DebugCmd::Values(facade::CmdArgParser parser, CommandContext* cmd_cntx) { + string_view subcmd = parser.Next(); auto* rb = static_cast(cmd_cntx->rb()); if (absl::EqualsIgnoreCase(subcmd, "ON")) { shard_set->RunBriefInParallel([&](EngineShard* es) { @@ -1676,10 +1668,10 @@ void DebugCmd::Compression(CmdArgParser parser, CommandContext* cmd_cntx) { } } -void DebugCmd::IOStats(CmdArgList args, CommandContext* cmd_cntx) { +void DebugCmd::IOStats(facade::CmdArgParser parser, CommandContext* cmd_cntx) { auto* rb = static_cast(cmd_cntx->rb()); - bool per_second = !args.empty() && absl::EqualsIgnoreCase(args[0], "PS"); + bool per_second = parser.HasNext() && absl::EqualsIgnoreCase(parser.Peek(), "PS"); vector stats(shard_set->pool()->size()); shard_set->pool()->AwaitBrief( @@ -1703,7 +1695,7 @@ void DebugCmd::IOStats(CmdArgList args, CommandContext* cmd_cntx) { } } -void DebugCmd::Segments(CmdArgList args, CommandContext* cmd_cntx) { +void DebugCmd::Segments(facade::CmdArgParser, CommandContext* cmd_cntx) { auto* rb = static_cast(cmd_cntx->rb()); vector info(shard_set->size()); @@ -1724,17 +1716,15 @@ void DebugCmd::Segments(CmdArgList args, CommandContext* cmd_cntx) { rb->SendVerbatimString(result); } -void DebugCmd::CompactTable(CmdArgList args, CommandContext* cmd_cntx) { +void DebugCmd::CompactTable(facade::CmdArgParser parser, CommandContext* cmd_cntx) { auto* rb = static_cast(cmd_cntx->rb()); - double threshold = 0.25; - if (args.size() > 0) { - if (!absl::SimpleAtod(facade::ToSV(args[0]), &threshold)) { - return rb->SendError("Invalid threshold value"); - } - if (threshold <= 0.0 || threshold > 1.0) { - return rb->SendError("Threshold must be between 0 and 1"); - } + double threshold = parser.NextOrDefault(0.25); + if (auto err = parser.TakeError(); err) + return rb->SendError("Invalid threshold value"); + + if (threshold <= 0.0 || threshold > 1.0) { + return rb->SendError("Threshold must be between 0 and 1"); } const DbIndex db_idx = cmd_cntx->server_conn_cntx()->db_index(); @@ -1823,23 +1813,19 @@ void DebugCmd::DoPopulateBatch(const PopulateOptions& options, const PopulateBat new Transaction{local_tx.get(), EngineShard::tlocal()->shard_id(), nullopt}; cmn::BackedArguments backed_args; - CmdArgVec arg_vec; facade::CapturingReplyBuilder crb; absl::InsecureBitGen gen; CommandContext cmd_cntx{&crb, cntx_}; cmd_cntx.SetupTx(exec_cid, stub_tx.get()); auto invoke = [&](const CommandId* cid) { - arg_vec.clear(); - for (auto sv : backed_args.view()) - arg_vec.push_back(sv); - auto args_span = absl::MakeSpan(arg_vec); + facade::ParsedArgs args{backed_args}; stub_tx->MultiSwitchCmd(cid); crb.SetReplyMode(ReplyMode::NONE); - stub_tx->InitByArgs(cntx_->ns, cntx_->conn_state.db_index, facade::ParsedArgs{backed_args}); + stub_tx->InitByArgs(cntx_->ns, cntx_->conn_state.db_index, args); cmd_cntx.UpdateCid(cid); - cmd_cntx.SetTailArgs(facade::ParsedArgs{backed_args}); - sf_.service().InvokeCmd(args_span, &cmd_cntx); + cmd_cntx.SetTailArgs(args); + sf_.service().InvokeCmd(args, &cmd_cntx); }; for (unsigned i = 0; i < batch.sz; ++i) { diff --git a/src/server/debugcmd.h b/src/server/debugcmd.h index e6c2e2455e56..958a792d439e 100644 --- a/src/server/debugcmd.h +++ b/src/server/debugcmd.h @@ -39,17 +39,17 @@ class DebugCmd { static void Shutdown(); private: - void Populate(CmdArgList args, CommandContext* cmd_cntx); + void Populate(facade::CmdArgParser parser, CommandContext* cmd_cntx); static std::optional ParsePopulateArgs(facade::CmdArgParser parser, CommandContext* cmd_cntx); void PopulateRangeFiber(uint64_t from, uint64_t count, const PopulateOptions& opts); - void Reload(CmdArgList args, CommandContext* cmd_cntx); - void Replica(CmdArgList args, CommandContext* cmd_cntx); - void Migration(CmdArgList args, CommandContext* cmd_cntx); + void Reload(facade::CmdArgParser parser, CommandContext* cmd_cntx); + void Replica(facade::CmdArgParser parser, CommandContext* cmd_cntx); + void Migration(facade::CmdArgParser parser, CommandContext* cmd_cntx); void Exec(CommandContext* cmd_cntx); - void Inspect(std::string_view key, CmdArgList args, CommandContext* cmd_cntx); + void Inspect(std::string_view key, facade::CmdArgParser parser, CommandContext* cmd_cntx); void Watched(CommandContext* cmd_cntx); void TxAnalysis(CommandContext* cmd_cntx); void ObjHist(CommandContext* cmd_cntx); @@ -57,13 +57,13 @@ class DebugCmd { void Shards(CommandContext* cmd_cntx); void LogTraffic(facade::CmdArgParser parser, CommandContext* cmd_cntx); void RecvSize(std::string_view param, CommandContext* cmd_cntx); - void Topk(CmdArgList args, CommandContext* cmd_cntx); - void Keys(CmdArgList args, CommandContext* cmd_cntx); - void Values(CmdArgList args, CommandContext* cmd_cntx); + void Topk(facade::CmdArgParser parser, CommandContext* cmd_cntx); + void Keys(facade::CmdArgParser parser, CommandContext* cmd_cntx); + void Values(facade::CmdArgParser parser, CommandContext* cmd_cntx); void Compression(facade::CmdArgParser parser, CommandContext* cmd_cntx); - void IOStats(CmdArgList args, CommandContext* cmd_cntx); - void Segments(CmdArgList args, CommandContext* cmd_cntx); - void CompactTable(CmdArgList args, CommandContext* cmd_cntx); + void IOStats(facade::CmdArgParser parser, CommandContext* cmd_cntx); + void Segments(facade::CmdArgParser parser, CommandContext* cmd_cntx); + void CompactTable(facade::CmdArgParser parser, CommandContext* cmd_cntx); void CountUniqueStrings(const CommandContext* cmd_cntx) const; struct PopulateBatch { DbIndex dbid; diff --git a/src/server/generic_family.cc b/src/server/generic_family.cc index 27c2e4d3a4dd..76a64f87c441 100644 --- a/src/server/generic_family.cc +++ b/src/server/generic_family.cc @@ -2295,7 +2295,8 @@ void GenericFamily::FieldExpire(facade::CmdArgParser parser, CommandContext* cmd return cmd_cntx->SendError(err.MakeReply()); } facade::CmdArgVec fields; - cmd_cntx->tail_args().Tail(parser.UnparsedStart()).ToVec(&fields); + auto field_args = cmd_cntx->tail_args().Tail(parser.UnparsedStart()); + fields.assign(field_args.begin(), field_args.end()); auto cb = [&](Transaction* t, EngineShard* shard) { return OpFieldExpire(t->GetOpArgs(shard), key, ttl_sec, fields); diff --git a/src/server/main_service.cc b/src/server/main_service.cc index 650d3e985c10..b641e0aa8dd5 100644 --- a/src/server/main_service.cc +++ b/src/server/main_service.cc @@ -578,9 +578,7 @@ Transaction::MultiMode DeduceExecMode(ExecScriptUse state, for (const auto& scmd : exec_info.body) { // We can only tell if eval is transactional based on they keycount if (absl::StartsWith(scmd.Cid()->name(), "EVAL")) { - CmdArgVec arg_vec{}; - auto args = scmd.Slice(&arg_vec); - auto keys = DetermineKeys(scmd.Cid(), args); + auto keys = DetermineKeys(scmd.Cid(), scmd.Args()); transactional |= (keys && keys.value().NumArgs() > 0); } else { transactional |= scmd.Cid()->IsTransactional(); @@ -1487,14 +1485,6 @@ DispatchResult Service::DispatchCommand(facade::ParsedArgs args, facade::ParsedC cmd_cntx->SetTailArgs(args_no_cmd); - ArgSlice tail_args; - if (cmd_cntx->IsDeferredReply()) { - args_no_cmd.ToVec(&cmd_cntx->arg_slice_backing); // Ensure lifetime - tail_args = cmd_cntx->arg_slice_backing; - } else { - tail_args = args_no_cmd.ToSlice(&cmd_cntx->arg_slice_backing); - } - // Block on CLIENT PAUSE if needed if (auto* conn = cmd_cntx->conn(); conn /* replica context doesn't have an owner */) { if (VLOG_IS_ON(2)) { @@ -1554,7 +1544,7 @@ DispatchResult Service::DispatchCommand(facade::ParsedArgs args, facade::ParsedC return DispatchResult::ERROR; } - DispatchResult res = InvokeCmd(tail_args, cmd_cntx); + DispatchResult res = InvokeCmd(cmd_cntx->tail_args(), cmd_cntx); if (dispatched_tx) { DCHECK(dfly_cntx->transaction == dispatched_tx.get()); // A new top-level transaction is created here only for top-level commands. Nested commands @@ -1627,7 +1617,7 @@ class ReplyGuard { std::string_view cid_name_; }; -DispatchResult Service::InvokeCmd(CmdArgList tail_args, CommandContext* cmd_cntx) { +DispatchResult Service::InvokeCmd(const facade::ParsedArgs& tail_args, CommandContext* cmd_cntx) { auto* cid = cmd_cntx->cid(); DCHECK(cid); DCHECK(!cid->Validate(tail_args)); @@ -1991,8 +1981,7 @@ void Service::TryEnqueueEvalAsyncCmd(const Interpreter::CallArgs& ca, CommandCon ParsedArgs parsed_args{*ca.args}; if (auto [cid, tail] = registry_.FindExtended(parsed_args); cid != nullptr) { auto reply_mode = abort_on_error ? ReplyMode::ONLY_ERR : ReplyMode::NONE; - CmdArgVec scratch; - info->async_cmds.emplace_back(cid, tail.ToSlice(&scratch), reply_mode); + info->async_cmds.emplace_back(cid, tail, reply_mode); info->async_cmds_heap_mem += info->async_cmds.back().UsedMemory(); } else if (abort_on_error) { // If we don't abort on errors, we can ignore it completely early_async_error = ReportUnknownCmd(ca.args->at(0)); @@ -2056,14 +2045,11 @@ void Service::CallFromScript(Interpreter::CallArgs& ca, CommandContext* cmd_cntx default: { // Save EVAL's state that DispatchCommand overwrites. auto saved_tail = cmd_cntx->tail_args(); - CmdArgVec saved_backing; - saved_backing.swap(cmd_cntx->arg_slice_backing); auto* prev = cmd_cntx->SwapReplier(&replier); DispatchCommand(ParsedArgs{*ca.args}, cmd_cntx, AsyncPreference::ONLY_SYNC); cmd_cntx->SwapReplier(prev); - saved_backing.swap(cmd_cntx->arg_slice_backing); cmd_cntx->SetTailArgs(saved_tail); } } @@ -2419,19 +2405,16 @@ template void IterateAllKeys(const ConnectionState::ExecInfo* exec_ for (auto& [dbid, key] : exec_info->watched_keys) f(MutableSlice{key.data(), key.size()}); - CmdArgVec arg_vec{}; - for (const auto& scmd : exec_info->body) { if (!scmd.Cid()->IsTransactional()) continue; - auto args = scmd.Slice(&arg_vec); - auto key_res = DetermineKeys(scmd.Cid(), args); + auto key_res = DetermineKeys(scmd.Cid(), scmd.Args()); if (!key_res.ok()) continue; for (unsigned i : key_res->Range()) - f(arg_vec[i]); + f(scmd.Args()[i]); } } @@ -2525,15 +2508,13 @@ void Service::Exec(CmdArgParser, CommandContext* cmd_cntx) { opts.max_squash_size = ServerState::tlocal()->max_squash_cmd_num; MultiCommandSquasher::Execute(std::move(cmd_gen), rb, cntx, this, opts); } else { - CmdArgVec arg_vec; DCHECK_EQ(cmd_cntx->cid(), exec_cid_); for (const auto& scmd : exec_info.body) { - CmdArgList args = scmd.Slice(&arg_vec); - if (scmd.Cid()->IsTransactional()) { cmd_cntx->tx()->MultiSwitchCmd(scmd.Cid()); - OpStatus st = cmd_cntx->tx()->InitByArgs(cntx->ns, cntx->conn_state.db_index, args); + OpStatus st = + cmd_cntx->tx()->InitByArgs(cntx->ns, cntx->conn_state.db_index, scmd.Args()); if (st != OpStatus::OK) { cmd_cntx->SendError(st); break; @@ -2544,7 +2525,7 @@ void Service::Exec(CmdArgParser, CommandContext* cmd_cntx) { // execution inside exec. cmd_cntx->UpdateCid(scmd.Cid()); cmd_cntx->SetTailArgs(scmd.Args()); - auto invoke_res = InvokeCmd(args, cmd_cntx); + auto invoke_res = InvokeCmd(scmd.Args(), cmd_cntx); if ((invoke_res != DispatchResult::OK) || rb->GetError()) // checks for i/o error, not logical error. break; diff --git a/src/server/main_service.h b/src/server/main_service.h index a2704624afdb..fcdfc84ef0b0 100644 --- a/src/server/main_service.h +++ b/src/server/main_service.h @@ -46,7 +46,7 @@ class Service : public facade::ServiceInterface { facade::ConnectionContext* cntx) final; // Check OOM and invoke command with args - facade::DispatchResult InvokeCmd(CmdArgList tail_args, CommandContext* cmd_cntx); + facade::DispatchResult InvokeCmd(const facade::ParsedArgs& tail_args, CommandContext* cmd_cntx); // Verify command prepares execution in correct state. // It's usually called before command execution. Only for multi/exec transactions it's checked diff --git a/src/server/multi_command_squasher.cc b/src/server/multi_command_squasher.cc index 705fd81240f4..7b9a22b1e338 100644 --- a/src/server/multi_command_squasher.cc +++ b/src/server/multi_command_squasher.cc @@ -119,24 +119,23 @@ MultiCommandSquasher::SquashResult MultiCommandSquasher::TrySquash(CmdRef cmd) { return SquashResult::NOT_SQUASHED; } - auto args = cmd.Slice(&tmp_keylist_); - if (args.empty()) + if (cmd.args.empty()) return SquashResult::NOT_SQUASHED; // Instead of returning an error, we treat command as non-squashable, allowing the // standalone execution path to handle it. // Validate returns an optional ErrorReply - if (cid.Validate(args).has_value()) + if (cid.Validate(cmd.args).has_value()) return SquashResult::NOT_SQUASHED; - auto keys = DetermineKeys(&cid, args); + auto keys = DetermineKeys(&cid, cmd.args); if (!keys.ok() || keys->NumArgs() == 0) return SquashResult::NOT_SQUASHED; // Check if all command keys belong to one shard ShardId last_sid = kInvalidSid; - for (string_view key : keys->Range(args)) { + for (string_view key : keys->Range(cmd.args)) { ShardId sid = Shard(key, shard_set->size()); if (last_sid == kInvalidSid || last_sid == sid) last_sid = sid; @@ -161,8 +160,6 @@ MultiCommandSquasher::SquashResult MultiCommandSquasher::TrySquash(CmdRef cmd) { bool MultiCommandSquasher::ExecuteStandalone(RedisReplyBuilder* rb, CmdRef cmd) { DCHECK(order_.empty()); // check no squashed chain is interrupted - auto args = cmd.Slice(&tmp_keylist_); - // In pipeline mode the reply is captured and deferred into the parsed command, preserving // the reply order with squashed commands whose replies are sent later by the connection. optional crb; @@ -181,7 +178,7 @@ bool MultiCommandSquasher::ExecuteStandalone(RedisReplyBuilder* rb, CmdRef cmd) auto* tx = cntx_->transaction; if (cmd.cid->IsTransactional()) { tx->MultiSwitchCmd(cmd.cid); - auto status = tx->InitByArgs(cntx_->ns, cntx_->conn_state.db_index, args); + auto status = tx->InitByArgs(cntx_->ns, cntx_->conn_state.db_index, cmd.args); if (status != OpStatus::OK) { rb->SendError(status); resolve(); @@ -192,7 +189,7 @@ bool MultiCommandSquasher::ExecuteStandalone(RedisReplyBuilder* rb, CmdRef cmd) CommandContext cmd_cntx{rb, cntx_}; cmd_cntx.SetupTx(cmd.cid, tx); cmd_cntx.SetTailArgs(cmd.args); - service_->InvokeCmd(args, &cmd_cntx); + service_->InvokeCmd(cmd.args, &cmd_cntx); resolve(); return true; @@ -203,7 +200,6 @@ OpStatus MultiCommandSquasher::SquashedHopCb(EngineShard* es, RespVersion resp_v DCHECK(!sinfo.dispatched.empty()); CapturingReplyBuilder crb(ReplyMode::FULL, resp_v); - CmdArgVec arg_vec; CommandContext local_cntx{&crb, cntx_}; local_cntx.SetupTx(nullptr, sinfo.local_tx.get()); @@ -219,7 +215,6 @@ OpStatus MultiCommandSquasher::SquashedHopCb(EngineShard* es, RespVersion resp_v for (auto& dispatched : sinfo.dispatched) { auto* ctx = &local_cntx; - auto args = dispatched.Slice(&arg_vec); crb.SetReplyMode(dispatched.reply_mode); // With tiered storage enabled, it makes sense to dispatch async commands concurrently @@ -234,13 +229,13 @@ OpStatus MultiCommandSquasher::SquashedHopCb(EngineShard* es, RespVersion resp_v ctx->SetupTx(dispatched.cid, local_cntx.tx()); ctx->tx()->MultiSwitchCmd(dispatched.cid); - auto status = ctx->tx()->InitByArgs(cntx_->ns, cntx_->conn_state.db_index, args); + auto status = ctx->tx()->InitByArgs(cntx_->ns, cntx_->conn_state.db_index, dispatched.args); if (status != OpStatus::OK) { ctx->SendError(status); // Calls Resolve() in async, routes to crb in non async } else { ctx->UpdateCid(dispatched.cid); ctx->SetTailArgs(dispatched.args); - service_->InvokeCmd(args, ctx); + service_->InvokeCmd(dispatched.args, ctx); } if (!do_async) { diff --git a/src/server/multi_command_squasher.h b/src/server/multi_command_squasher.h index 66d2041ef30c..12da54a3d0fa 100644 --- a/src/server/multi_command_squasher.h +++ b/src/server/multi_command_squasher.h @@ -106,7 +106,6 @@ class MultiCommandSquasher { size_t num_shards_ = 0; size_t num_commands_ = 0; // Total commands processed - std::vector tmp_keylist_; Stats stats_; }; diff --git a/src/server/slowlog.cc b/src/server/slowlog.cc index e775f3192abb..2fd6585d968c 100644 --- a/src/server/slowlog.cc +++ b/src/server/slowlog.cc @@ -19,7 +19,7 @@ void SlowLogShard::Reset() { log_entries_.clear(); } -void SlowLogShard::Add(const string_view command_name, CmdArgList args, +void SlowLogShard::Add(const string_view command_name, const facade::ParsedArgs& args, const string_view client_name, const string_view client_ip, uint64_t exec_time_usec, uint64_t unix_ts_usec) { DCHECK_GT(log_entries_.capacity(), 0u); @@ -35,7 +35,7 @@ void SlowLogShard::Add(const string_view command_name, CmdArgList args, slowlog_args.emplace_back(command_name, 0); for (size_t i = 0; i < slowlog_effective_length; ++i) { - string_view arg = facade::ArgS(args, i); + string_view arg = args[i]; size_t extra_bytes = 0; // If any of the arguments is deemed too long, it will be truncated // and the truncated string will be suffixed by the number of truncated bytes in diff --git a/src/server/slowlog.h b/src/server/slowlog.h index f5dc479790e5..bce46e73a9fd 100644 --- a/src/server/slowlog.h +++ b/src/server/slowlog.h @@ -13,8 +13,6 @@ namespace dfly { -using facade::CmdArgList; - constexpr size_t kMaximumSlowlogArgCount = 31; // 32 - 1 for the command name constexpr size_t kMaximumSlowlogArgLength = 128; @@ -35,8 +33,9 @@ class SlowLogShard { return log_entries_; } - void Add(const std::string_view command_name, CmdArgList args, const std::string_view client_name, - const std::string_view client_ip, uint64_t exec_time_usec, uint64_t unix_ts_usec); + void Add(const std::string_view command_name, const facade::ParsedArgs& args, + const std::string_view client_name, const std::string_view client_ip, + uint64_t exec_time_usec, uint64_t unix_ts_usec); void Reset(); void ChangeLength(size_t new_length);