Repository navigation
Router wasn't propagating the client disconnects fully to the child, the response wait could cause a delayed cancellation - #29707
Conversation
…the response wait could cause a delayed cancellation
|
Hi @Me42019, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
ggml-gh-bot
Bot
added
the
draft
github-actions
Bot
removed
the
draft
Me42019
changed the title
ServeurpersoCom
left a comment
ServeurpersoCom
left a comment
There was a problem hiding this comment.
LGTM, nice catch: in streaming mode the router already notices a disconnect on the next chunk write and the child stops right away, but in non-streaming mode the child writes nothing until the end, so the router never noticed and the child kept generating for nobody. With cli->stop() the child now sees the closed connection through the same should_stop and cancel path as a direct client, and the deadline makes that check happen every second even when other slots are busy, so both modes now behave the same.
| def test_router_disconnect_stops_child_generation(): | ||
| global server | ||
| server.server_slots = True | ||
| server.n_ctx = 8192 | ||
| server.n_threads = 1 | ||
| server.start() |
There was a problem hiding this comment.
this test is quite time-sensitive so I don't think it will be reliable on CI. maybe just remove this test for now
There was a problem hiding this comment.
Removed the timing-sensitive test as suggested. Thanks.
Assisted-by: Codex
… child, the response wait could cause a delayed cancellation (ggml-org#29707) * Router wasn't propagating the client disconnects fully to the child, the response wait could cause a delayed cancellation * tests: satisfy router disconnect type check * server : remove timing-sensitive router cancellation test Assisted-by: Codex
… child, the response wait could cause a delayed cancellation (ggml-org#29707) * Router wasn't propagating the client disconnects fully to the child, the response wait could cause a delayed cancellation * tests: satisfy router disconnect type check * server : remove timing-sensitive router cancellation test Assisted-by: Codex
When someone closes a request to the llama.cpp router while a model is still generating, the router should stop the request it sent to the child llama-server. Before this fix, the router stopped reading the child’s response but left its downstream HTTP request running. The child could keep generating tokens for a client that had gone away.
The fix closes both gaps in that cancellation path. The router proxy now calls the existing HTTP client’s stop() method when it cleans up.The child’s response wait now uses a fixed timeout, so other wakeups cannot indefinitely postpone its check for a closed connection. The regression test starts generation through the router, disconnects the client, and checks that the child slot becomes idle.
Cancellation is not necessarily instantaneous: a token already in flight may finish while the signal travels through the router. In the live HIP test, the child stopped and released its slot about 0.4 seconds after the upstream socket closed; a normal request through the router still worked afterward.
AI supported the investigation, implementation, and testing, while I focused on reviewing and understanding the changes.