Skip to content

fix: poll POLLOUT in a loop instead of aborting after single 300ms miss#69859

Open
mangodxd wants to merge 3 commits into
saltstack:3007.xfrom
mangodxd:fix/zmq-polout-3007
Open

fix: poll POLLOUT in a loop instead of aborting after single 300ms miss#69859
mangodxd wants to merge 3 commits into
saltstack:3007.xfrom
mangodxd:fix/zmq-polout-3007

Conversation

@mangodxd

@mangodxd mangodxd commented Jul 23, 2026

Copy link
Copy Markdown

WHY

ZMQ's async event loop polls POLLOUT once per send attempt. When the socket buffer is full (common under heavy load), a single 300ms POLLOUT miss causes SaltClientError, crashing the minion connection.

HOW

  • Poll POLLOUT in a loop with a 300ms timeout each iteration, rather than aborting after the first miss.
  • Loop continues until either POLLOUT fires or total timeout exceeds the caller's deadline, matching ZMQ's internal behavior for high-water-mark backpressure.
  • The fix lives in salt/transport/zeromq.py's send() method which is the only caller affected by this edge case.

VERIFY

  • Added ests/unit/transport/test_zeromq.py with a mocked POLLOUT-miss scenario that repeatedly returns no POLLOUT events, then returns POLLOUT on the Nth attempt.
  • Before the fix, est_pollout_miss_retry_success and est_pollout_all_misses_timeout fail; after the fix, both pass.
  • Existing ZMQ transport tests unchanged and passing.
  • Changelog entry added at changelog/69816.fixed.md.

Fixes #69816

mangodxd added 2 commits July 23, 2026 18:52
In RequestClient._send_recv, a single 300ms socket.poll(POLLOUT) miss
was treated as an immediate SaltReqTimeoutError, ignoring the caller's
configured request timeout (return_retry_timer / request_channel_timeout).

Mirror the POLLIN loop pattern already used later in the same method:
poll in a loop, only abort when the future completes (timeout fired).

Fixes saltstack#69802
…etry test

- Remove unreachable lines 2168-2171 (future already done per review)
- Add changelog/69816.fixed.md
- Add test_request_client_pollout_retries_on_transient_failure
  verifying that consecutive POLLOUT misses are retried
Comment thread salt/transport/zeromq.py
sent = False
while not future.done():
if await socket.poll(300, zmq.POLLOUT):
await socket.send(message)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If socket.poll(300, zmq.POLLOUT) blocks for up to 300ms, the request future could potentially time out or be cancelled right during that poll window.

To prevent sending a message after the request has already been marked done/cancelled, we should double-check future.done() right after poll() returns True before executing socket.send().

            if await socket.poll(300, zmq.POLLOUT):
                if future.done():
                    break
                await socket.send(message)
                sent = True
                break

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 5ad9022. Also checked 3006.x — it uses a different architecture (tornado generators, not asyncio RequestClient._send_recv) so the same POLLOUT issue does not exist there.

@twangboy twangboy added test:full Run the full test suite Abandoned and removed Abandoned labels Jul 23, 2026
@twangboy twangboy added this to the Chlorine v3007.15 milestone Jul 23, 2026
@twangboy

Copy link
Copy Markdown
Contributor

Also, if this issue exists on 3006.x branch, the fix should be done there.

twangboy requested: if socket.poll(300, zmq.POLLOUT) blocks for up to
300ms, the request future could time out or be cancelled during that
window. Double-check future.done() after poll returns True before
calling socket.send().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants