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

@mxmsk mxmsk commented Jul 14, 2026

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 g7r July 14, 2026 14:41
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 mxmsk force-pushed the feature/allow-non-discoverable-init-nodes branch from 5063cff to aba52e1 Compare July 14, 2026 15:26
@mxmsk mxmsk force-pushed the feature/allow-non-discoverable-init-nodes branch from aba52e1 to e23f7f6 Compare July 14, 2026 16:09
@mxmsk mxmsk merged commit 20c40ff into master Jul 14, 2026
25 of 36 checks passed
Sign up for free to 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