Update cluster-slots to explain the deterministic ordering #159
Conversation
ref: valkey-io/valkey#265 Signed-off-by: Roshan Khatri <[email protected]>
commands/cluster-slots.md
Outdated
| @@ -34,7 +34,8 @@ Each nested result is: | |||
| - ...continues until all replicas for this master are returned. | |||
|
|
|||
| Each result includes all active replicas of the master instance | |||
| for the listed slot range. Failed replicas are not returned. | |||
| for the listed slot range. These active replicas are ordered deterministically. | |||
| Failed replicas are not returned. | |||
There was a problem hiding this comment.
I think we don't need a newline here.
There was a problem hiding this comment.
@hpatro FYI, the recommendation is to have one sentence per line. The single linebreak doesn't have a semantic meaning in markdown so it will not be rendered as a linebreak, but if we change the text later, the diff will be per line so it's easier to see the diff if we do this. See the README of this repo: https://github.com/valkey-io/valkey-doc?tab=readme-ov-file#styling-guidelines
commands/cluster-slots.md
Outdated
| @@ -34,7 +34,8 @@ Each nested result is: | |||
| - ...continues until all replicas for this master are returned. | |||
|
|
|||
| Each result includes all active replicas of the master instance | |||
| for the listed slot range. Failed replicas are not returned. | |||
| for the listed slot range. These active replicas are ordered deterministically. | |||
There was a problem hiding this comment.
I think we could just mention that the command response is deterministic across all nodes in a cluster. WDYT ?
There was a problem hiding this comment.
Also, should we clarify that they are ordered lexicographically?
There was a problem hiding this comment.
I think we should. Because clients could check the result according to their ipaddress (becuase ipaddress as the first item in the result).
There was a problem hiding this comment.
I think we could just mention that the command response is deterministic across all nodes in a cluster
Do you mean at the start of the doc? like this
`CLUSTER SLOTS` returns details about which cluster slots map to which Valkey instances.
The command response is deterministic across all nodes in a cluster and is ordered lexicographically.
There was a problem hiding this comment.
I'm fine with placing it at the top.
Signed-off-by: Roshan Khatri <[email protected]>
Signed-off-by: Madelyn Olson <[email protected]>
|
LGTM! |
|
|
||
| The command response is deterministic across all nodes in a cluster, which means that if two nodes return the same response they have the same view of the cluster. |
There was a problem hiding this comment.
This sentence is not per slot range, so I don't think it belongs under this section.
I would put this sentence at line 3, almost at the beginning of the page.
There was a problem hiding this comment.
I see that it was higher up earlier and was moved down by request by others. :) If it's not important enough to be at the top of the page, then maybe move it down further, to just before the example?
| Primaries are ordered by the slots they serve and then replicas are ordered lexicographically by the node-id they were assigned by the cluster. | ||
| The third nested reply is guaranteed to be the networking information of the master instance for the slot range. | ||
| All networking information after the third nested reply are replicas of the master. | ||
|
|
There was a problem hiding this comment.
Here, we're jumping between describing the primary, the replicas and the slot ranges.
This section is about a slot range, which is each line in the nested reply. Looking at the whole text from "Each result includes all active replicas of the master instance for the listed slot range" until the example below, I would start with describing the ordering of the slot ranges (including the sentence that explains that if a shard has non-contiguous slots, then the networking information is repeated for each range) and then say what the primary for this slot range is and that the replicas for this slot range are ordered lexicographically by node-id.
Rough idea:
Each result includes all active replicas of the primary instance for the listed slot range.
Failing replicas are not returned.The slot ranges are returned in ascending order.
If a cluster shard serves non-contiguous slots (e.g. 1-400,900,1800-6000) then primary and replica networking information results are duplicated for each slot range.The third nested reply is the networking information of the primary instance for the slot range.
All networking information after the third nested reply are replicas of the primary.
The replicas of each primary are ordered lexicographically by the node-id they were assigned by the cluster.
zuiderkwast
changed the title
…#159) Update cluster-slots docs ref: valkey-io/valkey#265 --------- Signed-off-by: Roshan Khatri <[email protected]> Signed-off-by: Madelyn Olson <[email protected]> Co-authored-by: Madelyn Olson <[email protected]> Signed-off-by: asafpamzn <[email protected]>
Update cluster-slots docs
ref: valkey-io/valkey#265