Skip to content

Skip nodes with unresolved address during initialization - #37

Merged
mxmsk merged 1 commit into
masterfrom
feature/allow-non-discoverable-init-nodes
Jul 14, 2026
Merged

Skip nodes with unresolved address during initialization#37
mxmsk merged 1 commit into
masterfrom
feature/allow-non-discoverable-init-nodes

Conversation

@mxmsk

Copy link
Copy Markdown
Contributor

This change fixes several issues:

  1. Major issue: all addresses passed to the NewCluster must be resolvable. This breaks the ability to include a future nodes in configuration that could simplify cluster migrations.
  2. NewCluster saves nodes by using provided addresses but then the existing Cluster manages them by connection addresses (dns-resolved ip's). This lead to unnecessary reconnection on first updateMappings call.
  3. When new node is discovered from CLUSTER SLOTS it is added with connection address, that potentially breaks TLS, because server name become inaccessible. To fix, redis setup should provide "hostname" with cluster slots metadata.
  4. Minor issue: NewCluster checked initialized nodes in master map while adding them to the nodes map. This could lead to unnecessary connections if address has duplicates.

@mxmsk
mxmsk requested a review from g7rJuly 14, 2026 14:41
g7r
g7r approved these changes Jul 14, 2026
if r.AddrHostnames == nil {
r.AddrHostnames = make(map[string]string)
}
r.AddrHostnames[addrStr] = hostname + ":" + portStr

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd suggest using net.JoinHostPort here and on addrStr := ... line.

@mxmsk
mxmskforce-pushed the feature/allow-non-discoverable-init-nodes branch from 5063cff to aba52e1CompareJuly 14, 2026 15:26
@mxmsk
mxmskforce-pushed the feature/allow-non-discoverable-init-nodes branch from aba52e1 to e23f7f6CompareJuly 14, 2026 16:09
@mxmsk
mxmsk merged commit 20c40ff into masterJul 14, 2026
25 of 36 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@mxmsk@g7r