Do not set SO_LINGER to 0 when not shutting down#26871
Merged
Tim-Brooks merged 2 commits intoelastic:masterfrom Oct 4, 2017
Merged
Do not set SO_LINGER to 0 when not shutting down#26871Tim-Brooks merged 2 commits intoelastic:masterfrom
Tim-Brooks merged 2 commits intoelastic:masterfrom
Conversation
This is a follow up to elastic#26764. That commit set SO_LINGER to 0 in order to fix a scenario where we were running out of resources during CI. We are primarily interested in setting this to 0 when stopping the tranport. Allowing TIMED_WAIT is standard for other failure scenarios during normal operation. Unfortunately this commit set SO_LINGER to 0 every time we close NodeChannels. NodeChannels can be closed in case of an exception or other failures (such as parsing a response). We want to only disable linger when actually shutting down.
original-brownbear
approved these changes
Oct 3, 2017
Contributor
original-brownbear
left a comment
There was a problem hiding this comment.
makes sense imo => LGTM :)
s1monw
approved these changes
Oct 4, 2017
Contributor
s1monw
left a comment
There was a problem hiding this comment.
left a suggestion LGTM otherwise
| private final AtomicLong requestIdGenerator = new AtomicLong(); | ||
| private final CounterMetric numHandshakes = new CounterMetric(); | ||
| private static final String HANDSHAKE_ACTION_NAME = "internal:tcp/handshake"; | ||
| private volatile boolean closingTransport = false; |
Contributor
There was a problem hiding this comment.
I think we can just use this.stopped() instead of having a separate boolean? we are setting the state value before we execute the doStop method
Contributor
Author
There was a problem hiding this comment.
Great suggestion. I made that change.
Tim-Brooks
added a commit
that referenced
this pull request
Oct 4, 2017
This is a follow up to #26764. That commit set SO_LINGER to 0 in order to fix a scenario where we were running out of resources during CI. We are primarily interested in setting this to 0 when stopping the tranport. Allowing TIMED_WAIT is standard for other failure scenarios during normal operation. Unfortunately this commit set SO_LINGER to 0 every time we close NodeChannels. NodeChannels can be closed in case of an exception or other failures (such as parsing a response). We want to only disable linger when actually shutting down.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a follow up to #26764. That commit set SO_LINGER to 0 in order
to fix a scenario where we were running out of resources during CI. We
are primarily interested in setting this to 0 when stopping the
tranport. Allowing TIMED_WAIT is standard for other failure scenarios
during normal operation.
Unfortunately this commit set SO_LINGER to 0 every time we close
NodeChannels. NodeChannels can be closed in case of an exception or
other failures (such as parsing a response). We want to only disable
linger when actually shutting down.