diff --git a/core/pom.xml b/core/pom.xml
index 0f63887062f6..f85e960e8175 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -585,10 +585,15 @@ THE SOFTWARE.
/usr/local/yjp/lib/yjp.jar
-
- com.google.guava
- guava
-
+
+ com.google.guava
+ guava
+
+
+ com.google.guava
+ guava-testlib
+ test
+
com.jcraft
diff --git a/core/src/main/java/hudson/slaves/ChannelPinger.java b/core/src/main/java/hudson/slaves/ChannelPinger.java
index 49ae7c1cf997..aa81b5b3bf6c 100644
--- a/core/src/main/java/hudson/slaves/ChannelPinger.java
+++ b/core/src/main/java/hudson/slaves/ChannelPinger.java
@@ -34,11 +34,11 @@
import jenkins.slaves.PingFailureAnalyzer;
import java.io.IOException;
+import java.util.Arrays;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.logging.Level;
import java.util.logging.Logger;
-import static java.util.logging.Level.*;
-
/**
* Establish a periodic ping to keep connections between {@link Slave slaves}
* and the main Jenkins node alive. This prevents network proxies from
@@ -49,22 +49,43 @@
@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";
+ private static final String TIMEOUT_SECONDS_PROPERTY = ChannelPinger.class.getName() + ".pingTimeoutSeconds";
+ private static final String INTERVAL_MINUTES_PROPERTY = ChannelPinger.class.getName() + ".pingInterval";
+ private static final String INTERVAL_SECONDS_PROPERTY = ChannelPinger.class.getName() + ".pingIntervalSeconds";
/**
- * Interval for the ping in minutes.
+ * Timeout for the ping in seconds.
*/
- private int pingInterval = 5;
+ private final int pingTimeoutSeconds;
+
+ /**
+ * Interval for the ping in seconds.
+ */
+ private final int pingIntervalSeconds;
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);
+ pingTimeoutSeconds = Integer.getInteger(TIMEOUT_SECONDS_PROPERTY, 4 * 60);
+
+ // A little extra hoop-jumping to migrate from the old system property
+ Integer intervalSeconds = Integer.getInteger(INTERVAL_SECONDS_PROPERTY);
+ Integer intervalMinutes = Integer.getInteger(INTERVAL_MINUTES_PROPERTY);
+ if (intervalMinutes != null) {
+ LOGGER.warning("Property '" + INTERVAL_MINUTES_PROPERTY + "' is deprecated. Please migrate to '" + INTERVAL_SECONDS_PROPERTY + "'");
+
+ if (intervalSeconds != null) {
+ LOGGER.log(Level.WARNING, "Ignoring {0}={1} because {2}={3}",
+ new Object[] { INTERVAL_MINUTES_PROPERTY, intervalMinutes, INTERVAL_SECONDS_PROPERTY, intervalSeconds });
+ } else {
+ intervalSeconds = intervalMinutes * 60;
}
}
+
+ pingIntervalSeconds = intervalSeconds == null ? 5 * 60 : intervalSeconds;
+
+ if (pingIntervalSeconds < pingTimeoutSeconds) {
+ LOGGER.log(Level.WARNING, "Ping interval ({0}) is less than ping timeout ({1})",
+ new Object[] { pingIntervalSeconds, pingTimeoutSeconds });
+ }
}
@Override
@@ -73,13 +94,13 @@ public void preOnline(Computer c, Channel channel, FilePath root, TaskListener l
}
public void install(Channel channel) {
- if (pingInterval < 1) {
+ if (pingTimeoutSeconds < 1 || pingIntervalSeconds < 1) {
LOGGER.fine("Slave ping is disabled");
return;
}
try {
- channel.call(new SetUpRemotePing(pingInterval));
+ channel.call(new SetUpRemotePing(pingTimeoutSeconds, pingIntervalSeconds));
LOGGER.fine("Set up a remote ping for " + channel.getName());
} catch (Exception e) {
LOGGER.severe("Failed to set up a ping for " + channel.getName());
@@ -87,38 +108,66 @@ public void install(Channel channel) {
// set up ping from both directions, so that in case of a router dropping a connection,
// both sides can notice it and take compensation actions.
- setUpPingForChannel(channel, pingInterval);
+ setUpPingForChannel(channel, pingTimeoutSeconds, pingIntervalSeconds);
}
- private static class SetUpRemotePing extends MasterToSlaveCallable {
- private static final long serialVersionUID = -2702219700841759872L;
- private int pingInterval;
- public SetUpRemotePing(int pingInterval) {
- this.pingInterval = pingInterval;
+ static class SetUpRemotePing extends MasterToSlaveCallable {
+ private final int pingTimeoutSeconds;
+ private final int pingIntervalSeconds;
+
+ SetUpRemotePing(int pingTimeoutSeconds, int pingIntervalSeconds) {
+ this.pingTimeoutSeconds = pingTimeoutSeconds;
+ this.pingIntervalSeconds = pingIntervalSeconds;
}
+ @Override
public Void call() throws IOException {
- setUpPingForChannel(Channel.current(), pingInterval);
+ setUpPingForChannel(Channel.current(), pingTimeoutSeconds, pingIntervalSeconds);
return null;
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+
+ SetUpRemotePing that = (SetUpRemotePing) o;
+ return this.pingTimeoutSeconds == that.pingTimeoutSeconds
+ && this.pingIntervalSeconds == that.pingIntervalSeconds;
+ }
+
+ @Override
+ public int hashCode() {
+ // TODO(deadmoose): switch to Objects.hash once Java 7's fully available
+ return Arrays.hashCode(new Object[] { pingTimeoutSeconds, pingIntervalSeconds });
+ }
+
+ @Override
+ public String toString() {
+ return "SetUpRemotePing(" + pingTimeoutSeconds + "," + pingIntervalSeconds + ")";
+ }
}
- private static void setUpPingForChannel(final Channel channel, int interval) {
+ static void setUpPingForChannel(final Channel channel, int timeoutSeconds, int intervalSeconds) {
final AtomicBoolean isInClosed = new AtomicBoolean(false);
- final PingThread t = new PingThread(channel, interval * 60 * 1000) {
+ final PingThread t = new PingThread(channel, timeoutSeconds * 1000L, intervalSeconds * 1000L) {
protected void onDead(Throwable cause) {
try {
for (PingFailureAnalyzer pfa : PingFailureAnalyzer.all()) {
pfa.onPingFailure(channel,cause);
}
if (isInClosed.get()) {
- LOGGER.log(FINE,"Ping failed after the channel "+channel.getName()+" is already partially closed.",cause);
+ LOGGER.log(Level.FINE,"Ping failed after the channel "+channel.getName()+" is already partially closed.",cause);
} else {
- LOGGER.log(INFO,"Ping failed. Terminating the channel "+channel.getName()+".",cause);
+ LOGGER.log(Level.INFO,"Ping failed. Terminating the channel "+channel.getName()+".",cause);
channel.close(cause);
}
} catch (IOException e) {
- LOGGER.log(SEVERE,"Failed to terminate the channel "+channel.getName(),e);
+ LOGGER.log(Level.SEVERE,"Failed to terminate the channel "+channel.getName(),e);
}
}
protected void onDead() {
@@ -136,6 +185,7 @@ public void onClosed(Channel channel, IOException cause) {
});
t.start();
- LOGGER.fine("Ping thread started for " + channel + " with a " + interval + " minute interval");
+ LOGGER.log(Level.FINE, "Ping thread started for {0} with a {1} second interval and a {2} second timeout.",
+ new Object[] { channel, intervalSeconds, timeoutSeconds });
}
}
diff --git a/core/src/test/java/hudson/slaves/ChannelPingerTest.java b/core/src/test/java/hudson/slaves/ChannelPingerTest.java
new file mode 100644
index 000000000000..9018ea55ee81
--- /dev/null
+++ b/core/src/test/java/hudson/slaves/ChannelPingerTest.java
@@ -0,0 +1,117 @@
+package hudson.slaves;
+
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.powermock.api.mockito.PowerMockito.mockStatic;
+import static org.powermock.api.mockito.PowerMockito.verifyNoMoreInteractions;
+import static org.powermock.api.mockito.PowerMockito.verifyStatic;
+
+import com.google.common.testing.EqualsTester;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+import hudson.remoting.Channel;
+
+import java.util.Map;
+import java.util.HashMap;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ ChannelPinger.class })
+public class ChannelPingerTest {
+
+ @Mock private Channel mockChannel;
+
+ private Map savedSystemProperties = new HashMap();
+
+ @Before
+ public void setUp() throws Exception {
+ MockitoAnnotations.initMocks(this);
+ mockStatic(ChannelPinger.class);
+ }
+
+ @Before
+ public void preserveSystemProperties() throws Exception {
+ preserveSystemProperty("hudson.slaves.ChannelPinger.pingInterval");
+ preserveSystemProperty("hudson.slaves.ChannelPinger.pingIntervalSeconds");
+ preserveSystemProperty("hudson.slaves.ChannelPinger.pingTimeoutSeconds");
+ }
+
+ @After
+ public void restoreSystemProperties() throws Exception {
+ for (Map.Entry entry : savedSystemProperties.entrySet()) {
+ if (entry.getValue() != null) {
+ System.setProperty(entry.getKey(), entry.getValue());
+ } else {
+ System.clearProperty(entry.getKey());
+ }
+ }
+ }
+
+ private void preserveSystemProperty(String propertyName) {
+ savedSystemProperties.put(propertyName, System.getProperty(propertyName));
+ System.clearProperty(propertyName);
+ }
+
+ @Test
+ public void testDefaults() throws Exception {
+ ChannelPinger channelPinger = new ChannelPinger();
+ channelPinger.install(mockChannel);
+
+ verify(mockChannel).call(eq(new ChannelPinger.SetUpRemotePing(240, 300)));
+ verifyStatic();
+ ChannelPinger.setUpPingForChannel(mockChannel, 240, 300);
+ }
+
+ @Test
+ public void testFromSystemProperties() throws Exception {
+ System.setProperty("hudson.slaves.ChannelPinger.pingTimeoutSeconds", "42");
+ System.setProperty("hudson.slaves.ChannelPinger.pingIntervalSeconds", "73");
+
+ ChannelPinger channelPinger = new ChannelPinger();
+ channelPinger.install(mockChannel);
+
+ verify(mockChannel).call(new ChannelPinger.SetUpRemotePing(42, 73));
+ verifyStatic();
+ ChannelPinger.setUpPingForChannel(mockChannel, 42, 73);
+ }
+
+ @Test
+ public void testFromOldSystemProperty() throws Exception {
+ System.setProperty("hudson.slaves.ChannelPinger.pingInterval", "7");
+
+ ChannelPinger channelPinger = new ChannelPinger();
+ channelPinger.install(mockChannel);
+
+ verify(mockChannel).call(eq(new ChannelPinger.SetUpRemotePing(240, 420)));
+ verifyStatic();
+ ChannelPinger.setUpPingForChannel(mockChannel, 240, 420);
+ }
+
+ @Test
+ public void testNewSystemPropertyTrumpsOld() throws Exception {
+ System.setProperty("hudson.slaves.ChannelPinger.pingIntervalSeconds", "73");
+ System.setProperty("hudson.slaves.ChannelPinger.pingInterval", "7");
+
+ ChannelPinger channelPinger = new ChannelPinger();
+ channelPinger.install(mockChannel);
+
+ verify(mockChannel).call(eq(new ChannelPinger.SetUpRemotePing(240, 73)));
+ verifyStatic();
+ ChannelPinger.setUpPingForChannel(mockChannel, 240, 73);
+ }
+
+ @Test
+ public void testSetUpRemotePingEquality() {
+ new EqualsTester()
+ .addEqualityGroup(new ChannelPinger.SetUpRemotePing(1, 2), new ChannelPinger.SetUpRemotePing(1, 2))
+ .addEqualityGroup(new ChannelPinger.SetUpRemotePing(2, 3), new ChannelPinger.SetUpRemotePing(2, 3))
+ .testEquals();
+ }
+}
diff --git a/pom.xml b/pom.xml
index 2a83ea812a17..4cfad9a7652b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -96,6 +96,7 @@ THE SOFTWARE.
https://api.github.com
jenkins-jira
+ 11.0.1
1.7.7
2.7.1
1.4.1
@@ -182,7 +183,12 @@ THE SOFTWARE.
com.google.guava
guava
- 11.0.1
+ ${guavaVersion}
+
+
+ com.google.guava
+ guava-testlib
+ ${guavaVersion}