Skip to content

Hitless-Upgrade: Add handling of MOVING push notification with "null" host:port info. #3738

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

Open
wants to merge 40 commits into
base: feat/hitless-upgrade-sync-standalone
Choose a base branch
from

Conversation

petyaslavova
Copy link
Collaborator

Pull Request check-list

Please make sure to review and check all of these items:

  • Do tests and lints pass with this change?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Description of change

Please provide a description of the change here.

petyaslavova and others added 30 commits July 24, 2025 16:40
… Refactored the maintenance events tests not to be multithreaded - we don't need it for those tests.
…ot processed in in Moving state. Tests are updated
Signed-off-by: Elena Kolevska <[email protected]>

Cleanup

Signed-off-by: Elena Kolevska <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
@petyaslavova petyaslavova force-pushed the ps_add_fail_over_events_handling branch from 8d7cc00 to 10ded34 Compare August 15, 2025 14:37
Base automatically changed from ps_add_fail_over_events_handling to feat/hitless-upgrade-sync-standalone August 19, 2025 09:08
@petyaslavova petyaslavova requested a review from Copilot August 20, 2025 15:16
@petyaslavova petyaslavova marked this pull request as ready for review August 20, 2025 15:17
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 adds support for handling MOVING push notifications that contain "null" host:port information, which occurs when Redis doesn't provide specific node address details during cluster maintenance operations.

Key Changes

  • Modified NodeMovingEvent to accept None values for host and port parameters
  • Updated parser logic to handle "null" values in MOVING push notifications
  • Enhanced maintenance event handling to process events without specific destination addresses

Reviewed Changes

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

Show a summary per file
File Description
redis/maintenance_events.py Updated NodeMovingEvent constructor to accept optional host/port and modified hash calculation
redis/_parsers/base.py Added parsing logic to handle "null" host:port values in MOVING notifications
redis/connection.py Enhanced connection pool methods to support event hash tracking and conditional host updates
tests/test_maintenance_events.py Added test cases for events with None host/port values
tests/test_maintenance_events_handling.py Added comprehensive integration test for null MOVING events and updated existing tests

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

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