From ca8702173424f343ddd0bfec0b817f39c30d4549 Mon Sep 17 00:00:00 2001 From: Stephen Connolly Date: Tue, 27 Sep 2016 15:11:17 +0100 Subject: [PATCH 1/2] [FIXED JENKINS-38539] Turn on SO_KEEPALIVE and provide CLI option to turn it off again - Most OSes have a default SO_KEEPALIVE of 2 hours, and perform keepalive without generating any significant traffic The master side of the connection already has SO_KEEPALIVE enabled, this just allows both OSes to keep their own guidance and therefore assist when tuning the agent side is more appropriate than changing the kernel parameters on the master side (as the master is handling the HTTP requests of users) - Would probably be perfectly safe to not even expose the -noKeepAlive option as the SO_KEEPALIVE should be invisible But there may be users that have the requirement to disable, so safer to provide the option - Change was developed against the stable-2.x branch --- src/main/java/hudson/remoting/Engine.java | 26 ++++++++++++++++++++ src/main/java/hudson/remoting/Launcher.java | 7 ++++++ src/main/java/hudson/remoting/jnlp/Main.java | 5 ++++ 3 files changed, 38 insertions(+) diff --git a/src/main/java/hudson/remoting/Engine.java b/src/main/java/hudson/remoting/Engine.java index 3ec197183..a17f41dc0 100644 --- a/src/main/java/hudson/remoting/Engine.java +++ b/src/main/java/hudson/remoting/Engine.java @@ -138,6 +138,13 @@ public void run() { private boolean noReconnect; + /** + * Determines whether the socket will have {@link Socket#setKeepAlive(boolean)} set or not. + * + * @since TODO + */ + private boolean keepAlive = true; + private JarCache jarCache = new FileSystemJarCache(new File(System.getProperty("user.home"),".jenkins/cache/jars"),true); public Engine(EngineListener listener, List hudsonUrls, String secretKey, String slaveName) { @@ -178,6 +185,24 @@ public void setNoReconnect(boolean noReconnect) { this.noReconnect = noReconnect; } + /** + * Returns {@code true} if and only if the socket to the master will have {@link Socket#setKeepAlive(boolean)} set. + * + * @return {@code true} if and only if the socket to the master will have {@link Socket#setKeepAlive(boolean)} set. + */ + public boolean isKeepAlive() { + return keepAlive; + } + + /** + * Sets the {@link Socket#setKeepAlive(boolean)} to use for the connection to the master. + * + * @param keepAlive the {@link Socket#setKeepAlive(boolean)} to use for the connection to the master. + */ + public void setKeepAlive(boolean keepAlive) { + this.keepAlive = keepAlive; + } + public void setCandidateCertificates(List candidateCertificates) { this.candidateCertificates = candidateCertificates == null ? null @@ -409,6 +434,7 @@ private Socket connect(@CheckForNull String host, @CheckForNull String port) thr s.connect(targetAddress); s.setTcpNoDelay(true); // we'll do buffering by ourselves + s.setKeepAlive(keepAlive); // set read time out to avoid infinite hang. the time out should be long enough so as not // to interfere with normal operation. the main purpose of this is that when the other peer dies diff --git a/src/main/java/hudson/remoting/Launcher.java b/src/main/java/hudson/remoting/Launcher.java index 399d8a589..a41264985 100644 --- a/src/main/java/hudson/remoting/Launcher.java +++ b/src/main/java/hudson/remoting/Launcher.java @@ -189,6 +189,10 @@ public boolean verify(String s, SSLSession sslSession) { @Option(name="-noReconnect",usage="Doesn't try to reconnect when a communication fail, and exit instead") public boolean noReconnect = false; + @Option(name = "-noKeepAlive", + usage = "Do not open the socket to the master with SO_KEEPALIVE enabled") + public boolean noKeepAlive = false; + public static void main(String... args) throws Exception { Launcher launcher = new Launcher(); CmdLineParser parser = new CmdLineParser(launcher); @@ -229,6 +233,9 @@ public void run() throws Exception { if (this.noReconnect) { jnlpArgs.add("-noreconnect"); } + if (this.noKeepAlive) { + jnlpArgs.add("-noKeepAlive"); + } try { hudson.remoting.jnlp.Main._main(jnlpArgs.toArray(new String[jnlpArgs.size()])); } catch (CmdLineException e) { diff --git a/src/main/java/hudson/remoting/jnlp/Main.java b/src/main/java/hudson/remoting/jnlp/Main.java index 8f061984a..e0965ac14 100644 --- a/src/main/java/hudson/remoting/jnlp/Main.java +++ b/src/main/java/hudson/remoting/jnlp/Main.java @@ -85,6 +85,10 @@ public class Main { usage="If the connection ends, don't retry and just exit.") public boolean noReconnect = false; + @Option(name="-noKeepAlive", + usage="Do not open the socket to the master with SO_KEEPALIVE enabled") + public boolean noKeepAlive = false; + @Option(name = "-cert", usage = "Specify additional X.509 encoded PEM certificates to trust when connecting to Jenkins " + "root URLs. If starting with @ then the remainder is assumed to be the name of the " + @@ -173,6 +177,7 @@ public Engine createEngine() { if(jarCache!=null) engine.setJarCache(new FileSystemJarCache(jarCache,true)); engine.setNoReconnect(noReconnect); + engine.setKeepAlive(!noKeepAlive); if (candidateCertificates != null && !candidateCertificates.isEmpty()) { CertificateFactory factory; try { From 2c8824a6f2a17d66715adf626d17a44faf4558ec Mon Sep 17 00:00:00 2001 From: Stephen Connolly Date: Tue, 27 Sep 2016 16:31:57 +0100 Subject: [PATCH 2/2] [JENKINS-38541] Tweak help text for CLI -noKeepAlive option --- src/main/java/hudson/remoting/Launcher.java | 2 +- src/main/java/hudson/remoting/jnlp/Main.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/remoting/Launcher.java b/src/main/java/hudson/remoting/Launcher.java index a41264985..06e5ab361 100644 --- a/src/main/java/hudson/remoting/Launcher.java +++ b/src/main/java/hudson/remoting/Launcher.java @@ -190,7 +190,7 @@ public boolean verify(String s, SSLSession sslSession) { public boolean noReconnect = false; @Option(name = "-noKeepAlive", - usage = "Do not open the socket to the master with SO_KEEPALIVE enabled") + usage = "Disable TCP socket keep alive on connection to the master.") public boolean noKeepAlive = false; public static void main(String... args) throws Exception { diff --git a/src/main/java/hudson/remoting/jnlp/Main.java b/src/main/java/hudson/remoting/jnlp/Main.java index e0965ac14..3b5fb8099 100644 --- a/src/main/java/hudson/remoting/jnlp/Main.java +++ b/src/main/java/hudson/remoting/jnlp/Main.java @@ -86,7 +86,7 @@ public class Main { public boolean noReconnect = false; @Option(name="-noKeepAlive", - usage="Do not open the socket to the master with SO_KEEPALIVE enabled") + usage="Disable TCP socket keep alive on connection to the master.") public boolean noKeepAlive = false; @Option(name = "-cert",