[maintenance events] Expose maintenance events as public API - #4618
Open
atakavci wants to merge 10 commits into
Open
[maintenance events] Expose maintenance events as public API#4618atakavci wants to merge 10 commits into
atakavci wants to merge 10 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR promotes Redis maintenance push notifications to a supported public API by making MaintenanceEvent/MaintenanceEventListener and per-event subclasses public, and by moving listener registration to MaintenanceNotificationsConfig (wired per-connection by MaintenanceAwareVisitor).
Changes:
- Split maintenance event subclasses into top-level public
*Eventtypes with public read accessors (seq/ttl/shards/target). - Add
MaintenanceNotificationsConfig.Builder#maintenanceListener(...)and wire the config listener alongside the internal controller inMaintenanceAwareVisitor. - Remove per-
Connectionmaintenance listener storage and related APIs; update formatter includes to cover the new event files.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/redis/clients/jedis/MaintenanceEvent.java | Makes base event type public and adds getSeq(); removes nested subclasses. |
| src/main/java/redis/clients/jedis/MaintenanceEventListener.java | Makes listener interface public (but needs Javadoc update after API move). |
| src/main/java/redis/clients/jedis/MovingEvent.java | New top-level public event type with getters. |
| src/main/java/redis/clients/jedis/MigratingEvent.java | New top-level public event type with getters. |
| src/main/java/redis/clients/jedis/MigratedEvent.java | New top-level public event type with getters. |
| src/main/java/redis/clients/jedis/FailingOverEvent.java | New top-level public event type with getters. |
| src/main/java/redis/clients/jedis/FailedOverEvent.java | New top-level public event type with getters. |
| src/main/java/redis/clients/jedis/MaintenanceNotificationsConfig.java | Adds config-level listener registration + getter (needs immutability/thread-safety tweak + tests). |
| src/main/java/redis/clients/jedis/MaintenanceAwareVisitor.java | Builds per-connection listener set from controller + config listener (HashSet sizing can be improved). |
| src/main/java/redis/clients/jedis/Connection.java | Removes maintenance listener set and related add/remove/get APIs. |
| src/main/java/redis/clients/jedis/args/LatencyEvent.java | Formatting-only changes. |
| pom.xml | Adds **/*Event.java to formatter plugin includes to cover newly split files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit ce7e617. Configure here.
Comment on lines
3
to
6
| /** | ||
| * A server maintenance event. One subclass per type, each carrying the fields relevant to that | ||
| * type. Dispatched to a {@link MaintenanceEventListener} via {@link #accept}. | ||
| * type. Dispatched to a {@link MaintenanceEventHandler} when provided. | ||
| */ |
| * {@code [MOVING, seq, time_s, host:port]} — endpoint moves to {@code target} within | ||
| * {@code ttlSeconds}. | ||
| */ | ||
| static final class MovingEvent extends MaintenanceEvent { |
| * {@code [MIGRATING, seq, time_s, shards]} — {@code time_s} = starts-within; {@code shardIds} | ||
| * diagnostic. | ||
| */ | ||
| static final class MigratingEvent extends MaintenanceEvent { |
| } | ||
| } | ||
| /** {@code [MIGRATED, seq, shards]} — terminator; no time_s on the wire. */ | ||
| static final class MigratedEvent extends MaintenanceEvent { |
| * {@code [FAILING_OVER, seq, time_s, shards]} — {@code time_s} = starts-within; {@code shardIds} | ||
| * diagnostic. | ||
| */ | ||
| static final class FailingOverEvent extends MaintenanceEvent { |
| final class FailedOverEvent extends MaintenanceEvent { | ||
| final String shardIds; | ||
| /** {@code [FAILED_OVER, seq, shards]} — terminator. */ | ||
| static final class FailedOverEvent extends MaintenanceEvent { |
Comment on lines
1
to
7
| package redis.clients.jedis; | ||
|
|
||
| /** | ||
| * Typed listener for server maintenance push events. Registered on a {@link Connection} via | ||
| * {@link Connection#addMaintenanceEventListener}; the connection dispatches each parsed event to | ||
| * the matching method synchronously on its read thread, before the triggering read returns. A | ||
| * listener may mutate the delivering connection (e.g. relax timeouts, request rebind); exceptions | ||
| * propagate to the read loop. | ||
| */ | ||
| interface MaintenanceEventListener { | ||
| public interface MaintenanceEventListener { | ||
|
|
||
| void onMoving(MovingEvent e, Connection c); | ||
| void onEvent(MaintenanceEvent e); | ||
|
|
||
| void onMigrating(MigratingEvent e, Connection c); | ||
|
|
||
| void onMigrated(MigratedEvent e, Connection c); | ||
|
|
||
| void onFailingOver(FailingOverEvent e, Connection c); | ||
|
|
||
| void onFailedOver(FailedOverEvent e, Connection c); | ||
| } |
| private final MaintenanceEventListener listener; | ||
| private final AtomicReference<MaintenanceEvent> lastEvent = new AtomicReference<>(); | ||
|
|
||
| public MaintenanceEventListenerAdapter(MaintenanceEventListener listener) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4604
Summary
Adds a public, read-only way for applications to observe server maintenance events (MOVING, MIGRATING, MIGRATED, FAILING_OVER, FAILED_OVER). Users register a
MaintenanceEventListeneronMaintenanceNotificationsConfigand receive typedMaintenanceEventobjects, without any exposure to internal connection state. Internally, the old per-type listener contract is split off into a package-private handler so the public surface stays minimal.Key Decisions & Assumptions
MaintenanceEventListenergets a singleonEvent(MaintenanceEvent)method and never sees theConnection— only the event and its typed getters. The internal per-type contract (onMoving,onMigrating, …, each with aConnection) moves to a new package-privateMaintenanceEventHandler.MaintenanceEventbecomes public; its subclasses are now public static nested classes with accessors (getSeq,getType,getTarget,getTtlSeconds,getShardIds) plus anEventTypeenum.LinkedHashSetguarantees the internal controller runs first (completing rebind/handoff bookkeeping) before the user's listener adapter runs. When no custom listener is configured, a singleton set is used.MaintenanceEventListenerAdapterbridges the public listener to the internal handler and drops stale/duplicate events by comparing sequence numbers (CAS on the last seenseq).Behavioral / Conceptual Changes
MaintenanceNotificationsConfig.builder().maintenanceListener(...)and be notified of maintenance events.Connection#addMaintenanceEventListener/remove/get) is removed; the handler set is now assembled byMaintenanceAwareVisitorat init time.seq; out-of-order or repeated events are dropped.Testing
Existing maintenance tests were updated to the renamed internal
MaintenanceEventHandlertype (recording listeners and the consumer's set signature); the changes are mechanical and preserve the prior coverage of dispatch and listener-exception propagation.Notes
pom.xmlwidens a plugin include to**/*Event.java, andlogback-test.xmllowersMaintenanceAwareVisitorlogging to WARN.args/LatencyEvent.javachanges are indentation/formatting only.Note
Medium Risk
Public API and listener wiring changes affect connection-pool rebind and timeout behavior during maintenance; user callbacks run on the read thread after internal controller logic, so slow or throwing listeners can impact I/O.
Overview
Exposes Redis server maintenance notifications (MOVING, MIGRATING/MIGRATED, FAILING_OVER/FAILED_OVER) as a public API so apps can observe events without touching
Connectioninternals.MaintenanceEventis now public with anEventTypeenum and getters (getSeq,getType,getTtlSeconds,getShardIds,getTarget); per-type classes stay nested static types.MaintenanceEventListeneris public with a singleonEvent(MaintenanceEvent)callback (no per-type methods, noConnectionargument).Registration moves to
MaintenanceNotificationsConfig.builder().maintenanceListener(...).MaintenanceAwareVisitorbuilds a per-connection handler set: internalMaintenanceEventControllerfirst (rebind/timeouts), thenMaintenanceEventListenerAdapterwhen a listener is configured. The adapter deduplicates by sequence before calling the user listener.Connectionno longer stores or exposes maintenance listeners (add/remove/getMaintenanceEventListenerremoved). Internal dispatch uses package-privateMaintenanceEventHandler;MaintenanceEventConsumerfans out to handlers on the read thread unchanged in spirit.pom.xmladds**/*Event.javato formatter includes;LatencyEventis formatting-only; test/log tweaks follow the handler rename.Reviewed by Cursor Bugbot for commit ce7e617. Bugbot is set up for automated code reviews on this repo. Configure here.