-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Adding OSS Notification Classes for SMIGRATING and SMIGRATED. Handling of SMIGRATING is completed and covered with tests. #3849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat/hitless_upgrade_sync_cluster_client
Are you sure you want to change the base?
Conversation
b7bf8e3 to
a983c20
Compare
…ceholder for smigrated handler in OSSMaintNotificationsHandler class
a983c20 to
e3c8b3f
Compare
There was a problem hiding this 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 introduces OSS (Open Source Software) cluster maintenance notification support for handling slot migration events in Redis. The implementation adds two new notification classes (OSSNodeMigratingNotification and OSSNodeMigratedNotification) along with parser integration and comprehensive unit tests for the notification classes themselves.
- New notification classes to track slot migration with source/destination nodes and affected slots
- Thread-safe
OSSMaintNotificationsHandlerfor managing maintenance notifications with expiration cleanup - Parser integration to recognize and route OSS maintenance notifications (SMIGRATING and SMIGRATED messages)
- Full unit test coverage for notification class behavior (initialization, equality, hashing, collections)
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| redis/maint_notifications.py | Adds OSSNodeMigratingNotification, OSSNodeMigratedNotification classes and OSSMaintNotificationsHandler for managing OSS slot migration notifications |
| redis/_parsers/base.py | Adds parsing logic for SMIGRATING/SMIGRATED messages and integrates OSS notification handlers into push notification routing |
| redis/_parsers/resp3.py | Adds oss_cluster_maint_push_handler_func attribute to support OSS maintenance notification handling |
| redis/_parsers/hiredis.py | Adds oss_cluster_maint_push_handler_func attribute to support OSS maintenance notification handling |
| tests/maint_notifications/test_maint_notifications.py | Comprehensive unit tests for OSSNodeMigratingNotification and OSSNodeMigratedNotification classes |
| tests/maint_notifications/test_cluster_maint_notifications_handling.py | Integration test for SMIGRATING notification handling and test infrastructure refactoring |
| tests/maint_notifications/proxy_server_helpers.py | Helper functions for RESP format conversion and HTTP client response handling improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
In this PR the following changes are introduced:
OSS Notification Classes: New
OSSNodeMigratingNotificationandOSSNodeMigratedNotificationclasses that track slot migration events with source/destination nodes and affected slotsThread-Safe Handler:
OSSMaintNotificationsHandlerprovides thread-safe management of maintenance notifications with automatic expiration cleanupParser Integration: Seamless integration with existing parser infrastructure to recognize and route OSS maintenance notifications
Testing: Full unit test coverage for all notification classes ensuring proper behavior across initialization, equality, hashing, and collection operations
Out of scope for the current PR: initializing the nodes manager handler that will react on SMIGRATED notifications.