Fix/empty hostname cluster slots#5373
Conversation
|
@NeoPhi please sign off on the commit to make DCO happy. |
There was a problem hiding this comment.
Pull request overview
Fixes a cluster-topology parsing edge case in the shared Rust core (glide-core) where AWS ElastiCache returns hostname: "" in CLUSTER SLOTS metadata, which previously produced invalid connection addresses (e.g., :6379) and could lead to AllConnectionsUnavailable.
Changes:
- Treat empty-string
hostnamevalues inCLUSTER SLOTSmetadata as absent during parsing (so IP-based fallback works). - Add a unit test covering the empty-hostname metadata case (for both metadata formats).
- Document the fix in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| glide-core/redis-rs/redis/src/cluster_topology.rs | Filters out empty hostname metadata and adds a unit test to ensure fallback to IP-based addresses. |
| CHANGELOG.md | Adds a CORE “Fixes” entry describing the ElastiCache empty-hostname issue and resolution. |
| let h = String::from_utf8_lossy(value_bytes).into_owned(); | ||
| if !h.is_empty() { | ||
| metadata_hostname = Some(h); |
There was a problem hiding this comment.
String::from_utf8_lossy(value_bytes).into_owned() allocates even when the hostname is empty. You can avoid the allocation by keeping the Cow<str> and only calling into_owned() after the non-empty check (e.g., check is_empty() on the Cow first).
| let h = String::from_utf8_lossy(value_bytes).into_owned(); | |
| if !h.is_empty() { | |
| metadata_hostname = Some(h); | |
| let h_cow = String::from_utf8_lossy(value_bytes); | |
| if !h_cow.is_empty() { | |
| metadata_hostname = Some(h_cow.into_owned()); |
|
@NeoPhi Other than the DCO and copilot small efficiency comment, LGTM. |
6e7ebb2 to
c05bbc1
Compare
|
@NeoPhi DCO still failing |
AWS ElastiCache (plaintext, cluster mode, Redis 7.2.4) returns
`hostname: ""` (empty string) in the CLUSTER SLOTS metadata for
every node. The parser was wrapping this in `Some("")`, which
caused `unwrap_or_else` at line 252 to use the empty string
instead of falling back to the IP address from the primary
identifier.
This resulted in connection addresses like `:6379` (no host),
which fail immediately. Because `refresh_slots_inner()` silently
drops connection errors, `createClient` resolves successfully but
the client has zero usable connections. Every subsequent command
fails with `AllConnectionsUnavailable`.
The fix filters out empty hostname strings at parse time, so they
are treated the same as absent hostnames, and the IP from the
primary identifier is used instead.
Signed-off-by: Daniel Rinehart <danielr@philo.com>
c05bbc1 to
0e909af
Compare
|
Sorry about that. Think I got it this time. |
|
@NeoPhi merged, thanks! |
Fix: treat empty hostname in CLUSTER SLOTS metadata as absent
AWS ElastiCache (plaintext, cluster mode, Redis 7.2.4) returns
`hostname: ""` (empty string) in the CLUSTER SLOTS metadata for
every node. The parser was wrapping this in `Some("")`, which
caused `unwrap_or_else` at line 252 to use the empty string
instead of falling back to the IP address from the primary
identifier.
This resulted in connection addresses like `:6379` (no host),
which fail immediately. Because `refresh_slots_inner()` silently
drops connection errors, `createClient` resolves successfully but
the client has zero usable connections. Every subsequent command
fails with `AllConnectionsUnavailable`.
The fix filters out empty hostname strings at parse time, so they
are treated the same as absent hostnames, and the IP from the
primary identifier is used instead.
Signed-off-by: Daniel Rinehart <danielr@philo.com>
Signed-off-by: prashanna-frsh <prashanna.rajendran@freshworks.com>
Fix: treat empty hostname in CLUSTER SLOTS metadata as absent
AWS ElastiCache (plaintext, cluster mode, Redis 7.2.4) returns
`hostname: ""` (empty string) in the CLUSTER SLOTS metadata for
every node. The parser was wrapping this in `Some("")`, which
caused `unwrap_or_else` at line 252 to use the empty string
instead of falling back to the IP address from the primary
identifier.
This resulted in connection addresses like `:6379` (no host),
which fail immediately. Because `refresh_slots_inner()` silently
drops connection errors, `createClient` resolves successfully but
the client has zero usable connections. Every subsequent command
fails with `AllConnectionsUnavailable`.
The fix filters out empty hostname strings at parse time, so they
are treated the same as absent hostnames, and the IP from the
primary identifier is used instead.
Signed-off-by: Daniel Rinehart <danielr@philo.com>
Summary
AWS ElastiCache (plaintext, cluster mode, Redis 7.2.4) returns
hostname: ""(empty string) in the CLUSTER SLOTS metadata for every node. The parser was wrapping this inSome(""), which causedunwrap_or_elseat line 252 to use the empty string instead of falling back to the IP address from the primary identifier.This resulted in connection addresses like
:6379(no host), which fail immediately. Becauserefresh_slots_inner()silently drops connection errors,createClientresolves successfully but the client has zero usable connections. Every subsequent command fails withAllConnectionsUnavailable.Issue link
This Pull Request is linked to issue (URL): #5367
Features / Behaviour Changes
The fix filters out empty hostname strings at parse time, so they are treated the same as absent hostnames, and the IP from the primary identifier is used instead.
Implementation
Checks for a non-empty hostname.
Limitations
Testing
Unit test added.
Checklist
Before submitting the PR make sure the following are checked:
make *-linttargets) and Prettier has been run (make prettier-fix).