Skip to content

Implement connection refresh on circular moved - #5893

Merged
alexr-bq merged 7 commits into
mainfrom
alexr/redirect-feature
May 26, 2026
Merged

Implement connection refresh on circular moved#5893
alexr-bq merged 7 commits into
mainfrom
alexr/redirect-feature

Conversation

@alexr-bq

@alexr-bq alexr-bq commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Modifies behaviour of glide when receiving a MOVED request to the same address. Now this triggers a reconnect.

Features / Behaviour Changes

When Redis/Valkey servers are deployed behind DNS records, load balancers, or proxies, a MOVED error may be returned with a target endpoint that is the same endpoint from which the error originated.

Before this change, when Glide received a MOVED error pointing to the same endpoint, the retry would fail because:

  1. The multiplexed connection writes the retry request to a local buffer (succeeds)
  2. The server has already closed the connection after sending the MOVED response
  3. The disconnection is only detected during the read phase
  4. At that point, it's unsafe to retry because we can't determine if the server received the request
  5. The error (FatalReceiveError) propagates back to the application

After this change, when a MOVED error points to the same endpoint (circular MOVED), the client:

  1. Detects the circular condition by comparing the redirect address with the current connection address
  2. Triggers a reconnection before retrying the command
  3. The retry occurs on a fresh connection, allowing the DNS/proxy/load balancer to route to a different underlying server host

This fix is similar to StackExchange.Redis PR #3003 which addresses the same issue.

Implementation

Core fix in glide-core/redis-rs/redis/src/cluster_async/mod.rs:

In the RetryMethod::MovedRedirect handler (~line 1343), added detection for circular MOVED:

if let Some((redirect_addr, _slot)) = redirect_node {
    let is_circular = redirect_addr == address;
    if is_circular {
        log_debug_lazy!(
            "cluster",
            format!(
                "Detected circular MOVED redirect to same address: {}. \
                 Reconnecting before retry to avoid potential connection issues.",
                address
            )
        );
        // Reset routing and reconnect with retry
        request.info.reset_routing();
        return Next::Reconnect {
            request: Some(request),
            target: address,
        }
        .into();
    }
}

Key implementation details:

  • Uses string comparison for address matching, consistent with how addresses are compared elsewhere in the codebase
  • Returns Next::Reconnect with request: Some(request) which triggers reconnection AND queues the request for retry via pending_requests_tx
  • Skips the normal slot refresh since the slot map is already correct (the endpoint is the same, just the underlying server changed)
  • Only applies to MOVED errors, not ASK (ASK is a different scenario for temporary redirects during migration)

New tests in glide-core/redis-rs/redis/tests/test_cluster_async.rs:

Added two tests that verify the fix by tracking connection establishment (via PING count):

  • test_async_cluster_circular_moved_triggers_reconnect - Tests GET command with circular MOVED
  • test_async_cluster_circular_moved_set_triggers_reconnect - Tests SET command with circular MOVED

The tests verify that:

  1. A reconnect occurs between the MOVED response and the successful retry (ping count increases)
  2. The command ultimately succeeds

Limitations

  • This fix only handles the async cluster client path. The sync cluster client may need a similar fix if it exhibits the same issue.
  • The fix assumes that the MOVED address format matches the connection address format exactly (string comparison). This should always be true since both come from the same source format.

Testing

Unit tests:

  • Two new MockEnv-based tests that verify reconnection occurs on circular MOVED
  • Tests pass with the fix and fail without it (verified by temporarily commenting out the fix)
  • Run with: GLIDE_VERSION=test cargo test --manifest-path glide-core/redis-rs/redis/Cargo.toml --test test_cluster_async -- test_async_cluster_circular_moved --nocapture

End-to-end test:

  • Tested with a local custom proxy

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Linters have been run (make *-lint targets) and Prettier has been run (make prettier-fix).
  • Destination branch is correct - main or release
  • Create merge commit if merging release branch into main, squash otherwise.

@alexr-bq
alexr-bq requested a review from a team as a code owner May 7, 2026 17:45
@yipin-chen
yipin-chen requested a review from jduo May 21, 2026 00:29
Comment thread glide-core/redis-rs/redis/src/cluster_async/mod.rs Outdated
Comment thread glide-core/redis-rs/redis/tests/test_cluster_async.rs
Comment thread glide-core/redis-rs/redis/src/cluster_async/mod.rs
alexr-bq and others added 6 commits May 25, 2026 16:21
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
Extract shared is_circular_moved_redirect() function to detect when a
MOVED redirect points to the same address (circular redirect). Apply
this detection to both single command and pipeline paths.

For pipelines, circular MOVED errors now route through the reconnect
logic instead of the normal redirect logic, matching the behavior of
single commands.

