Skip to content
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

Sleep if readable sockets are empty #1684

Closed
wants to merge 1 commit into from
Closed

Conversation

okkez
Copy link
Contributor

@okkez okkez commented Sep 6, 2017

This can reduce CPU usage following case:

[Sender] -(1)-> [ELB] -(2)-> [Aggregator]

Sender enables require_ack_response.

If (2) is disconnected, but sender send logs to ELB and wait for ack responses. Soeckets that are waiting for ack response cannot be readable for long time.

This will cause busy loop here.

@@ -493,7 +493,10 @@ def ack_reader
end

readable_sockets, _, _ = IO.select(sockets, nil, nil, select_interval)
next unless readable_sockets
unless readable_sockets
Copy link
Member

Choose a reason for hiding this comment

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

Could you add comment why this sleep is needed?

@okkez
Copy link
Contributor Author

okkez commented Sep 6, 2017

I noticed that this PR cannot fix #1665. 🙇

@okkez okkez closed this Sep 12, 2017
@okkez okkez deleted the fix-1665 branch September 12, 2017 04:29
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.

2 participants