Skip to content

[fix] Keep the storage worker alive when a request fails outside its handlers - #204

Open
huniu20 wants to merge 1 commit into
Ascend:mainfrom
huniu20:fix/storage-worker-survives-request-failure
Open

huniu20 wants to merge 1 commit into
Ascend:mainfrom
huniu20:fix/storage-worker-survives-request-failure

Conversation

@huniu20

@huniu20 huniu20 commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

A storage unit has one worker thread. recv_multipart and the final send_multipart in _process_one_worker_request were not guarded, so one failure there killed the worker: the proxy kept queueing requests nobody answered, and shutdown() hung in zmq_context.term(). Catch per request so only that caller times out.

Also warn once per hang (with ip:port) when a metrics probe times out, since it shares that worker.

…handlers

A storage unit has a single worker thread. `recv_multipart` and the final
`send_multipart` in `_process_one_worker_request` were outside any try block,
so one failure there ended the thread while the proxy kept queueing requests
nobody would answer, and `shutdown()` then blocked in `zmq_context.term()`
on the dead worker's socket. Catch and log per request so only that caller
times out.

Also warn (once per hang, with ip:port) when a metrics probe times out: it is
served by the same worker, so it is the earliest sign a unit stopped answering.

Signed-off-by: huniu20 <hn20@mails.tsinghua.edu.cn>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T09:03:32.884712Z bb01112 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bb0111276c

ℹ️ 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._process_one_worker_request(worker_socket, monitor)
try:
self._process_one_worker_request(worker_socket, monitor)
except Exception as e:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Replace the worker socket after multipart I/O errors

When recv_multipart() or send_multipart() raises after consuming or emitting only part of a multipart message, this handler logs the exception and reuses the same DEALER socket. PyZMQ performs multipart I/O frame by frame, so unread frames or an outstanding SNDMORE envelope remain on that socket; the next request can then treat a payload frame as its identity or append its response to the previous caller's route, producing malformed replies and stranding an additional caller. Discard and recreate the worker socket (and update the poller), or terminate the unit, rather than continuing on a potentially poisoned socket.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant