Skip to content

Conversation

ggivo
Copy link
Contributor

@ggivo ggivo commented Aug 15, 2025

Description:

This change extends Lettuce’s support for Redis maintenance event notifications by adding the none option for moving-endpoint-type when enabling maintenance events.

According to the specification, moving-endpoint-type determines whether the MOVING push message contains an internal/external IP or FQDN.
The new special value none indicates that the MOVING message should not contain any endpoint; instead, the endpoint field is null.

When the client receives a MOVING message with moving-endpoint-type=none, it will schedule a graceful reconnect to the currently configured endpoint after half of the grace period specified in the message has elapsed.

Summary of changes:

  • Added support for moving-endpoint-type=none option in CLIENT MAINT_NOTIFICATIONS.
  • Implemented delayed rebind behavior: reconnect is scheduled after half of the grace period when none is used.

Example to enable maintenance notifications with none

CLIENT MAINT_NOTIFICATIONS ON moving-endpoint-type none

Tested manually & unit test

Make sure that:

  • You have read the contribution guidelines.
  • You have created a feature request first to discuss your contribution intent. Please reference the feature request ticket number in the pull request.
  • You applied code formatting rules using the mvn formatter:format target. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.

none indicates that the MOVING message doesn’t need to contain an endpoint. In such a case, the client is expected to schedule a graceful reconnect to its currently configured endpoint after half of the grace period that was communicated by the server is over.
@ggivo ggivo requested review from Copilot and tishun August 15, 2025 08:50
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends Lettuce's Redis maintenance event notifications to support the moving-endpoint-type=none option. When this option is used, the MOVING message doesn't contain an endpoint and triggers a graceful reconnect to the currently configured endpoint after half the grace period has elapsed.

  • Added NONE enum value to AddressType with proper documentation
  • Implemented deferred rebind logic that schedules reconnection after half the grace period
  • Modified MOVING event parsing to handle null endpoints correctly

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
MaintenanceEventsOptions.java Added NONE address type enum value with documentation
RedisHandshake.java Added mapping for NONE address type to "none" string
MaintenanceAwareConnectionWatchdog.java Implemented deferred rebind logic and null endpoint handling
MaintenanceAwareConnectionWatchdogUnitTests.java Added test coverage for null endpoint scenario and refactored helper methods

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

ggivo and others added 5 commits August 15, 2025 11:52
null for rebind adress can be returned as part of MOVING notification if client is connected using 'moving-endpoint-type=none'
@ggivo ggivo changed the title Support for none moving-endpoint-type in maintenance event notifications Support for none moving-endpoint-type in maintenance event notifications (CAE-1285) Aug 15, 2025
Copy link
Collaborator

