Unverified Commit 10bbeb68 authored by guybe7's avatar guybe7 Committed by GitHub
Browse files

Add command tips to COMMAND DOCS (#10104)

Adding command tips (see https://redis.io/topics/command-tips) to commands.

Breaking changes:
1. Removed the "random" and "sort_for_script" flags. They are now command tips.
(this isn't affecting redis behavior since #9812, but could affect some client applications
that's relying on COMMAND command flags)

Summary of changes:
1. add BLOCKING flag (new flag) for all commands that could block. The ACL category with
  the same name is now implicit.
2. move RANDOM flag to a `nondeterministic_output` tip
3. move SORT_FOR_SCRIPT flag to `nondeterministic_output_order` tip
3. add REQUEST_POLICY and RESPONSE_POLICY where appropriate as documented in the tips
4. deprecate (ignore) the `random` flag for RM_CreateCommand

Other notes:
1. Proxies need to send `RANDOMKEY` to all shards and then select one key randomly.
  The other option is to pick a random shard and transfer `RANDOMKEY `to it, but that scheme
  fails if this specific shard is empty
2. Remove CMD_RANDOM from `XACK` (i.e. XACK does not have RANDOM_OUTPUT)
   It was added in 9e4fb96c, I guess by mistake.
   Also from `(P)EXPIRETIME` (new command, was flagged "random" by mistake).
3. Add `nondeterministic_output` to `OBJECT ENCODING` (for the same reason `XTRIM` has it:
   the reply may differ depending on the internal representation in memory)
4. RANDOM on `HGETALL` was wrong (there due to a limitation of the old script sorting logic), now
  it's `nondeterministic_output_order`
5. Unrelated: Hide CMD_PROTECTED from COMMAND
parent 9c602922
...@@ -13,12 +13,14 @@ ...@@ -13,12 +13,14 @@
] ]
], ],
"command_flags": [ "command_flags": [
"WRITE", "WRITE"
"RANDOM"
], ],
"acl_categories": [ "acl_categories": [
"STREAM" "STREAM"
], ],
"command_tips": [
"NONDETERMINISTIC_OUTPUT"
],
"key_specs": [ "key_specs": [
{ {
"flags": [ "flags": [
......
...@@ -7,12 +7,14 @@ ...@@ -7,12 +7,14 @@
"arity": -2, "arity": -2,
"function": "zrandmemberCommand", "function": "zrandmemberCommand",
"command_flags": [ "command_flags": [
"READONLY", "READONLY"
"RANDOM"
], ],
"acl_categories": [ "acl_categories": [
"SORTEDSET" "SORTEDSET"
], ],
"command_tips": [
"NONDETERMINISTIC_OUTPUT"
],
"key_specs": [ "key_specs": [
{ {
"flags": [ "flags": [
......
...@@ -7,12 +7,14 @@ ...@@ -7,12 +7,14 @@
"arity": -3, "arity": -3,
"function": "zscanCommand", "function": "zscanCommand",
"command_flags": [ "command_flags": [
"READONLY", "READONLY"
"RANDOM"
], ],
"acl_categories": [ "acl_categories": [
"SORTEDSET" "SORTEDSET"
], ],
"command_tips": [
"NONDETERMINISTIC_OUTPUT"
],
"key_specs": [ "key_specs": [
{ {
"flags": [ "flags": [
......
...@@ -831,7 +831,8 @@ int64_t commandFlagsFromString(char *s) { ...@@ -831,7 +831,8 @@ int64_t commandFlagsFromString(char *s) {
else if (!strcasecmp(t,"deny-script")) flags |= CMD_NOSCRIPT; else if (!strcasecmp(t,"deny-script")) flags |= CMD_NOSCRIPT;
else if (!strcasecmp(t,"allow-loading")) flags |= CMD_LOADING; else if (!strcasecmp(t,"allow-loading")) flags |= CMD_LOADING;
else if (!strcasecmp(t,"pubsub")) flags |= CMD_PUBSUB; else if (!strcasecmp(t,"pubsub")) flags |= CMD_PUBSUB;
else if (!strcasecmp(t,"random")) flags |= CMD_RANDOM; else if (!strcasecmp(t,"random")) { /* Deprecated. Silently ignore. */ }
else if (!strcasecmp(t,"blocking")) flags |= CMD_BLOCKING;
else if (!strcasecmp(t,"allow-stale")) flags |= CMD_STALE; else if (!strcasecmp(t,"allow-stale")) flags |= CMD_STALE;
else if (!strcasecmp(t,"no-monitor")) flags |= CMD_SKIP_MONITOR; else if (!strcasecmp(t,"no-monitor")) flags |= CMD_SKIP_MONITOR;
else if (!strcasecmp(t,"no-slowlog")) flags |= CMD_SKIP_SLOWLOG; else if (!strcasecmp(t,"no-slowlog")) flags |= CMD_SKIP_SLOWLOG;
...@@ -913,6 +914,9 @@ RedisModuleCommand *moduleCreateCommandProxy(struct RedisModule *module, const c ...@@ -913,6 +914,9 @@ RedisModuleCommand *moduleCreateCommandProxy(struct RedisModule *module, const c
* * **"pubsub"**: The command publishes things on Pub/Sub channels. * * **"pubsub"**: The command publishes things on Pub/Sub channels.
* * **"random"**: The command may have different outputs even starting * * **"random"**: The command may have different outputs even starting
* from the same input arguments and key values. * from the same input arguments and key values.
* Starting from Redis 7.0 this flag has been deprecated.
* Declaring a command as "random" can be done using
* command tips, see https://redis.io/topics/command-tips.
* * **"allow-stale"**: The command is allowed to run on slaves that don't * * **"allow-stale"**: The command is allowed to run on slaves that don't
* serve stale data. Don't use if you don't know what * serve stale data. Don't use if you don't know what
* this means. * this means.
...@@ -938,6 +942,7 @@ RedisModuleCommand *moduleCreateCommandProxy(struct RedisModule *module, const c ...@@ -938,6 +942,7 @@ RedisModuleCommand *moduleCreateCommandProxy(struct RedisModule *module, const c
* * **"may-replicate"**: This command may generate replication traffic, even * * **"may-replicate"**: This command may generate replication traffic, even
* though it's not a write command. * though it's not a write command.
* * **"no-mandatory-keys"**: All the keys this command may take are optional * * **"no-mandatory-keys"**: All the keys this command may take are optional
* * **"blocking"**: The command has the potential to block the client.
* * **"allow-busy"**: Permit the command while the server is blocked either by * * **"allow-busy"**: Permit the command while the server is blocked either by
* a script or by a slow module command, see * a script or by a slow module command, see
* RM_Yield. * RM_Yield.
...@@ -9955,8 +9960,8 @@ void moduleUnregisterCommands(struct RedisModule *module) { ...@@ -9955,8 +9960,8 @@ void moduleUnregisterCommands(struct RedisModule *module) {
if (cp->module == module) { if (cp->module == module) {
if (cmd->key_specs != cmd->key_specs_static) if (cmd->key_specs != cmd->key_specs_static)
zfree(cmd->key_specs); zfree(cmd->key_specs);
for (int j = 0; cmd->hints && cmd->hints[j]; j++) for (int j = 0; cmd->tips && cmd->tips[j]; j++)
sdsfree((sds)cmd->hints[j]); sdsfree((sds)cmd->tips[j]);
for (int j = 0; cmd->history && cmd->history[j].since; j++) { for (int j = 0; cmd->history && cmd->history[j].since; j++) {
sdsfree((sds)cmd->history[j].since); sdsfree((sds)cmd->history[j].since);
sdsfree((sds)cmd->history[j].changes); sdsfree((sds)cmd->history[j].changes);
......
...@@ -2632,6 +2632,8 @@ void setImplictACLCategories(struct redisCommand *c) { ...@@ -2632,6 +2632,8 @@ void setImplictACLCategories(struct redisCommand *c) {
c->acl_categories |= ACL_CATEGORY_PUBSUB; c->acl_categories |= ACL_CATEGORY_PUBSUB;
if (c->flags & CMD_FAST) if (c->flags & CMD_FAST)
c->acl_categories |= ACL_CATEGORY_FAST; c->acl_categories |= ACL_CATEGORY_FAST;
if (c->flags & CMD_BLOCKING)
c->acl_categories |= ACL_CATEGORY_BLOCKING;
/* If it's not @fast is @slow in this binary world. */ /* If it's not @fast is @slow in this binary world. */
if (!(c->acl_categories & ACL_CATEGORY_FAST)) if (!(c->acl_categories & ACL_CATEGORY_FAST))
...@@ -2671,8 +2673,8 @@ void populateCommandStructure(struct redisCommand *c) { ...@@ -2671,8 +2673,8 @@ void populateCommandStructure(struct redisCommand *c) {
/* Count things so we don't have to use deferred reply in COMMAND reply. */ /* Count things so we don't have to use deferred reply in COMMAND reply. */
while (c->history && c->history[c->num_history].since) while (c->history && c->history[c->num_history].since)
c->num_history++; c->num_history++;
while (c->hints && c->hints[c->num_hints]) while (c->tips && c->tips[c->num_tips])
c->num_hints++; c->num_tips++;
c->num_args = populateArgsStructure(c->args); c->num_args = populateArgsStructure(c->args);
populateCommandLegacyRangeSpec(c); populateCommandLegacyRangeSpec(c);
...@@ -4099,8 +4101,7 @@ void addReplyFlagsForCommand(client *c, struct redisCommand *cmd) { ...@@ -4099,8 +4101,7 @@ void addReplyFlagsForCommand(client *c, struct redisCommand *cmd) {
{CMD_ADMIN, "admin"}, {CMD_ADMIN, "admin"},
{CMD_PUBSUB, "pubsub"}, {CMD_PUBSUB, "pubsub"},
{CMD_NOSCRIPT, "noscript"}, {CMD_NOSCRIPT, "noscript"},
{CMD_RANDOM, "random"}, {CMD_BLOCKING, "blocking"},
{CMD_SORT_FOR_SCRIPT, "sort_for_script"},
{CMD_LOADING, "loading"}, {CMD_LOADING, "loading"},
{CMD_STALE, "stale"}, {CMD_STALE, "stale"},
{CMD_SKIP_MONITOR, "skip_monitor"}, {CMD_SKIP_MONITOR, "skip_monitor"},
...@@ -4109,15 +4110,16 @@ void addReplyFlagsForCommand(client *c, struct redisCommand *cmd) { ...@@ -4109,15 +4110,16 @@ void addReplyFlagsForCommand(client *c, struct redisCommand *cmd) {
{CMD_FAST, "fast"}, {CMD_FAST, "fast"},
{CMD_NO_AUTH, "no_auth"}, {CMD_NO_AUTH, "no_auth"},
{CMD_MAY_REPLICATE, "may_replicate"}, {CMD_MAY_REPLICATE, "may_replicate"},
/* {CMD_SENTINEL, "sentinel"}, Hidden on purpose */
/* {CMD_ONLY_SENTINEL, "only_sentinel"}, Hidden on purpose */
{CMD_NO_MANDATORY_KEYS, "no_mandatory_keys"}, {CMD_NO_MANDATORY_KEYS, "no_mandatory_keys"},
{CMD_PROTECTED, "protected"}, /* {CMD_PROTECTED, "protected"}, Hidden on purpose */
{CMD_NO_ASYNC_LOADING, "no_async_loading"}, {CMD_NO_ASYNC_LOADING, "no_async_loading"},
{CMD_NO_MULTI, "no_multi"}, {CMD_NO_MULTI, "no_multi"},
{CMD_MOVABLE_KEYS, "movablekeys"}, {CMD_MOVABLE_KEYS, "movablekeys"},
{CMD_ALLOW_BUSY, "allow_busy"}, {CMD_ALLOW_BUSY, "allow_busy"},
{0,NULL} {0,NULL}
}; };
/* "sentinel" and "only-sentinel" are hidden on purpose. */
addReplyCommandFlags(c, cmd->flags, flagNames); addReplyCommandFlags(c, cmd->flags, flagNames);
} }
...@@ -4251,10 +4253,10 @@ void addReplyCommandHistory(client *c, struct redisCommand *cmd) { ...@@ -4251,10 +4253,10 @@ void addReplyCommandHistory(client *c, struct redisCommand *cmd) {
} }
} }
void addReplyCommandHints(client *c, struct redisCommand *cmd) { void addReplyCommandTips(client *c, struct redisCommand *cmd) {
addReplySetLen(c, cmd->num_hints); addReplySetLen(c, cmd->num_tips);
for (int j = 0; j<cmd->num_hints; j++) { for (int j = 0; j<cmd->num_tips; j++) {
addReplyBulkCString(c, cmd->hints[j]); addReplyBulkCString(c, cmd->tips[j]);
} }
} }
...@@ -4415,7 +4417,7 @@ void addReplyCommandInfo(client *c, struct redisCommand *cmd) { ...@@ -4415,7 +4417,7 @@ void addReplyCommandInfo(client *c, struct redisCommand *cmd) {
addReplyLongLong(c, lastkey); addReplyLongLong(c, lastkey);
addReplyLongLong(c, keystep); addReplyLongLong(c, keystep);
addReplyCommandCategories(c, cmd); addReplyCommandCategories(c, cmd);
addReplyCommandHints(c, cmd); addReplyCommandTips(c, cmd);
addReplyCommandKeySpecs(c, cmd); addReplyCommandKeySpecs(c, cmd);
addReplyCommandSubCommands(c, cmd, addReplyCommandInfo, 0); addReplyCommandSubCommands(c, cmd, addReplyCommandInfo, 0);
} }
......
...@@ -190,8 +190,7 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT]; ...@@ -190,8 +190,7 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT];
#define CMD_ADMIN (1ULL<<4) #define CMD_ADMIN (1ULL<<4)
#define CMD_PUBSUB (1ULL<<5) #define CMD_PUBSUB (1ULL<<5)
#define CMD_NOSCRIPT (1ULL<<6) #define CMD_NOSCRIPT (1ULL<<6)
#define CMD_RANDOM (1ULL<<7) #define CMD_BLOCKING (1ULL<<8) /* Has potential to block. */
#define CMD_SORT_FOR_SCRIPT (1ULL<<8)
#define CMD_LOADING (1ULL<<9) #define CMD_LOADING (1ULL<<9)
#define CMD_STALE (1ULL<<10) #define CMD_STALE (1ULL<<10)
#define CMD_SKIP_MONITOR (1ULL<<11) #define CMD_SKIP_MONITOR (1ULL<<11)
...@@ -2118,14 +2117,7 @@ typedef int redisGetKeysProc(struct redisCommand *cmd, robj **argv, int argc, ge ...@@ -2118,14 +2117,7 @@ typedef int redisGetKeysProc(struct redisCommand *cmd, robj **argv, int argc, ge
* *
* CMD_NOSCRIPT: Command not allowed in scripts. * CMD_NOSCRIPT: Command not allowed in scripts.
* *
* CMD_RANDOM: Random command. Command is not deterministic, that is, the same * CMD_BLOCKING: The command has the potential to block the client.
* command with the same arguments, with the same key space, may
* have different results. For instance SPOP and RANDOMKEY are
* two random commands.
*
* CMD_SORT_FOR_SCRIPT: Sort command output array if called from script, so that the
* output is deterministic. When this flag is used (not always
* possible), then the "random" flag is not needed.
* *
* CMD_LOADING: Allow the command while loading the database. * CMD_LOADING: Allow the command while loading the database.
* *
...@@ -2198,7 +2190,7 @@ struct redisCommand { ...@@ -2198,7 +2190,7 @@ struct redisCommand {
const char *deprecated_since; /* In case the command is deprecated, when did it happen? */ const char *deprecated_since; /* In case the command is deprecated, when did it happen? */
redisCommandGroup group; /* Command group */ redisCommandGroup group; /* Command group */
commandHistory *history; /* History of the command */ commandHistory *history; /* History of the command */
const char **hints; /* An array of strings that are meant o be hints for clients/proxies regarding this command */ const char **tips; /* An array of strings that are meant to be tips for clients/proxies regarding this command */
redisCommandProc *proc; /* Command implementation */ redisCommandProc *proc; /* Command implementation */
int arity; /* Number of arguments, it is possible to use -N to say >= N */ int arity; /* Number of arguments, it is possible to use -N to say >= N */
uint64_t flags; /* Command flags, see CMD_*. */ uint64_t flags; /* Command flags, see CMD_*. */
...@@ -2228,7 +2220,7 @@ struct redisCommand { ...@@ -2228,7 +2220,7 @@ struct redisCommand {
* COMMAND INFO and COMMAND GETKEYS */ * COMMAND INFO and COMMAND GETKEYS */
int num_args; int num_args;
int num_history; int num_history;
int num_hints; int num_tips;
int key_specs_num; int key_specs_num;
int key_specs_max; int key_specs_max;
dict *subcommands_dict; dict *subcommands_dict;
......
...@@ -215,8 +215,8 @@ class Command(object): ...@@ -215,8 +215,8 @@ class Command(object):
def history_table_name(self): def history_table_name(self):
return "%s_History" % (self.fullname().replace(" ", "_")) return "%s_History" % (self.fullname().replace(" ", "_"))
def hints_table_name(self): def tips_table_name(self):
return "%s_Hints" % (self.fullname().replace(" ", "_")) return "%s_tips" % (self.fullname().replace(" ", "_"))
def arg_table_name(self): def arg_table_name(self):
return "%s_Args" % (self.fullname().replace(" ", "_")) return "%s_Args" % (self.fullname().replace(" ", "_"))
...@@ -233,19 +233,19 @@ class Command(object): ...@@ -233,19 +233,19 @@ class Command(object):
s += "{0}" s += "{0}"
return s return s
def hints_code(self): def tips_code(self):
if not self.desc.get("hints"): if not self.desc.get("command_tips"):
return "" return ""
s = "" s = ""
for hint in self.desc["hints"].split(' '): for hint in self.desc["command_tips"]:
s += "\"%s\",\n" % hint s += "\"%s\",\n" % hint.lower()
s += "NULL" s += "NULL"
return s return s
def struct_code(self): def struct_code(self):
""" """
Output example: Output example:
"set","Set the string value of a key","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_STRING,SET_History,SET_Hints,setCommand,-3,"write denyoom @string",{{"write read",KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}}},.args=SET_Args "set","Set the string value of a key","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_STRING,SET_History,SET_tips,setCommand,-3,"write denyoom @string",{{"write read",KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}}},.args=SET_Args
""" """
def _flags_code(): def _flags_code():
...@@ -282,7 +282,7 @@ class Command(object): ...@@ -282,7 +282,7 @@ class Command(object):
get_optional_desc_string(self.desc, "deprecated_since"), get_optional_desc_string(self.desc, "deprecated_since"),
GROUPS[self.group], GROUPS[self.group],
self.history_table_name(), self.history_table_name(),
self.hints_table_name(), self.tips_table_name(),
self.desc.get("function", "NULL"), self.desc.get("function", "NULL"),
self.desc["arity"], self.desc["arity"],
_flags_code(), _flags_code(),
...@@ -328,14 +328,14 @@ class Command(object): ...@@ -328,14 +328,14 @@ class Command(object):
else: else:
f.write("#define %s NULL\n\n" % self.history_table_name()) f.write("#define %s NULL\n\n" % self.history_table_name())
f.write("/* %s hints */\n" % self.fullname()) f.write("/* %s tips */\n" % self.fullname())
code = self.hints_code() code = self.tips_code()
if code: if code:
f.write("const char *%s[] = {\n" % self.hints_table_name()) f.write("const char *%s[] = {\n" % self.tips_table_name())
f.write("%s\n" % code) f.write("%s\n" % code)
f.write("};\n\n") f.write("};\n\n")
else: else:
f.write("#define %s NULL\n\n" % self.hints_table_name()) f.write("#define %s NULL\n\n" % self.tips_table_name())
if self.args: if self.args:
for arg in self.args: for arg in self.args:
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment