Repository navigation
fix(scheduler): initialize RabbitMQ IO thread before shutdown - #2532
Open
linsheng-li wants to merge 1 commit into
Open
linsheng-li wants to merge 1 commit into
linsheng-li wants to merge 1 commit into
Conversation
Collaborator
🤖 Open Code ReviewTarget: PR #2532 ✅ OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s). Generated by cloud-assistant via Open Code Review. |
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.
Description
Stopping a scheduler that has never initialized RabbitMQ logs
AttributeError: 'OptimizedScheduler' object has no attribute '_io_loop_thread'during API shutdown. This includes local-queue deployments and initialization skipped in CI. No restart is required to trigger it.The constructor initializes
_rabbitmq_io_loop_thread, but bothinitialize_rabbitmq()andrabbitmq_close()use_io_loop_thread. Initialize the same field in the constructor. The production change is one line; connection cleanup, thread joins, shutdown ordering, and exception handling are unchanged. No new dependencies or configuration are required.Related issue: N/A; the reproduction and regression tests are included here.
Type of change
How Has This Been Tested?
pytest tests/mem_scheduler/test_rabbitmq_shutdown.py -q— 5 passed. Without the fix, 3 failed, 2 passed. Covers uninitialized/repeated close, CI initialization skip, connection cleanup with a real IO thread, native optimized-scheduler lifecycle cleanup, and propagation of unexpected join errors.make format— all Ruff checks passed; 631 files unchanged.Additional directory run:
pytest tests/mem_scheduler/ -q— 70 passed, 1 skipped, 1 failed. The existingtest_dynamic_cache_layers_accessrequires PyTorch, which is absent in the test image; the same failure was reproduced against the unmodified source. Pytest also warns that the image lacks the asyncio plugin. The full repository suite was not run.Checklist