Fix EnvServer executor installation#1455
Merged
Merged
Conversation
ApprovabilityVerdict: Approved This is a minimal bug fix that adds a documented utility call ( You can customize Macroscope's approvability policy. Learn more. |
3b867da to
556c341
Compare
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.
Issue
PR #1453 added
scale_executors(concurrency=512)toEnvServer.run_server()so the router/server process could use a larger default executor forasyncio.to_threadwork. The call happens beforeasyncio.run(server.run()), so there is no running event loop yet.verifiers.utils.thread_utils.scale_executors()handles that case by creating/resizing the shared executor, but its documented contract says the caller must later callinstall_default_executor()from inside the real loop.EnvWorker.run()already does that.EnvServer.run()did not, so the server loop kept Python's default executor instead of the scaled one. In the local probe from the bug scan, the server-side loop was still atbefore_install=20, and only changed toafter_install=512afterinstall_default_executor().The practical bug is that the router/server half of #1453's event-loop-lag fix was only partially wired:
scale_executors(512)created the intended executor, but the actual server event loop never used it. Any server-sideasyncio.to_threadwork would still be capped by the default executor rather than the configured 512-worker pool.Fix
Call
install_default_executor()at the start ofEnvServer.run(), which is the first point whereasyncio.run()has created the real event loop. This mirrors the worker lifecycle and keepsrun_server()responsible for pre-loop scaling/uvloop setup.This PR intentionally does not touch
uv.lockand does not add a test.Verification
uv run --frozen ruff formatuv run --frozen ruff check --fixuv run --frozen pytest tests/test_env_server.py -qNote
Fix executor installation in
EnvServer.runby callinginstall_default_executorat startupAdds a call to
install_default_executor()at the start ofEnvServer.run()in env_server.py, before the death pipe monitor and signal handlers are set up. This ensures the process-wide default executor is installed before any async or threaded work begins.Macroscope summarized 556c341.