Repository navigation
[fix] Bound controller RPC send/recv so a lost reply cannot hang the caller - #203
Conversation
…caller ## Problem The client's controller RPC pool is created without a timeout, so every controller request (`get_meta`, `kv_retrieve_meta`, `kv_list`, ...) waits on `recv` forever. A DEALER silently drops a reply it can no longer route, so one lost response blocks the caller indefinitely with no error. Storage RPCs are already bounded by `TQ_SIMPLE_STORAGE_SEND_RECV_TIMEOUT`. ## Change - Add `TQ_CONTROLLER_SEND_RECV_TIMEOUT` (seconds, default 200, matching `TQ_SIMPLE_STORAGE_SEND_RECV_TIMEOUT`) and pass it to the client's `controller_rpc_pool`. - On timeout the call raises (`RuntimeError` wrapping `zmq.Again`); the pool already discards a socket whose lease raised, so a late reply cannot be read by the next request. ## Validation - New `test_controller_rpc_raises_when_controller_never_replies`: a ROUTER that never answers makes `get_partition_list` raise within the timeout. - `python -m pytest -q tests --ignore=tests/e2e -k "not yuanrong and not mooncake"`: 612 passed, 3 skipped (baseline 611 passed; the extra one is the new test). - `python -m compileall -q transfer_queue tutorial tests` and ruff pass. Signed-off-by: huniu20 <hn20@mails.tsinghua.edu.cn>
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23d7a7a29f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| self.zmq_context, | ||
| client_id, | ||
| "request_handle_socket", | ||
| timeout=TQ_CONTROLLER_SEND_RECV_TIMEOUT, |
There was a problem hiding this comment.
When a controller is silent long enough to hit this new timeout, pyzmq raises zmq.error.Again from _request_controller; public methods such as async_get_meta and async_set_custom_meta call that helper without a catch, so their synchronous and asynchronous forms now leak Again instead of the documented RuntimeError. The new test only exercises get_partition_list, which has its own blanket wrapper, so translate Again once in _request_controller to give every controller RPC the same public failure contract.
Useful? React with 👍 / 👎.
Controller RPCs had no timeout, so a lost reply blocked the caller forever. Add
TQ_CONTROLLER_SEND_RECV_TIMEOUT(default 200s, same asTQ_SIMPLE_STORAGE_SEND_RECV_TIMEOUT) to the client's controller socket pool.