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

Define and document command tips #9876

Closed
guybe7 opened this issue Dec 1, 2021 · 1 comment · Fixed by #10104
Closed

Define and document command tips #9876

guybe7 opened this issue Dec 1, 2021 · 1 comment · Fixed by #10104
Assignees

Comments

@guybe7
Copy link
Collaborator

guybe7 commented Dec 1, 2021

See #9656

We need to define and document general command hints that can help clients/proxies know how to behave.

Sketch

Simple examples (for proxies):

  • PING should probably be sent to all shards
  • DBSIZE should be sent to all shards and be aggregated as a sum

Non-deterministic commands:
Since EVAL is no longer propagated as is (see #9812), redis no longer uses the random and to-sort flags, and they should be moved to client hints like so:

  • random-output
  • random-order-output (HGETALL should be here, not in random-output)
  • the old COMMAND command flags (i.e. sort_for_script) will be gone (breaking change in some way)

Modules:

  • RM_CreateCommand (or alternative) should be able to set that flag (same as it sets the no-monitor flag and others)

other flags

Final list of tips

boolean

  • blocking: if found, this command has the potential to block (but not always, see block_keyword below)
  • random-output: the output of the command is random (e.g. RANDOMKEY, SPOP)
  • random-order-output: the output is not random, but comes in random order (e.g. HGETALL, SMEMBERS)

key-value

  • block_keyword: identify commands which the potential to block: the ACL category blocking is too loose: XREAD and other commands have it, but they will only block if a specific arg was given (usually BLOCK <time>). it could be useful to add something like block_keyword:<word>, which the proxy will search in argv to determine if the command may indeed block.
    we could have a problem if that keyword is a keyname etc. but we can just define it as best-effort - there's probably no harm in mistaking a non-blocking command with a blocking one (but there is the harm in mistaking in the other direction).
  • request_policy: there are two implicit cases: 1) command goes to one shard, according to the has slots of the key(s). 2) command doesn't have keys, goes to some shard (probably the first). otherwise, we need to split the command into several shards. a partial list of options:
    • ALL_SHARDS - Forward to all shards (PING). The operation is atomic on all shards.
    • FEW_SHARDS - Forward to several shards, used by multi-key commands (MSET, MGET, DEL, etc.). The operation is atomic on relevant shards only.
  • reply_policy: the default here is to append all replies, if there are no keys, order doesn't matter (KEYS), otherwise order by the order of keys in request (MGET). how do we aggregate commands that are sent to multiple shards? a partial list of options:
    • ALL_SUCCEEDED - Return OK if all shards responded OK. Examples: CONFIG SET, SCRIPT FLUSH
    • ONE_SUCCEEDED - Return OK if at least one shard responded OK. Example: SCRIPT KILL. Used by SCRIPT LOAD. This is meant to return the script’s SHA1 digest, which can be done based on any of the shards' replies.
    • AGG_LOGICAL_AND - Relevant for SCRIPT EXISTS, which returns an array of 0/1 indicating which of the provided scripts exist
    • AGG_LOGICAL_OR - For symmetry
    • AGG_MIN - Used by WAIT. Returns the lowest number among the ones the shards return
    • AGG_MAX - For symmetry
    • AGG_SUM - Sums the integer values returned by the shards. Example: DBSIZE

docs PR: redis/redis-doc#1697 (probably the more up-to-date than the description above)

@oranagra oranagra added this to Backlog in 7.0 via automation Dec 1, 2021
@oranagra oranagra moved this from Backlog to To Do in 7.0 Dec 1, 2021
@oranagra oranagra added 7.0-must-have state:major-decision Requires core team consensus labels Dec 1, 2021
@oranagra
Copy link
Member

oranagra commented Dec 8, 2021

oranagra pushed a commit that referenced this issue Dec 15, 2021
Delete the hardcoded command table and replace it with an auto-generated table, based
on a JSON file that describes the commands (each command must have a JSON file).

These JSON files are the SSOT of everything there is to know about Redis commands,
and it is reflected fully in COMMAND INFO.

These JSON files are used to generate commands.c (using a python script), which is then
committed to the repo and compiled.

The purpose is:
* Clients and proxies will be able to get much more info from redis, instead of relying on hard coded logic.
* drop the dependency between Redis-user and the commands.json in redis-doc.
* delete help.h and have redis-cli learn everything it needs to know just by issuing COMMAND (will be
  done in a separate PR)
