Fix high CPU on Windows memory:// backend and add Windows CI#340
Fix high CPU on Windows memory:// backend and add Windows CI#340chrisguidry merged 12 commits intomainfrom
Conversation
Docket's worker loops rely on Redis blocking behavior that fakeredis doesn't implement — `get_message()` and `xread()` return immediately instead of blocking, turning the loops into CPU spinners. The `xreadgroup` loop already had a workaround; this applies the same pattern to the cancellation listener and strike monitor. Also fixes a few things that prevent running on Windows at all: signal handler registration (raises `NotImplementedError` on Windows instead of working), `fcntl` import (doesn't exist on Windows — swapped for cross-platform locking with `msvcrt`), and Docker SDK imports in conftest that fail when Docker isn't installed. Adds a Windows CI job (memory:// only, 3 Python versions) and a regression test that directly measures idle CPU usage. Closes #339 Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
📚 Documentation has been built for this PR! You can download the documentation directly here: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #340 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 99 99
Lines 3090 3097 +7
Branches 28 27 -1
=========================================
+ Hits 3090 3097 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…ersions The `Container` type annotation in conftest fixtures was evaluated at runtime on Windows where the docker import is guarded. Adding `from __future__ import annotations` makes all annotations lazy strings. Also adds 3.11 and 3.13 to the Windows CI matrix for full coverage. Co-Authored-By: Claude Opus 4.6 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6ed32c7f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The fakeredis sleep workarounds in strikelist and worker are only reachable on the memory:// backend. Real Redis backends block server-side, so these lines are never hit — causing coverage failures across all non-memory CI matrix legs. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Too fragile for CI — CPU contention on shared runners makes process_time() measurements unreliable. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The pragma: no cover was on the inner `if memory://` check but the uncovered line was the `return` in the outer `if not streams` block. Move the pragma to the outer if so the whole branch is excluded — on real Redis, xread blocks for 60s so this branch is never taken in tests. Also add tzdata as a conditional Windows dependency so zoneinfo can find IANA timezone names (Windows doesn't have /usr/share/zoneinfo). Relates to #339
…servers On Windows, select.select() only works with actual sockets — if the context manager exits and server_close() invalidates the socket while serve_forever is still selecting on it, you get WinError 10038. Calling shutdown() first signals serve_forever to exit its loop cleanly. Relates to #339
- Calibrate test_task_duration_is_measured against actual platform sleep behavior instead of hardcoding 0.1-0.2s bounds (asyncio.sleep can return early on Windows) - Widen redelivery_timeout in test_workers_with_same_redelivery_timeout from 200ms to 1s — gives lease renewal more margin on Windows where event loop timer resolution is ~15ms, without changing test duration - Add test_worker_drains_active_tasks_on_shutdown to reliably exercise the finally block drain path (worker.py:608) using event handshake - Set Windows 3.11 coverage threshold to 98% matching non-Windows 3.11 (known coverage.py reporting instability on 3.11) Relates to #339
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The redelivery counter test used 50ms timeout with 75ms sleep (25ms margin), and the abandoned worker test used 100ms/125ms. Windows ~15ms timer resolution makes these too tight, causing worker crashes in CI. Widened both to 200ms/250ms. Co-Authored-By: Claude Opus 4.6 <[email protected]>
redis-py 7.2.0 (released Feb 16) broke async cluster initialization — their internal NodesManager passes `lib_name` to ClusterNode.__init__ which now raises a deprecation error via the new DriverInfo API (#3880). We'll unpin once they fix it. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The cluster include entries need to match the base matrix's redis-py-version exactly, otherwise GHA treats them as new combinations and they lose cov-threshold from the python-version includes. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
redis-py 7.2.0 deprecated `lib_name`/`lib_version` on `Connection.__init__` in favor of `DriverInfo`, but their own `RedisCluster` still passes `lib_name="redis-py"` internally, which triggers a `DeprecationWarning`. Our `filterwarnings = ["error"]` promotes that to an exception, breaking cluster tests. We pinned `<7.2` in #340 as a stopgap. This adds a targeted warning filter for that specific deprecation so we can run with redis-py 7.2.0 unpinned. Also bumps the Redis version in CI from 8.0 to 8.6 to track the latest release. Closes #341 Co-Authored-By: Claude Opus 4.6 <[email protected]>
redis-py 7.2.0 deprecated `lib_name`/`lib_version` on `Connection.__init__` in favor of `DriverInfo`, but their own `RedisCluster` still passes `lib_name="redis-py"` internally, which triggers a `DeprecationWarning`. Our `filterwarnings = ["error"]` promotes that to an exception, breaking cluster tests. We pinned `<7.2` in #340 as a stopgap. This adds a targeted warning filter for that specific deprecation so we can run with redis-py 7.2.0 unpinned. Also bumps the Redis version in CI from 8.0 to 8.6 to track the latest release. Closes #341 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
Docket's worker loops rely on Redis blocking behavior that fakeredis
doesn't implement —
get_message()andxread()return immediatelyinstead of blocking, turning the loops into CPU spinners. The
xreadgrouploop already had a workaround; this applies the samepattern to the cancellation listener and strike monitor.
Also fixes a few things that prevent running on Windows at all: signal
handler registration (raises
NotImplementedErroron Windows insteadof working),
fcntlimport (doesn't exist on Windows — swapped forcross-platform locking with
msvcrt), and Docker SDK imports inconftest that fail when Docker isn't installed.
Adds a Windows CI job (memory:// only, 3 Python versions) and a
regression test that directly measures idle CPU usage.
Closes #339
🤖 Generated with Claude Code