Block broker thread on event queue instead of IO if IO is irrelevant in current state #1870
Conversation
…in current state This fixes the case where idle (typically bootstrap) broker threads are consuming CPU on Windows (with a low socket.blocking.max.ms) due to aggressively short timeouts on IO. The drawback is that it might take up to 1000ms to detect disconnects on idle connections, which should not be a problem in practise.
|
Is this PR under review? |
There was a problem hiding this comment.
@edenhill I found it a bit odd to only introduce the 1 second wake-ups when blocking forever. Other than that LGTM
| * When blocking on the queue we will want to detect IO disconnects | ||
| * eventually. | ||
| * 1 wakeup/second is reasonable. */ | ||
| if (unlikely(remains_ms == RD_POLL_INFINITE)) |
There was a problem hiding this comment.
Does it make sense to defend against all 'excessive' values for abs_timeout as opposed to just RD_POLL_INFINITE? A value of 300000ms still seems fairly intrusive given we would wakeup infinite blocking calls every 1 second.
There was a problem hiding this comment.
Right, so a MIN(remains_ms, 1000) makes sense!
|
I'm not sure this is the best approach. PR #1930 enables low-latency wakeups on Windows by using TCP sockets, so that might a better way forward now. |
|
Is this still needed since the merger of #1930? If not should we go ahead and close this one out |
|
I'm keeping this open as a reminder to check if it still applies |
|
|
This fixes the case where idle (typically bootstrap) broker threads
are consuming CPU on Windows (with a low socket.blocking.max.ms) due
to aggressively short timeouts on IO.
The drawback is that it might take up to 1000ms to detect disconnects
on idle connections, which should not be a problem in practice.
Related issue: