Replies: 1 comment 2 replies
-
|
@apache/kvrocks-committers Hi everyone, I'd like to hear your thoughts. |
Beta Was this translation helpful? Give feedback.
2 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Subcommand support is blocking ACL development, so I want to write this proposal first.
1. Background
Kvrocks currently models command dispatch around top-level commands.
Many of these commands are effectively command families with multiple subcommands, where behavior, arity, flags,
key-spec, and privilege semantics vary by subcommand.
In upstream today, subcommand logic is implemented ad-hoc inside each
Commander::Parse()orCommander::Execute(),typically by parsing some argv position (often
args[1]) and branching viaif/elsetrees.This works for functional behavior, but system-level features cannot reliably answer:
This proposal introduces
Subcommandas a system-level, first-class abstraction as an upgrade to the commandregistration model. ACL is a major driver, but the abstraction is not ACL-specific.
2. Motivation
2.1 Problems in the current model
command" (
cmd|sub), but there is no framework-level object to provide it.CommandAttributesis registered for a top-level command, but subcommands often differ in arity/flags/keys.2.2 Concrete benefit examples
CLUSTERsubcommands have different concurrency semantics; this is currently encoded as ad-hoc logic.COMMAND-style introspectionCOMMAND GETKEYSandCOMMAND INFOcan become more precise for command families oncecmd|subis explicit.cmd|sub.3. Goals and Non-goals
3.1 Goals
Subcommandabstraction in the command system.cmdorcmd|sub) for system consumers.REDIS_REGISTER_COMMANDS) and existing command behavior.3.2 Non-goals
Commanderclasses immediately.4. Proposed Design
4.1 Terminology
root command: the top-level command name (e.g.config)subcommand: the subcommand identifier within a command family (e.g.get)canonical full name:root|sub(e.g.config|get)Canonical names are internal identities used for:
+cmd|sub)4.2 Data Model (Registration-Time Metadata)
The design is intentionally incremental: it reuses existing
CommandAttributesas the base metadata record, and addssubcommand-family metadata:
root command->SubcommandFamilySubcommandFamilycontains:SubcommandResolver: how to locate/interpret the sub token from argvsubcommand table:sub->CommandAttributes*for that subcommand4.2.1 SubcommandResolver
Not every family uses
argv[1]:COMMAND <sub> ...=> sub atargv[1]XGROUP <key> <sub> ...=> sub atargv[2]The resolver must be command-family-specific, but centrally registered.
4.3 ResolvedCommand (Runtime Output)
Introduce a framework-level
ResolvedCommandresult that can be used across subsystems:root(lowercased)sub(lowercased)attributes(the effectiveCommandAttributes*used for arity/flags/key extraction)FullName()(computed asroot|subwhensubis present)4.4 Dispatch Model (Two modes)
To avoid requiring large refactors, the framework supports two modes during migration:
root|subfor identity/metadata purposes.root|suband directly instantiates the handler registered for that subcommand.Both are compatible with the same registry and resolution logic.
5. Compatibility and Impact on Existing Code
5.1 Existing command registration remains valid
REDIS_REGISTER_COMMANDS(...)registrations continue to work.5.2 Performance expectations
The runtime overhead should be bounded:
ResolvedCommandin the request loop to avoid repeated resolution.6. Incremental Migration Plan
Phase 1: Framework scaffolding (no behavior changes)
src/commands/.ResolvedCommandandCommandTable::Resolve(...).Serverand keep the existing string-based overload.Phase 2: Dual-path resolution
Connection::ExecuteCommandsto use argv-based lookup.compatbehavior until opted-in.Phase 3: Opt-in families
For each command family (e.g.
COMMAND,CLIENT,CONFIG,CLUSTER, laterACLin downstream branches):7. Example (Illustrative; not required in this change)
Example canonical names:
command|count,command|getkeys,command|infoconfig|get,config|set,config|rewrite8. Code Touch Points
This section is intended for reviewers. It lists concrete code locations in upstream and the minimal set of changes
needed to introduce this abstraction (without requiring command-family refactors).
8.1 New code to add (framework-level)
Add a subcommand registry implementation under
src/commands/src/commands/subcommand_registry.hsrc/commands/subcommand_registry.ccSubcommandResolvertype that operates onstd::vector<std::string>argv.SubcommandFamilywith:std::map<std::string, const CommandAttributes*>(orunordered_mapif preferred)RegisterFamily(parent, resolver)RegisterSubcommand(parent, sub, attributes*)GetFamily(parent)/LookupSubcommand(parent, sub)Add a
ResolvedCommandrepresentationsrc/commands/commander.h(type declaration nearCommandAttributes)root, optionalsub, and resolvedconst CommandAttributes*.FullName()computed on demand (do not allocate on hot path).8.2 Modify existing code (command registration and resolution)
Extend
CommandTablewith a resolution helpersrc/commands/commander.hsrc/commands/commander.ccCommandTable::Resolve(const std::vector<std::string> &cmd_tokens)returningResolvedCommand.CommandTable::Get()(as today).subsubexists and is registered, return its attributesAdd a subcommand registration API/macro (optional but recommended for ergonomics)
src/commands/commander.hREDIS_REGISTER_COMMANDS:REDIS_REGISTER_SUBCOMMANDS(parent, ...)MakeCmdAttr<T>:MakeSubCmdAttr<T>(parent, sub, ...)CommandAttributes::nameto the canonical full name (parent|sub) so the attributescarry a stable identity usable by system consumers.
8.3 Modify existing code (core dispatch path)
Add argv-based lookup in
Serversrc/server/server.hsrc/server/server.ccStatusOr<std::unique_ptr<redis::Commander>> LookupAndCreateCommand(const std::vector<std::string> &cmd_tokens);LookupAndCreateCommand(const std::string &cmd_name)CommandTable::Resolve(cmd_tokens)to obtain effectiveCommandAttributes*.attributes->factory()andSetAttributes(attributes).Switch request execution to argv-based lookup
src/server/redis_connection.ccConnection::ExecuteCommandscurrently callsServer::LookupAndCreateCommand(cmd_tokens.front()).Server::LookupAndCreateCommand(cmd_tokens).Beta Was this translation helpful? Give feedback.
All reactions