Add CLUSTER SLOT-STATS document. #150
Conversation
- Valkey PR link; valkey-io/valkey#351. Signed-off-by: Kyle Kim <[email protected]>
kyle-yh-kim
force-pushed
the
20-slotstats
branch
from
8ec5cbc
to
65d2006
Compare
commands.json
Outdated
| @@ -3095,6 +3095,87 @@ | |||
| "nondeterministic_output" | |||
| ] | |||
| }, | |||
| "CLUSTER SLOT-STATS": { | |||
There was a problem hiding this comment.
We no longer use this file AFAIK. We generate it dynamically on the fly.
There was a problem hiding this comment.
Question - if dynamically generated, how is the history section generated?
For example, for FLUSHDB;
"history": [
[
"4.0.0",
"Added the `ASYNC` flushing mode modifier."
],
[
"6.2.0",
"Added the `SYNC` flushing mode modifier."
]
],
There was a problem hiding this comment.
From the JSON file in the code repo.
- Removed commands.json changes. - Added ordering invariance within the markdown. Signed-off-by: Kyle Kim <[email protected]>
- Added RESP3 response. Signed-off-by: Kyle Kim <[email protected]>
- Added cpu, network-bytes-in and network-bytes-out metrics. - Added a tie breaker condition for ORDERBY argument. Signed-off-by: Kyle Kim <[email protected]>
Signed-off-by: Madelyn Olson <[email protected]>
| @@ -0,0 +1,99 @@ | |||
| `CLUSTER SLOT-STATS` returns an array of slot usage statistics for slots assigned to the current shard. | |||
There was a problem hiding this comment.
I just realize we kind of use metrics and statistics sort of interchangeably which now bothers me, but maybe it's fine.
There was a problem hiding this comment.
Agreed. We could consolidate the wording into “stats / statistics” throughout the documentation, given that SLOT-STATS is the chosen name for the command.
There was a problem hiding this comment.
The command name is the title of the page, so it doesn't need to be here.
| `CLUSTER SLOT-STATS` returns an array of slot usage statistics for slots assigned to the current shard. | |
| Returns an array of slot usage statistics for slots assigned to the current shard. |
|
@kyle-yh-kim Will you update this PR? It would be good to have docs for the feature. 8.0 is already released. |
- Consolidated metrics/statistics wording into statistics. - Remove verbosity. Signed-off-by: Kyle Kim <[email protected]>
|
Thanks for the reminder. All comments have been addressed, now awaiting for approval. |
| The `SLOTSRANGE` argument allows for request pagination. | ||
| The response is ordered in ascending slot number. | ||
|
|
||
| ##### Response in RESP2 |
There was a problem hiding this comment.
Level 5 heading under a level 3 heading?
So many levels are not easy to distinguish. The fontsizes will not be different enough. Not to mention the man pages. I think we should not use more than 3 levels.
Let's add an ## Examples heading and move the examples as level 3 headings under that?
I would like to loosely follow the order within the page that's normal for man pages, as described under "Sections within a manual page" on the man-pages.7 man page: https://www.man7.org/linux/man-pages/man7/man-pages.7.html
| ### ORDERBY | ||
| The `ORDERBY` argument returns an ordered slot statistics based on the specified statistic and sub-arguments to identify hot / cold slots across the cluster. In the event of a tie in the stats, ascending slot number is used as a tie breaker. | ||
|
|
||
| ##### Response in RESP2 | ||
| ``` | ||
| > CLUSTER SLOT-STATS ORDERBY KEY-COUNT LIMIT 2 DESC |
There was a problem hiding this comment.
Parameter documentation mixed with examples.
Let's put the parameter descriptions above under ## Options and all the examples below under ## Examples. See the page for the SET command for how options are documented.
| ## Supported filtering and ordering arguments | ||
| There exist two mutually exclusive arguments for controlling the output, namely; | ||
|
|
||
| ### SLOTSRANGE | ||
| Returns slot statistics based on the slots range provided. The range is inclusive. | ||
| The `SLOTSRANGE` argument allows for request pagination. | ||
| The response is ordered in ascending slot number. |
There was a problem hiding this comment.
The syntax is also inserted automatically above this text, from the JSON file. We should document the options. If they're mutually exclusive, the syntax will display this as < SLOTRANGE xxxx | ORDERBY yyy > so I don't think we need to mention that.
| ## Supported filtering and ordering arguments | |
| There exist two mutually exclusive arguments for controlling the output, namely; | |
| ### SLOTSRANGE | |
| Returns slot statistics based on the slots range provided. The range is inclusive. | |
| The `SLOTSRANGE` argument allows for request pagination. | |
| The response is ordered in ascending slot number. | |
| ## Options | |
| * `SLOTSRANGE`: Returns slot statistics based on the slots range provided. | |
| The range is inclusive. | |
| The `SLOTSRANGE` argument allows for request pagination. | |
| The response is ordered in ascending slot number. |
Docs for CLUSTER SLOT-STATS, with key-count, cpu-usec, network-bytes-in, and network-bytes-out metrics.