@tishun tishun left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@tishun tishun modified the milestone: 7.0.0.RELEASE Aug 18, 2025
@ggivo ggivo changed the title Support for none moving-endpoint-type in maintenance event notifications (CAE-1285) [hitless upgrade] Support for none moving-endpoint-type in maintenance event notifications (CAE-1285) Aug 25, 2025
@ggivo ggivo merged commit 73ac00c into redis:feature/maintenance-events Aug 25, 2025
2 checks passed
ggivo added a commit that referenced this pull request Aug 29, 2025
…e event notifications (CAE-1285) (#3396)

* support MOVING with none

none indicates that the MOVING message doesn’t need to contain an endpoint. In such a case, the client is expected to schedule a graceful reconnect to its currently configured endpoint after half of the grace period that was communicated by the server is over.

* formatting

* Apply suggestions from code review

Co-authored-by: Copilot <[email protected]>

* Fix NPE

* Add test to cover null rebindAddress

null for rebind adress can be returned as part of MOVING notification if client is connected using 'moving-endpoint-type=none'

* Add java docs to RebindAwareAddressSupplier

---------

Co-authored-by: Copilot <[email protected]>
tishun added a commit that referenced this pull request Sep 1, 2025
* [Hitless Upgrades] React to maintenance events (#3345)

* v0.1

* Simple reconnect now working

* Bind address from message is now considered

* Self-register the handler

* Format code

* Filter push messages in a more stable way

* (very hacky) Relax comand expire timers globbaly

* Configure if timeout relaxing should be applied

* Proper way to close channel

* Configure the timneout relaxing

* Sequential handover implemented

* Did not address formatting

* Prolong the rebind windwow for relaxed tiemouts

* PubSub no longer required; CommandExpiryWriter is now channel aware; Polishing

* Use the new MOVING push message from the RE server

* Unit test was not chaining delgates in the same way that the RedisClient/RedisClusterClient was

* Fix REBIND message validation

* Fixed the expiry mechanism

* Polishing

* Fix NPE.

Seems like AttributeMap.attr is not accurate and actually return's  null causing some unit test failures.

* Add support for MIGRATING/MIGRATED message handling in command expiry

This commit adds the ability to listen for MIGRATING and MIGRATED messages
and trigger extended command expiry timeouts during Redis shard migration.

Key changes:
- Enhanced RebindAwareConnectionWatchdog to detect MIGRATING/MIGRATED messages
- RebindAwareExpiryWriter to trigger timeout relaxation whenever MIGRATING message is received

This feature allows commands to have relaxed timeouts during shard migration
operations, preventing unnecessary timeouts when Redis is temporarily busy
with migration tasks.

* formating

* Fix Disabling relaxTimeouts after upgrade can interfere with an ongoing one from re-bind

* Additional fix for timeout relaxing disabled

* Fix push message listener registered multiple times after rebind.

* Fix: Report correct command timeout when relaxTimeout is configured

* Disable relaxedTimeout after configured grace period

- Introduce a delay before disabling relaxedTimeout
- Grace period duration is provided via push notification

* Code clean up
  - Remove reading from pub/sub chanel and relay only on push notifications

* Add FAILING_OVER/FAILED_OVER

* Polishing : Rename components to use the word 'maintenace'

---------

Co-authored-by: Igor Malinovskiy <[email protected]>
Co-authored-by: ggivo <[email protected]>
# Conflicts:
#	src/main/java/io/lettuce/core/ClientOptions.java

* [Hitless upgrades] Add unit tests for the newly introduced classes #3355 (#3356)

* Unit tests for the maintanence aware classes

* Did not format properly

* Proper license

* Cae 633 add functional tests notifications (#3357) - excluding JsonExample.java

* Cae 1130 timeout tests (#3377)

* initial WIP, with lots of debugging, and some non-working tests

* debug

* more attemtps at debugging

* Refactor: Move cluster state management methods from MaintenanceNotificationTest to RedisEnterpriseConfig

- Moved refreshClusterConfig, recordOriginalClusterState, and restoreOriginalClusterState methods
- Updated call sites in MaintenanceNotificationTest to use RedisEnterpriseConfig methods
- Added required imports and static variables to RedisEnterpriseConfig
- Maintained existing functionality while improving code organization

Improvements to RelaxedTimeoutConfigurationTest:
- Simplified traffic generation logic by removing complex multi-phase testing
- Streamlined BLPOP command execution with better timeout detection
- Added relaxed timeout detection and recording during maintenance events
- Improved logging and error handling for timeout analysis
- Enhanced test assertions to focus on relaxed timeout detection rather than success counts
- Added MOVING operation duration tracking for better test analysis

* Improve test reliability and cleanup: Add @AfterEach cleanup, enhance endpoint tracking, and improve logging

* add un-relaxed tests. will investigate further why they got broken at some point via diff

* CAE-1130: Update timeout configuration test and watchdog implementation

* Reset MaintenanceAwareConnectionWatchdog.java and log4j2-test.xml to upstream versions

* Clean up debug info and outdated comments from timeout tests

- Remove large debug block with reflection-based debugging in RelaxedTimeoutConfigurationTest
- Simplify excessive debug logging and verbose markers (*** and ===)
- Clean up maintenance notification test logging
- Improve push notification monitor message formatting
- Maintain all test functionality while improving code readability

* Refactor: Move inline comments above code and fix string comparisons

- Move all inline comments to be above the code they reference in:
  * RelaxedTimeoutConfigurationTest.java
  * RedisEnterpriseConfig.java
  * MaintenanceNotificationTest.java
- Replace string != comparisons with .equals() for proper string comparison
- Apply code formatting via Maven formatter

This improves code readability and follows Java best practices for string comparison.

* [Hitless Upgrades] Zero-server-side configuration with client-side opt-in (#3380)

* Support for Client-side opt-in

 A client can tell the server if it wants to receive maintenance push notifications via the following command:

 CLIENT MAINT_NOTIFICATIONS <ON | OFF> [parameter value parameter value ...]

* update maintenance events to latest format

 - MIGRATING <seq_number> <time> <shard_id-s>: A shard migration is going to start within <time> seconds.
 - MIGRATED <seq_number> <shard_id-s>: A shard migration ended.
 - FAILING_OVER <seq_number> <time> <shard_id-s>: A shard failover of a healthy shard started.
 - FAILED_OVER <seq_number> <shard_id-s>: A shard failover of a healthy shard ended.
 - MOVING <seq_number> <time> <endpoint>: A specific endpoint is going to move to another node within <time> seconds

* clean up

* Update FAILED_OVER & MIGRATED to include additional time field

* update is private reserver check & add unit tests

update is private reserver check

* add unit tests for handshake with enabled maintenance events

* add missing copyrights/docs

* format

* address review comments

* Revert address after rebind operation expires

* Update event's validation spec

  - MIGRATING <seq_number> <time> <shard_id-s>: A shard migration is going to start within <time> seconds.
  - MIGRATED <seq_number> <shard_id-s>: A shard migration ended.
  - FAILING_OVER <seq_number> <time> <shard_id-s>: A shard failover of a healthy shard started.
  - FAILED_OVER <seq_number> <shard_id-s>: A shard failover of a healthy shard ended.
  - MOVING <seq_number> <time> <endpoint>: A specific endpoint is going to move to another node within <time> seconds

* rebase

* format after rebase

* Apply suggestions from code review

Co-authored-by: Tihomir Krasimirov Mateev <[email protected]>

* javadoc updated

* Update src/main/java/io/lettuce/core/internal/NetUtils.java

Co-authored-by: Tihomir Krasimirov Mateev <[email protected]>

---------

Co-authored-by: Tihomir Krasimirov Mateev <[email protected]>

* [hitless upgrade] Support for none moving-endpoint-type in maintenance event notifications (CAE-1285) (#3396)

* support MOVING with none

none indicates that the MOVING message doesn’t need to contain an endpoint. In such a case, the client is expected to schedule a graceful reconnect to its currently configured endpoint after half of the grace period that was communicated by the server is over.

* formatting

* Apply suggestions from code review

Co-authored-by: Copilot <[email protected]>

* Fix NPE

* Add test to cover null rebindAddress

null for rebind adress can be returned as part of MOVING notification if client is connected using 'moving-endpoint-type=none'

* Add java docs to RebindAwareAddressSupplier

---------

Co-authored-by: Copilot <[email protected]>

* [Hitless Upgrades]  Enable maintenance events support by default (CAE-1303) (#3415)

* set default relaxed timeout to 10s

* Enable maintenance events support by default

* Enable maintenance events support by default

* fix tests
   - ensure MaintenanceAwareExpiryWriter is registered for events when wathcdog is created
   - command timeout was not applied

* fix tests

   - sporadic failure because of timeout of 50ms
   RedisHandshakeUnitTests.handshakeDelayedCredentialProvider:153 » ConditionTimeout
   - new command introduced during handshake, increase the timeout to 100ms

* resolve errors after rebase on main

  - reset() - removed with
    issue#3328 - remove deprecated code from issue#907 (#3395)

* Remove LettuceMaintenanceEventsDemo.java

    - no longer needed.

---------

Co-authored-by: Igor Malinovskiy <[email protected]>
Co-authored-by: ggivo <[email protected]>
Co-authored-by: kiryazovi-redis <[email protected]>
Co-authored-by: Copilot <[email protected]>
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