Skip to content

Conversation

@hwware
Copy link
Contributor

@hwware hwware commented Aug 18, 2021

Background:
Currently, for a cluster node, cluster bus port = command port+ 10000, which is a fixed value.

We would like to make the value flexible, thus user could have more choice for the offset between command port
and cluster bus port.

The parameter "cluster-port" can be added in redis.conf, the value could be from 1 to 65535.

Example for 4 node config files in a clsuter:

node1:

port 6381
cluster-enabled yes
cluster-config-file nodes1-6381.conf
cluster-node-timeout 15000
cluster-port 10000

node2:

port 6382
cluster-enabled yes
cluster-config-file nodes2-6382.conf
cluster-node-timeout 15000
cluster-port 10001

node3:

port 6383
cluster-enabled yes
cluster-config-file nodes3-6383.conf
cluster-node-timeout 15000
cluster-port 20000

node4:

port 6384
cluster-enabled yes
cluster-config-file nodes4-6384.conf
cluster-node-timeout 15000

the result:

image

@hwware hwware requested review from oranagra and yossigo August 18, 2021 15:21
@hwware hwware changed the title Make cluster-port-incr variable configurable, the default value is 10000 Add cluster-port parameter in the redis.conf Sep 23, 2021
@hwware
Copy link
Contributor Author

hwware commented Sep 23, 2021

@madolson I update the codes for the requirement: make the cluster port more flexible, instead of using a fixed value: command port+ 10000. Please take a look, Thanks

Copy link
Contributor

@madolson madolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation is good, just some style comments

Copy link
Contributor

@madolson madolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we should update the https://redis.io/commands/cluster-meet documentation in parallel. I realized that it was wrong.

@madolson
Copy link
Contributor

@redis/core-team Approval for the new config name

@madolson madolson added approval-needed Waiting for core team approval to be merged state:major-decision Requires core team consensus labels Sep 30, 2021
@hwware
Copy link
Contributor Author

hwware commented Sep 30, 2021

I suppose we should update the https://redis.io/commands/cluster-meet documentation in parallel. I realized that it was wrong.

I will update the documentation with more details after this PR is merged. Thanks for your words.

Copy link
Contributor

@madolson madolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@oranagra oranagra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. maybe we wanna add some basic test coverage?

@hwware hwware force-pushed the cluster_port_incr_config branch from b8f5cd8 to 6cc7bea Compare October 12, 2021 21:25
@madolson madolson removed the approval-needed Waiting for core team approval to be merged label Oct 18, 2021
@madolson madolson changed the title Add cluster-port parameter in the redis.conf Make Cluster-bus port configurable with new cluster-port config Oct 18, 2021
@madolson madolson merged commit 1c2b5f5 into redis:unstable Oct 19, 2021
@oranagra oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label Oct 19, 2021
enjoy-binbin added a commit to enjoy-binbin/redis that referenced this pull request Feb 25, 2022
In redis#9389, we add a new cluster-port config and make cluster bus port
configurable. Now add support for this parameter in redis-cli.

Mainly for `--cluster create` and `--cluster add-node`, we can use
`ip:port@bus_port` to pass the `cluster-port` parameter. The bus_port
is optional, if not specified it is the same as before.

Also add the `bus_port` field in `--cluster backup`, we also need
to backup the `cluster-port`.

Fixes redis#10342
enjoy-binbin added a commit to enjoy-binbin/redis that referenced this pull request Feb 27, 2022
In `CLUSTER MEET`, bus-port argument was added in 11436b1.
For cluster announce ip / port implementation, part of the
4.0-RC1.

And in redis#9389, we add a new cluster-port config and make
cluster bus port configurable, part of the 7.0-RC1.
oranagra pushed a commit that referenced this pull request Jun 23, 2022
In `CLUSTER MEET`, bus-port argument was added in 11436b1.
For cluster announce ip / port implementation, part of the
4.0-RC1.

And in #9389, we add a new cluster-port config and make
cluster bus port configurable, part of the 7.0-RC1.
oranagra pushed a commit that referenced this pull request Jul 11, 2022
In #9389, we add a new `cluster-port` config and make cluster bus port configurable,
and currently redis-cli --cluster create/add-node doesn't support with a configurable `cluster-port` instance.
Because redis-cli uses the old way (port + 10000) to send the `CLUSTER MEET` command.

Now we add this support on redis-cli `--cluster`, note we don't need to explicitly pass in the
`cluster-port` parameter, we can get the real `cluster-port` of the node in `clusterManagerNodeLoadInfo`,
so the `--cluster create` and `--cluster add-node` interfaces have not changed.

We will use the `cluster-port` when we are doing `CLUSTER MEET`, also note that `CLUSTER MEET` bus-port
parameter was added in 4.0, so if the bus_port (the one in redis-cli) is 0, or equal (port + 10000),
we just call `CLUSTER MEET` with 2 arguments, using the old form.

Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com>
enjoy-binbin added a commit to enjoy-binbin/redis that referenced this pull request Jul 31, 2023
In `CLUSTER MEET`, bus-port argument was added in 11436b1.
For cluster announce ip / port implementation, part of the
4.0-RC1.

And in redis#9389, we add a new cluster-port config and make
cluster bus port configurable, part of the 7.0-RC1.
enjoy-binbin added a commit to enjoy-binbin/redis that referenced this pull request Jul 31, 2023
In redis#9389, we add a new `cluster-port` config and make cluster bus port configurable,
and currently redis-cli --cluster create/add-node doesn't support with a configurable `cluster-port` instance.
Because redis-cli uses the old way (port + 10000) to send the `CLUSTER MEET` command.

Now we add this support on redis-cli `--cluster`, note we don't need to explicitly pass in the
`cluster-port` parameter, we can get the real `cluster-port` of the node in `clusterManagerNodeLoadInfo`,
so the `--cluster create` and `--cluster add-node` interfaces have not changed.

We will use the `cluster-port` when we are doing `CLUSTER MEET`, also note that `CLUSTER MEET` bus-port
parameter was added in 4.0, so if the bus_port (the one in redis-cli) is 0, or equal (port + 10000),
we just call `CLUSTER MEET` with 2 arguments, using the old form.

Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes state:major-decision Requires core team consensus

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants