Skip to content

Adding support in async standalone client for maintenance notifications#4114

Open
petyaslavova wants to merge 7 commits into
feat_add_sch_asyncfrom
ps_async_sch_standalone
Open

Adding support in async standalone client for maintenance notifications#4114
petyaslavova wants to merge 7 commits into
feat_add_sch_asyncfrom
ps_async_sch_standalone

Conversation

@petyaslavova

@petyaslavova petyaslavova commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Description of change

Please provide a description of the change here.

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.


Note

Medium Risk
Changes connection lifecycle, pool locking, and endpoint switching during server maintenance; behavior is complex but heavily tested and mirrors existing sync logic.

Overview
Adds RESP3 maintenance notification support to the async standalone Redis client and connection pool, aligned with the existing sync implementation.

Async connections gain mixins that wire push parsers, send CLIENT MAINT_NOTIFICATIONS on connect, relax/reschedule read timeouts during maintenance, and reset state on disconnect. A new redis/asyncio/maint_notifications.py handles MOVING / migrate / failover pushes via AsyncMaintNotificationsPoolHandler and AsyncMaintNotificationsConnectionHandler, using asyncio tasks (not threading.Timer) for TTL cleanup and delayed proactive reconnect. Pool updates (apply_moving_notification, cleanup, proactive reconnect) run atomically under the pool’s non-reentrant asyncio.Lock; BlockingConnectionPool can enter maintenance mode so get/release does not interleave with those mutations.

redis.asyncio.Redis and ConnectionPool accept maint_notifications_config (auto-enabled for default RESP3 TCP pools), with guards for RESP2, Unix sockets, missing hosts, and unsupported connection classes. ensure_connection no longer treats pending push data as a broken connection when maintenance notifications are enabled (sync pools get the same fix). Shared helpers in redis/maint_notifications.py and check_protocol_version (including SENTINEL) reduce duplication; sync/async clients fix single-connection reconnect in finally blocks.

Coverage is expanded with a large async maint-notifications test module plus targeted sync and pool URL-parsing test updates.

Reviewed by Cursor Bugbot for commit 0f3de05. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread redis/asyncio/connection.py
@jit-ci

jit-ci Bot commented Jun 11, 2026

Copy link
Copy Markdown

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

Comment thread redis/asyncio/connection.py
Comment thread redis/asyncio/connection.py
) -> None:
task = asyncio.create_task(self._run_after(delay, callback, *args))
self._scheduled_tasks.add(task)
task.add_done_callback(self._scheduled_tasks.discard)

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.

_scheduled_tasks aren't cancelled on pool/client disconnect

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think we should call the cancel on pool/client close - after that moment the client or pool should not usable and canceled revert of the original config is not needed. If we cancel on disconnect we will leave the configuration for the new and for the existing connections in the tmp state set by MOVING. I added the cancel logic to be called when pool's aclose is called.

connection.can_read.side_effect = [RedisConnectionError("closed"), True]
connection.should_reconnect.return_value = False

if pool_class == BlockingConnectionPool:

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.

In sync version you have a logic to set a BlockingConnectionPool to a maintenance mode which prevents creation/releasing new connections while MOVING notifications are processed, but async version missing this.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. Fixed it. In my initial pass, the blocking pool was taking _lock on both get_connection and release. I reverted release to always lock — removing the lock around await connection.disconnect() would leave the connection in neither _in_use_connections nor _available_connections while the await is suspended, which is exactly the window where the MOVING handler can iterate both lists and miss it. So release stays as it was. I kept the conditional locking on get_connection only (its critical section has no await so the sync flag pattern is safe there), matching how the sync BlockingConnectionPool does it.

and maint_notifications_config.enabled == "auto"
):
# Log warning but don't fail the connection
import logging

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.

It looks like there's already a module-level logger, so it should be used instead

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No for this file. So added this here to be in sync with the synchronous implementation.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0f3de05. Configure here.

Comment thread redis/asyncio/client.py
)
raise
finally:
if self.single_connection_client:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Client close skips task cancel

Medium Severity

Redis.aclose shuts down the pool via disconnect, while MOVING TTL cleanup is cancelled only from ConnectionPool.aclose through _on_close. After a normal client close, scheduled maintenance tasks can still run and mutate pool connection_kwargs or connection MOVING state on a pool the app treats as closed.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0f3de05. Configure here.

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