* redis.io should stop using commands.json and learn everything from Redis (ultimately one of the release
  artifacts should be a large JSON, containing all the information about all of the commands, which will be
  generated from COMMAND's reply)
* the byproduct of this is:
  * module commands will be able to provide that info and possibly be more of a first-class citizens
  * in theory, one may be able to generate a redis client library for a strictly typed language, by using this info.

### Interface changes

#### COMMAND INFO's reply change (and arg-less COMMAND)

Before this commit the reply at index 7 contained the key-specs list
and reply at index 8 contained the sub-commands list (Both unreleased).
Now, reply at index 7 is a map of:
- summary - short command description
- since - debut version
- group - command group
- complexity - complexity string
- doc-flags - flags used for documentation (e.g. "deprecated")
- deprecated-since - if deprecated, from which version?
- replaced-by - if deprecated, which command replaced it?
- history - a list of (version, what-changed) tuples
- hints - a list of strings, meant to provide hints for clients/proxies. see #9876
- arguments - an array of arguments. each element is a map, with the possibility of nesting (sub-arguments)
- key-specs - an array of keys specs (already in unstable, just changed location)
- subcommands - a list of sub-commands (already in unstable, just changed location)
- reply-schema - will be added in the future (see #9845)

more details on these can be found in redis/redis-doc#1697

only the first three fields are mandatory 

#### API changes (unreleased API obviously)

now they take RedisModuleCommand opaque pointer instead of looking up the command by name

- RM_CreateSubcommand
- RM_AddCommandKeySpec
- RM_SetCommandKeySpecBeginSearchIndex
- RM_SetCommandKeySpecBeginSearchKeyword
- RM_SetCommandKeySpecFindKeysRange
- RM_SetCommandKeySpecFindKeysKeynum

Currently, we did not add module API to provide additional information about their commands because
we couldn't agree on how the API should look like, see #9944.

### Somehow related changes
1. Literals should be in uppercase while placeholder in lowercase. Now all the GEO* command
   will be documented with M|KM|FT|MI and can take both lowercase and uppercase

### Unrelated changes
1. Bugfix: no_madaory_keys was absent in COMMAND's reply
2. expose CMD_MODULE as "module" via COMMAND
3. have a dedicated uint64 for ACL categories (instead of having them in the same uint64 as command flags)

Co-authored-by: Itamar Haber <itamar@garantiadata.com>
hwware pushed a commit to hwware/redis that referenced this issue Dec 20, 2021
Delete the hardcoded command table and replace it with an auto-generated table, based
on a JSON file that describes the commands (each command must have a JSON file).

These JSON files are the SSOT of everything there is to know about Redis commands,
and it is reflected fully in COMMAND INFO.

These JSON files are used to generate commands.c (using a python script), which is then
committed to the repo and compiled.

The purpose is:
* Clients and proxies will be able to get much more info from redis, instead of relying on hard coded logic.
* drop the dependency between Redis-user and the commands.json in redis-doc.
* delete help.h and have redis-cli learn everything it needs to know just by issuing COMMAND (will be
  done in a separate PR)
* redis.io should stop using commands.json and learn everything from Redis (ultimately one of the release
  artifacts should be a large JSON, containing all the information about all of the commands, which will be
  generated from COMMAND's reply)
* the byproduct of this is:
  * module commands will be able to provide that info and possibly be more of a first-class citizens
  * in theory, one may be able to generate a redis client library for a strictly typed language, by using this info.

### Interface changes

#### COMMAND INFO's reply change (and arg-less COMMAND)

Before this commit the reply at index 7 contained the key-specs list
and reply at index 8 contained the sub-commands list (Both unreleased).
Now, reply at index 7 is a map of:
- summary - short command description
- since - debut version
- group - command group
- complexity - complexity string
- doc-flags - flags used for documentation (e.g. "deprecated")
- deprecated-since - if deprecated, from which version?
- replaced-by - if deprecated, which command replaced it?
- history - a list of (version, what-changed) tuples
- hints - a list of strings, meant to provide hints for clients/proxies. see redis#9876
- arguments - an array of arguments. each element is a map, with the possibility of nesting (sub-arguments)
- key-specs - an array of keys specs (already in unstable, just changed location)
- subcommands - a list of sub-commands (already in unstable, just changed location)
- reply-schema - will be added in the future (see redis#9845)

more details on these can be found in redis/redis-doc#1697

only the first three fields are mandatory 

#### API changes (unreleased API obviously)

now they take RedisModuleCommand opaque pointer instead of looking up the command by name

- RM_CreateSubcommand
- RM_AddCommandKeySpec
- RM_SetCommandKeySpecBeginSearchIndex
- RM_SetCommandKeySpecBeginSearchKeyword
- RM_SetCommandKeySpecFindKeysRange
- RM_SetCommandKeySpecFindKeysKeynum

Currently, we did not add module API to provide additional information about their commands because
we couldn't agree on how the API should look like, see redis#9944.

### Somehow related changes
1. Literals should be in uppercase while placeholder in lowercase. Now all the GEO* command
   will be documented with M|KM|FT|MI and can take both lowercase and uppercase

### Unrelated changes
1. Bugfix: no_madaory_keys was absent in COMMAND's reply
2. expose CMD_MODULE as "module" via COMMAND
3. have a dedicated uint64 for ACL categories (instead of having them in the same uint64 as command flags)

Co-authored-by: Itamar Haber <itamar@garantiadata.com>
@guybe7 guybe7 changed the title Define and document command hints Define and document command tips Jan 11, 2022
@oranagra oranagra removed this from To Do in 7.0 Jan 14, 2022
@oranagra oranagra removed state:major-decision Requires core team consensus 7.0-must-have labels Jan 14, 2022
panjf2000 pushed a commit to panjf2000/redis that referenced this issue Feb 3, 2022
Delete the hardcoded command table and replace it with an auto-generated table, based
on a JSON file that describes the commands (each command must have a JSON file).

These JSON files are the SSOT of everything there is to know about Redis commands,
and it is reflected fully in COMMAND INFO.

These JSON files are used to generate commands.c (using a python script), which is then
committed to the repo and compiled.

The purpose is:
* Clients and proxies will be able to get much more info from redis, instead of relying on hard coded logic.
* drop the dependency between Redis-user and the commands.json in redis-doc.
* delete help.h and have redis-cli learn everything it needs to know just by issuing COMMAND (will be
  done in a separate PR)
* redis.io should stop using commands.json and learn everything from Redis (ultimately one of the release
  artifacts should be a large JSON, containing all the information about all of the commands, which will be
  generated from COMMAND's reply)
* the byproduct of this is:
  * module commands will be able to provide that info and possibly be more of a first-class citizens
  * in theory, one may be able to generate a redis client library for a strictly typed language, by using this info.

### Interface changes

#### COMMAND INFO's reply change (and arg-less COMMAND)

Before this commit the reply at index 7 contained the key-specs list
and reply at index 8 contained the sub-commands list (Both unreleased).
Now, reply at index 7 is a map of:
- summary - short command description
- since - debut version
- group - command group
- complexity - complexity string
- doc-flags - flags used for documentation (e.g. "deprecated")
- deprecated-since - if deprecated, from which version?
- replaced-by - if deprecated, which command replaced it?
- history - a list of (version, what-changed) tuples
- hints - a list of strings, meant to provide hints for clients/proxies. see redis#9876
- arguments - an array of arguments. each element is a map, with the possibility of nesting (sub-arguments)
- key-specs - an array of keys specs (already in unstable, just changed location)
- subcommands - a list of sub-commands (already in unstable, just changed location)
- reply-schema - will be added in the future (see redis#9845)

more details on these can be found in redis/redis-doc#1697

only the first three fields are mandatory 

#### API changes (unreleased API obviously)

now they take RedisModuleCommand opaque pointer instead of looking up the command by name

- RM_CreateSubcommand
- RM_AddCommandKeySpec
- RM_SetCommandKeySpecBeginSearchIndex
- RM_SetCommandKeySpecBeginSearchKeyword
- RM_SetCommandKeySpecFindKeysRange
- RM_SetCommandKeySpecFindKeysKeynum

Currently, we did not add module API to provide additional information about their commands because
we couldn't agree on how the API should look like, see redis#9944.

### Somehow related changes
1. Literals should be in uppercase while placeholder in lowercase. Now all the GEO* command
   will be documented with M|KM|FT|MI and can take both lowercase and uppercase

### Unrelated changes
1. Bugfix: no_madaory_keys was absent in COMMAND's reply
2. expose CMD_MODULE as "module" via COMMAND
3. have a dedicated uint64 for ACL categories (instead of having them in the same uint64 as command flags)

Co-authored-by: Itamar Haber <itamar@garantiadata.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants