fix(stream manager): exit gracefully when another worker already bound the port - #2753
fix(stream manager): exit gracefully when another worker already bound the port#2753bar-jpg wants to merge 1 commit into
Conversation
…d the port
The Stream Manager is started from the app module (docker/config/{cpu,gpu}_http.py),
which uvicorn imports once per worker. It binds a fixed STREAM_MANAGER_HOST:PORT
(127.0.0.1:7070 by default), so with NUM_WORKERS > 1 every worker starts a manager
and all but the first die with an unhandled OSError: [Errno 98] Address already in use.
A single manager per container is the correct end state - every worker already builds
a StreamManagerClient pointing at that address - so the losing instances now log and
exit cleanly instead of raising. The bind is also moved ahead of the health-check and
warm-up threads so a losing instance exits before preloading pipelines.
RoboflowTCPServer additionally sets allow_reuse_address, matching http.server.HTTPServer,
so a restarted manager can rebind while the previous socket is in TIME_WAIT. This does
not permit two live listeners, so the port remains an effective mutex.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
|
|
👋 Thanks for the pull request! Here is how automated Claude review works here, so you spend credits (and reviewer time) wisely. 🚧 Right now this is a draft, so automated Claude review is paused — nothing is being spent yet. Mark it Ready for review to trigger it. Warning 💸 The Claude reviewer bills in credits, not vibesAutomated review spins up a real agent that reads real code and spends real credits on every pass. It is glad to help — but it is not a rubber duck, a linter you poke in a loop, or a substitute for reading the contributing guide. Treat it like an expensive senior reviewer whose time you booked, and show up prepared. Draft when unsure, Ready when you mean it:
However you get there, arrive prepared:
Reviews are not free. A draft costs nothing to review; a Ready PR is a promise that it is worth reviewing.
|
|
Status: ❌ Request Changes — Gist: https://gist.github.com/jarbasrf/b335d7ab56a46b4d5c6b4725590add50. Resumo: The PR addresses the duplicate GPU worker bind race, but treating every |
Problem
The Stream Manager is started from the app module —
docker/config/gpu_http.pyanddocker/config/cpu_http.py— which uvicorn imports once per worker. It binds a fixedSTREAM_MANAGER_HOST:STREAM_MANAGER_PORT(127.0.0.1:7070by default,manager_app/app.py), through aTCPServerthat never setallow_reuse_address.So with
NUM_WORKERS > 1, every worker starts a manager, one wins the bind, and the rest die with an unhandled:The GPU/CPU images ship
ENABLE_STREAM_API=TruewithNUM_WORKERS=1, so the default config is safe — this is only reachable when a user raisesNUM_WORKERS, which the docs did not flag as a hazard.Reported by a customer running
roboflow-inference-server-gpuwithNUM_WORKERS=4on an A2, who sees the error at startup along with uvicorn workers dying and respawning, ending up with fewer workers serving than configured and reduced FPS. Their workaround is restarting the container, which just re-runs the same race.Why a single manager is the right end state
Not a regression, and not fixed by upgrading —
docker/config/gpu_http.pyis byte-identical fromv1.0.1throughmain.inference/core/interfaces/http/http_api.pybuilds aStreamManagerClientper worker pointing at the same127.0.0.1:7070. The design is already one manager, many clients — a single manager per container correctly serves all workers. Only the spawn location is wrong.Changes
manager_app/app.py—start()catchesOSErrorwitherrno.EADDRINUSEaround the server construction, logs an explanatory line, and returns cleanly instead of raising. The bind now happens before the health-check and warm-up threads start, so an instance that loses the race exits without preloading pipelines (previouslySTREAM_API_PRELOADED_PROCESSESwould have been honoured by all of them). OtherOSErrors still propagate.manager_app/tcp_server.py—allow_reuse_address = True, matchinghttp.server.HTTPServer, so a restarted manager can rebind while the previous socket lingers inTIME_WAIT. Verified that this does not permit two live listeners, so the port remains an effective mutex and does not mask the condition above.docker/config/{cpu,gpu}_http.py— log once at startup whenENABLE_STREAM_APIis set withNUM_WORKERS > 1, so the behaviour is greppable in exactly the logs users send us.docs/quickstart/docker_configuration_options.md— document the interaction underNUM_WORKERS. Also mentions the existingPRELOAD_HF_IDSconstraint already noted ininference/core/env.py, which had never made it into the docs.test_app.pycovers both the graceful-exit path (asserting pipelines are not warmed up) and that non-EADDRINUSEerrors still surface;test_tcp_server.pycoversallow_reuse_address.Reproduction
No GPU required —
cpu_http.pyhas the identical defect:Before:
[Errno 98]NUM_WORKERS - 1times. After: one manager log line, the rest exit quietly.Notes for reviewers
ENABLE_STREAM_API=Trueships in 8 images (CPU, CPU.dev, GPU, GPU.dev, GPU.3d, TRT, Jetson 5.1.1, Jetson 6.2.0).OSError, so the traceback frames are missing. This removes the conflict either way; if worker deaths persist there is a second cause to chase.spawnimports at startup, but adds stale-lock semantics and a worker-restart edge case — an orphaned manager keeps the port after its parent worker dies, so the lock and the port can disagree. The handling here covers that case correctly. Happy to add it if maintainers prefer.🤖 Generated with Claude Code