chore: allow sampling of topk hottest keys. #4649
Conversation
romange
force-pushed
the
Pr6
branch
2 times, most recently
from
045730a to
71a591f
Compare
The core data structure code was added long time ago, now we allow using it via `DEBUG TOPK ON/OFF` subcommand. Signed-off-by: Roman Gershman <roman@dragonflydb.io>
| @@ -584,6 +587,10 @@ void DebugCmd::Run(CmdArgList args, facade::SinkReplyBuilder* builder) { | |||
| return RecvSize(ArgS(args, 1), builder); | |||
| } | |||
|
|
|||
| if (subcmd == "TOPK" && args.size() >= 2) { | |||
There was a problem hiding this comment.
One of the things I don't really like about how we implement subcommands, is that we miss basic arity checks and they are also orthogonal to our ACL's (subcommands of ACL"s are actual commands stored in the registry as opposed to what we have here). Long time now I wanted to check if we can simplify this, but never got a chance to actually check this.
Not important just thinking out loud.
There was a problem hiding this comment.
I thought maybe we can provide parse function for every command and split the code of parsing and execution of the command
| public: | ||
| struct Options { | ||
| // HeavyKeeper options | ||
| uint64_t buckets = 1 << 16; | ||
| uint64_t arrays = 4; | ||
| uint32_t buckets = 1 << 16; |
There was a problem hiding this comment.
Why do you use sometimes uint64 for buckets and sometimes uint32
src/server/table.cc
Outdated
| if (top_keys) { | ||
| delete top_keys; | ||
| } |
There was a problem hiding this comment.
nit you don't need if(top_keys) delete nullptr is correct statement
| auto* rb = static_cast<RedisReplyBuilder*>(builder); | ||
| DCHECK_GE(args.size(), 1u); | ||
|
|
||
| string_view subcmd = ArgS(args, 0); |
There was a problem hiding this comment.
what about cmdArgParser
There was a problem hiding this comment.
we can give it as a starter task to someone
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
The core data structure code was added long time ago, now we allow using it via
DEBUG TOPK ON/OFFsubcommand.