Added test to verify pipeline commands recover from circular MOVED +
disconnect scenarios.

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
… patch-updates group across 1 directory (#5910)

chore(deps-dev): bump maturin

Bumps the patch-updates group with 1 update in the /python directory: [maturin](https://github.com/pyo3/maturin).

Updates `maturin` from 1.13.1 to 1.13.3
- [Release notes](https://github.com/pyo3/maturin/releases)
- [Changelog](https://github.com/PyO3/maturin/blob/main/Changelog.md)
- [Commits](PyO3/maturin@v1.13.1...v1.13.3)

---
updated-dependencies:
- dependency-name: maturin
  dependency-version: 1.13.3
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: patch-updates
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
…lution

- Add resolve_address parameter to is_circular_moved_redirect to handle
  cases where MOVED returns an IP but client connected via hostname
- Fix resolve_address to preserve original port when resolving IP to
  hostname (the MOVED response port is authoritative)
- Extend circular MOVED detection to pipeline path via handle_redirect_logic
- Add unit tests for hostname vs IP detection scenarios
- Add integration test for pipeline circular MOVED handling

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
@alexr-bq
alexr-bq force-pushed the alexr/redirect-feature branch from a7b13db to a35ecfd Compare May 25, 2026 23:21

@yipin-chen yipin-chen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@alexr-bq
alexr-bq merged commit 87c9991 into main May 26, 2026
85 of 87 checks passed
jamesx-improving added a commit that referenced this pull request May 26, 2026
* Implement connection refresh on circular moved



* fix: format redirect_node.map() call on single line



* fix(redis-rs): handle circular MOVED redirect in pipeline path

Extract shared is_circular_moved_redirect() function to detect when a
MOVED redirect points to the same address (circular redirect). Apply
this detection to both single command and pipeline paths.

For pipelines, circular MOVED errors now route through the reconnect
logic instead of the normal redirect logic, matching the behavior of
single commands.

Added test to verify pipeline commands recover from circular MOVED +
disconnect scenarios.



* chore(deps-dev): bump maturin from 1.13.1 to 1.13.3 in /python in the patch-updates group across 1 directory (#5910)

chore(deps-dev): bump maturin

Bumps the patch-updates group with 1 update in the /python directory: [maturin](https://github.com/pyo3/maturin).

Updates `maturin` from 1.13.1 to 1.13.3
- [Release notes](https://github.com/pyo3/maturin/releases)
- [Changelog](https://github.com/PyO3/maturin/blob/main/Changelog.md)
- [Commits](PyO3/maturin@v1.13.1...v1.13.3)

---
updated-dependencies:
- dependency-name: maturin
  dependency-version: 1.13.3
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: patch-updates
...





* fix(redis-rs): improve circular MOVED detection with hostname/IP resolution

- Add resolve_address parameter to is_circular_moved_redirect to handle
  cases where MOVED returns an IP but client connected via hostname
- Fix resolve_address to preserve original port when resolving IP to
  hostname (the MOVED response port is authoritative)
- Extend circular MOVED detection to pipeline path via handle_redirect_logic
- Add unit tests for hostname vs IP detection scenarios
- Add integration test for pipeline circular MOVED handling



* style: fix cargo fmt formatting



---------




(cherry picked from commit 87c9991)

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: James Xin <james.xin@improving.com>
Co-authored-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
affonsov pushed a commit that referenced this pull request Aug 21, 2026
* Implement connection refresh on circular moved

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>

* fix: format redirect_node.map() call on single line

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>

* fix(redis-rs): handle circular MOVED redirect in pipeline path

Extract shared is_circular_moved_redirect() function to detect when a
MOVED redirect points to the same address (circular redirect). Apply
this detection to both single command and pipeline paths.

For pipelines, circular MOVED errors now route through the reconnect
logic instead of the normal redirect logic, matching the behavior of
single commands.

Added test to verify pipeline commands recover from circular MOVED +
disconnect scenarios.

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>

* chore(deps-dev): bump maturin from 1.13.1 to 1.13.3 in /python in the patch-updates group across 1 directory (#5910)

chore(deps-dev): bump maturin

Bumps the patch-updates group with 1 update in the /python directory: [maturin](https://github.com/pyo3/maturin).

Updates `maturin` from 1.13.1 to 1.13.3
- [Release notes](https://github.com/pyo3/maturin/releases)
- [Changelog](https://github.com/PyO3/maturin/blob/main/Changelog.md)
- [Commits](PyO3/maturin@v1.13.1...v1.13.3)

---
updated-dependencies:
- dependency-name: maturin
  dependency-version: 1.13.3
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: patch-updates
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>

* fix(redis-rs): improve circular MOVED detection with hostname/IP resolution

- Add resolve_address parameter to is_circular_moved_redirect to handle
  cases where MOVED returns an IP but client connected via hostname
- Fix resolve_address to preserve original port when resolving IP to
  hostname (the MOVED response port is authoritative)
- Extend circular MOVED detection to pipeline path via handle_redirect_logic
- Add unit tests for hostname vs IP detection scenarios
- Add integration test for pipeline circular MOVED handling

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>

* style: fix cargo fmt formatting

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>

---------

Signed-off-by: Alex Rehnby-Martin <alex.rehnby-martin@improving.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

3 participants