Hold the dev backend listen socket in the granian supervisor - #7114
Hold the dev backend listen socket in the granian supervisor#7114FarhanAliRaza wants to merge 1 commit into
Conversation
On Linux granian 2.8 hands each worker a socket spec and the worker binds with SO_REUSEPORT only after it has imported and compiled the app. The file-watch reload stops the old worker before spawning the new one, so nothing listens for the whole boot and every request in that window is refused. Granian 2.7.4, the minimum we pin, bound the socket in the supervisor. Override `_init_shared_socket` in dev to bind once in the supervisor and pass the inheritable descriptor to workers, the path granian already uses on macOS and Windows. Requests sent during a reload now wait in the accept backlog and are answered by the new worker. Measured on a 60-page app in full dev mode, probing /ping every 50 ms across two edits: 21-22 refused probes between +0.20 s and +1.26 s before, 0 refused and 22-24 held with a 1.3 s maximum wait after. Single-worker throughput is unchanged (4071 vs 4017 keep-alive req/s). Claude-Session: https://claude.ai/code/session_01CtZAjq1esmYw7iRHd5TAbG
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def serve(self): | ||
| pass | ||
|
|
||
| mocker.patch.object(granian_server, "Server", FakeGranian) | ||
|
|
||
| exec_utils.run_granian_backend( | ||
| host="127.0.0.1", port=0, loglevel=exec_utils.LogLevel.INFO | ||
| ) | ||
|
|
||
| (server,) = servers | ||
| server._init_shared_socket() # pyright: ignore[reportAttributeAccessIssue] |
There was a problem hiding this comment.
The fake serve() is a no-op, and the test manually initializes and listens on the socket. It therefore does not exercise Granian's worker descriptor handoff or a real reload cycle. A regression that closes the supervisor descriptor during worker shutdown or fails to pass it to the replacement worker could still pass this test, so the intended hot-reload behavior lacks direct coverage.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Merging this PR will not alter performance
Comparing 
Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/units/utils/test_exec.py">
<violation number="1" location="tests/units/utils/test_exec.py:120">
P3: This test runs entirely in one process and never simulates the supervisor→worker handoff or a reload restart, so it doesn't exercise the queuing-across-restart behavior the PR title and docstring claim to verify. It only shows that a locally bound socket, once `listen()` is called, will accept a queued connection — which is true of any listening TCP socket — plus that `_sso` is inheritable. A regression that bound the socket correctly but broke the cross-process handoff would still pass. Consider an integration-style case that starts the server, kills/restarts the worker, and asserts a connection is held (not refused) during the reload window, matching the PR's measurement approach.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ) | ||
|
|
||
| (server,) = servers | ||
| server._init_shared_socket() # pyright: ignore[reportAttributeAccessIssue] |
There was a problem hiding this comment.
P3: This test runs entirely in one process and never simulates the supervisor→worker handoff or a reload restart, so it doesn't exercise the queuing-across-restart behavior the PR title and docstring claim to verify. It only shows that a locally bound socket, once listen() is called, will accept a queued connection — which is true of any listening TCP socket — plus that _sso is inheritable. A regression that bound the socket correctly but broke the cross-process handoff would still pass. Consider an integration-style case that starts the server, kills/restarts the worker, and asserts a connection is held (not refused) during the reload window, matching the PR's measurement approach.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/utils/test_exec.py, line 120:
<comment>This test runs entirely in one process and never simulates the supervisor→worker handoff or a reload restart, so it doesn't exercise the queuing-across-restart behavior the PR title and docstring claim to verify. It only shows that a locally bound socket, once `listen()` is called, will accept a queued connection — which is true of any listening TCP socket — plus that `_sso` is inheritable. A regression that bound the socket correctly but broke the cross-process handoff would still pass. Consider an integration-style case that starts the server, kills/restarts the worker, and asserts a connection is held (not refused) during the reload window, matching the PR's measurement approach.</comment>
<file context>
@@ -80,6 +81,55 @@ def serve(self):
+ )
+
+ (server,) = servers
+ server._init_shared_socket() # pyright: ignore[reportAttributeAccessIssue]
+ listener: socket.socket = server._sso # pyright: ignore[reportAttributeAccessIssue]
+ try:
</file context>
1 participant
Problem
On Linux, granian 2.8 gives each worker a bare socket spec. The worker binds with
SO_REUSEPORTonly after it has imported and compiled the app, and the file-watch reload stops the old worker before it spawns the new one. Nothing listens during the whole worker boot, so every request made while hot reload runs gets connection refused. Granian 2.7.4, the minimum version we pin, bound the socket in the supervisor, so this is a behaviour change we inherited.Change
run_granian_backend(dev only) subclasses the granian server and overrides_init_shared_socketto bind once in the supervisor and pass the inheritable descriptor down to workers. This is the path granian already takes on macOS and Windows. Requests during a reload now wait in the kernel accept backlog and are answered by the new worker. Production (run_granian_backend_prod) is untouched.Measurements
Probing
/pingevery 50 ms across two edits, 60-page app, full dev mode:Blank app, backend-only: 0 refused, longest wait 0.5 s. Reload cycle length is unchanged; only the failure mode changes.
Single-worker throughput, blank app
/ping, best of 3: 4071 vs 4017 req/s keep-alive, 3097 vs 3059 req/s new connection per request. Noise.Trade-off
When an edit has a syntax error the worker dies and nothing accepts. Before, requests in that state were refused immediately. Now they wait until the client times out, and once the file is fixed the queued request is answered about 0.6 s later.
https://claude.ai/code/session_01CtZAjq1esmYw7iRHd5TAbG