-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disable socket wakeup when sending requests #13221
Conversation
…13265) * disable wakeup list groups and send * add comment * fix getting kafka version, call patched method * clean up * fix getting ListGroupsRequest by version
I wonder: since it's clearly hard to come up with tests for this, we could try to add a big comment that explains the rationale for the changes (link to support case etc). Just so that we know exactly which parts we shouldn't touch in the future, which is what the tests would tell us as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me but left a nit comment
* Disable socket wakeup when sending requests * Disable socket wakeup when listing consumer groups and send request (#13265) * disable wakeup list groups and send * add comment * fix getting kafka version, call patched method * clean up * fix getting ListGroupsRequest by version * Add comment Co-authored-by: Fanny Jiang <fanny.jiang@datadoghq.com>
What does this PR do?
This PR disables socket wakeup when sending requests to the kafka-python library we use for the kafka_consumer check. Since our check runs in a single thread, it doesn't need to do a socket wakeup. This was also causing issues for set ups with high number of brokers per cluster:
Motivation
Additional Notes
dpkp/kafka-python#1761 added the ability to disable socket wakeups with the
wakeup
argument to fix theKafkaTimeoutError
error, although the fix wasn't being applied to the kafka-python function_send_request_to_node()
which we use, so we needed to reimplement it by hand.Here is the original Github issue: dpkp/kafka-python#1760
I've also opened a Github PR on kafka-python to add the
wakeup
argument to the_send_request_to_node()
function: dpkp/kafka-python#2335Review checklist (to be filled by reviewers)
changelog/
andintegration/
labels attachedqa/skip-qa
label.