[FIXED JENKINS-8990] Configurable ping interval

This lets you configure the ping interval for slaves via a system property. (hudson.slaves.ChannelPinger.pingInterval) The default ping interval has been lowered from 10 to 5 minutes. It also moves the ping setup logic into a ComputerListener (out of the JNLP slave Engine class). As a side effect, all slaves will now have a ping instead of just JNLP slaves.
jenkinsci · Mar 23, 2011 · 18327e9 · 18327e9
1 parent 67cd5b4
commit 18327e9
Show file tree

Hide file tree

Showing 2 changed files with 108 additions and 16 deletions.
diff --git a/core/src/main/java/hudson/slaves/ChannelPinger.java b/core/src/main/java/hudson/slaves/ChannelPinger.java
@@ -0,0 +1,107 @@
+package hudson.slaves;
+
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.model.Computer;
+import hudson.model.Slave;
+import hudson.model.TaskListener;
+import hudson.remoting.Callable;
+import hudson.remoting.Channel;
+import hudson.remoting.PingThread;
+
+import java.io.IOException;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.logging.Logger;
+
+/**
+ * Establish a periodic ping to keep connections between {@link Slave slaves}
+ * and the main Jenkins node alive. This prevents network proxies from
+ * terminating connections that are idle for too long.
+ */
+@Extension
+public class ChannelPinger extends ComputerListener {
+    private static final Logger LOGGER = Logger.getLogger(ChannelPinger.class.getName());
+    private static final String SYS_PROPERTY_NAME  = ChannelPinger.class.getName() + ".pingInterval";
+
+    /**
+     * Interval for the ping in minutes.
+     */
+    private int pingInterval = 5;
+
+    public ChannelPinger() {
+        String interval = System.getProperty(SYS_PROPERTY_NAME);
+        if (interval != null) {
+            try {
+                pingInterval = Integer.valueOf(interval);
+            } catch (NumberFormatException e) {
+                LOGGER.warning("Ignoring invalid " + SYS_PROPERTY_NAME + "=" + interval);
+            }
+        }
+    }
+
+    @Override
+    public void preOnline(Computer c, Channel channel, FilePath root, TaskListener listener)  {
+        if (pingInterval < 1) {
+            LOGGER.fine("Slave ping is disabled");
+            return;
+        }
+
+        try {
+            channel.call(new SetUpRemotePing(pingInterval));
+            LOGGER.fine("Set up a remote ping for " + c.getName());
+        } catch (Exception e) {
+            LOGGER.severe("Failed to set up a ping for " + c.getName());
+        }
+
+        // TODO: Set up a local to remote ping too?
+        // If we just want to keep some activity on the channel this doesn't
+        // matter, but if we consider the ping a 'are you alive?' check it
+        // might be useful.
+        //setUpPingForChannel(channel, pingInterval);
+    }
+
+    private static class SetUpRemotePing implements Callable<Void, IOException> {
+        private static final long serialVersionUID = -2702219700841759872L;
+        private int pingInterval;
+        public SetUpRemotePing(int pingInterval) {
+            this.pingInterval = pingInterval;
+        }
+
+        @Override
+        public Void call() throws IOException {
+            setUpPingForChannel(Channel.current(), pingInterval);
+            return null;
+        }
+    }
+
+    private static void setUpPingForChannel(final Channel channel, int interval) {
+        final AtomicBoolean isInClosed = new AtomicBoolean(false);
+        final PingThread t = new PingThread(channel, interval * 60 * 1000) {
+            protected void onDead() {
+                try {
+                    if (isInClosed.get()) {
+                        LOGGER.fine("Ping failed after socket is already closed");
+                    }
+                    else {
+                        LOGGER.info("Ping failed. Terminating the socket.");
+                        channel.close();
+                    }
+                } catch (IOException e) {
+                    LOGGER.severe("Failed to terminate the socket: " + e);
+                }
+            }
+        };
+
+        channel.addListener(new Channel.Listener() {
+            @Override
+            public void onClosed(Channel channel, IOException cause) {
+                LOGGER.fine("Terminating ping thread for " + channel);
+                isInClosed.set(true);
+                t.interrupt();  // make sure the ping thread is terminated
+            }
+        });
+
+        t.start();
+        LOGGER.fine("Ping thread started for " + channel + " with a " + interval + " minute interval");
+    }
+}
diff --git a/remoting/src/main/java/hudson/remoting/Engine.java b/remoting/src/main/java/hudson/remoting/Engine.java
@@ -39,7 +39,6 @@
 import java.util.List;
 import java.util.Collections;
 import java.util.logging.Logger;
-import static java.util.logging.Level.SEVERE;
 
 /**
  * Slave agent engine that proactively connects to Hudson master.
@@ -234,27 +233,13 @@ public void run() {
                     }
                 }
 
-                final Socket socket = s;
                 final Channel channel = new Channel("channel", executor,
                         in,
                         new BufferedOutputStream(s.getOutputStream()));
-                PingThread t = new PingThread(channel) {
-                    protected void onDead() {
-                        try {
-                            if (!channel.isInClosed()) {
-                                LOGGER.info("Ping failed. Terminating the socket.");
-                                socket.close();
-                            }
-                        } catch (IOException e) {
-                            LOGGER.log(SEVERE, "Failed to terminate the socket", e);
-                        }
-                    }
-                };
-                t.start();
+
                 listener.status("Connected");
                 channel.join();
                 listener.status("Terminated");
-                t.interrupt();  // make sure the ping thread is terminated
                 listener.onDisconnect();
 
                 if(noReconnect)