Add cluster bind comment #1590
Open
Add cluster bind comment #1590
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hello Matt, thank you, holding on this some more time as I guess we should consider what you found about "bind" basically a bug... My feeling is that it should resemble the way "save" works, except that for both I would like to have a special way to reset, this is very handy when dealing with includes. Like: Or something along this lines. Skipping this changes for 3.0-beta2... more comments ASAP. |
Update the comment to mention last-bind-wins
as well as add some IPv6 addresses to the
commented out examples.
Rationale
---------
I always thought "bind" was like "save" where
every later usage of the directive would be
aggregated together and processed as a bundle at
config bring-up time, but it turns out each read
of the "bind" directive erases prior usage.
Except, not completely. If you have:
bind 172.16.1.1 172.16.1.2 172.16.1.3
bind 172.16.1.6
The second "bind" will make the server only bind to 172.16.1.6
(since server.binaddr_count gets set to 1 for that evaluation
of the "bind" directive and listenToPort() will only loop through
binaddr_count elements of server.bindaddr[]), *BUT* the other
positions in server.bindaddr[] will still be populated by the
IPs from the prior "bind" minus the first IP which got overridden
by the second "bind."
So, we end up wtih:
bindaddr[0] = 172.16.1.6
bindaddr[1] = 172.16.1.2
bindaddr[2] = 172.16.1.3
They remain populated, but will never be used since: the maximum
evalation is server.bindaddr[server.bindaddr_count-1] *and* Redis
maintains no mechanism to alter, update, or re-bind IP addresses
at runtime, so extra addresses can't impact operations.
We're left with three possibilities for future improvements,
each is completely optional:
Optional fix A: after each evaluation of a "bind", run
sdsfree() all higher array indexes.
Optional fix B: make "bind" an additive directive where each
additional line adds new addresses (two line change, but may
break exising configurations relying on old last-line-wins
behavior). This may also break some configurations where people
are overriding the config "bind" line with a command line --bind
(which gets evaluated last, and therefore overrides any mention
of "bind" currently in a redis.conf).
Optional fix C: Just leave it alone since there's no functional
impact.
Add note about the first bind address being used as be the source address for contacting other cluster nodes, so the first bind address must be reachable from all other cluster nodes you expect to talk to. We can give more examples and use cases of these sutations in the official redis.io docs too.
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update redis.conf with notes about how "bind" works in general and with cluster.
(Somehow, these extra 12 lines of comments expanded into over 60 lines of commit notes.)