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.

Sign up for GitHub

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

Jump to bottom

Add per-socket keepalive options #44055

Merged
merged 16 commits into from
Aug 5, 2019
Merged

Add per-socket keepalive options #44055

merged 16 commits into from
Aug 5, 2019

Conversation

ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Jul 8, 2019

Uses JDK 11's per-socket configuration of TCP keepalive (supported on Linux and Mac), see https://bugs.openjdk.java.net/browse/JDK-8194298, and exposes these as transport settings. By default, these options are disabled for now (i.e. fall-back to OS behavior), but I would like to explore whether we can enable them by default, in particular to force keepalive configurations that are better tuned for running ES.

@ywelsch ywelsch added >enhancement :Distributed/Network Http and internode communication implementations v8.0.0 labels Jul 8, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@original-brownbear
Copy link
Member

@ywelsch this is failing third party tests:

10:27:39 > Task :libs:elasticsearch-nio:forbiddenApisMain FAILED
10:27:39 Forbidden class/interface use: jdk.net.ExtendedSocketOptions [non-portable or internal runtime class]
10:27:39   in org.elasticsearch.nio.ChannelFactory$RawChannelFactory (ChannelFactory.java:226)
10:27:39 Forbidden class/interface use: jdk.net.ExtendedSocketOptions [non-portable or internal runtime class]
10:27:39   in org.elasticsearch.nio.ChannelFactory$RawChannelFactory (ChannelFactory.java:227)
10:27:39 Forbidden class/interface use: jdk.net.ExtendedSocketOptions [non-portable or internal runtime class]
10:27:39   in org.elasticsearch.nio.ChannelFactory$RawChannelFactory (ChannelFactory.java:229)
10:27:39 Forbidden class/interface use: jdk.net.ExtendedSocketOptions [non-portable or internal runtime class]
10:27:39   in org.elasticsearch.nio.ChannelFactory$RawChannelFactory (ChannelFactory.java:230)
10:27:39 Forbidden class/interface use: jdk.net.ExtendedSocketOptions [non-portable or internal runtime class]
10:27:39   in org.elasticsearch.nio.ChannelFactory$RawChannelFactory (ChannelFactory.java:232)
10:27:39 Forbidden class/interface use: jdk.net.ExtendedSocketOptions [non-portable or internal runtime class]
10:27:39   in org.elasticsearch.nio.ChannelFactory$RawChannelFactory (ChannelFactory.java:233)
10:27:39 Scanned 28 class file(s) for forbidden API invocations (in 0.23s), 6 error(s).
10:27:39

@ywelsch
Copy link
Contributor Author

ywelsch commented Jul 9, 2019

@elasticmachine test this please

@Tim-Brooks
Copy link
Contributor

I started looking at this today and should be able to finishing reviewing it before Monday.

Tim-Brooks
Tim-Brooks approved these changes Aug 5, 2019
View reviewed changes
Copy link
Contributor

@Tim-Brooks Tim-Brooks left a comment

Choose a reason for hiding this comment

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

LGTM

original-brownbear
original-brownbear approved these changes Aug 5, 2019
View reviewed changes
Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

LGTM2

@ywelsch ywelsch added the v7.4.0 label Aug 5, 2019
@ywelsch ywelsch merged commit 245cb34 into elastic:master Aug 5, 2019
ywelsch added a commit that referenced this pull request Aug 6, 2019
@ywelsch
Add per-socket keepalive options (#44055)
7aeb2fe 
Uses JDK 11's per-socket configuration of TCP keepalive (supported on Linux and Mac), see
https://bugs.openjdk.java.net/browse/JDK-8194298, and exposes these as transport settings.
By default, these options are disabled for now (i.e. fall-back to OS behavior), but we would like
to explore whether we can enable them by default, in particular to force keepalive configurations
that are better tuned for running ES.
ywelsch added a commit that referenced this pull request Aug 6, 2019
@ywelsch
Run testExtendedSocketOptions only on JDK11+ (#44055)
a453cd4 
This functionality only works on JDK 11 or higher
ywelsch added a commit that referenced this pull request Aug 6, 2019
@ywelsch
Fix tests after backport of #44055
1784621
@colings86
Copy link
Contributor

It looks like the backporting is complete here so I have removed the backport pending label while generating 7.4.0 release notes. Please let me know if this is wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaveCTurner DaveCTurner DaveCTurner left review comments

@Tim-Brooks Tim-Brooks Tim-Brooks approved these changes

@original-brownbear original-brownbear original-brownbear approved these changes

Assignees
No one assigned
Labels
:Distributed/Network Http and internode communication implementations >enhancement v7.4.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ywelsch @elasticmachine @original-brownbear @Tim-Brooks @colings86 @DaveCTurner @jakelandis