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

[JENKINS-35190] Do not invoke PingFailureAnalyzer for agent=>master ping failures #2377

Merged
merged 1 commit into from
May 29, 2016
Merged

[JENKINS-35190] Do not invoke PingFailureAnalyzer for agent=>master ping failures #2377

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions core/src/main/java/hudson/slaves/ChannelPinger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ 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, pingInterval, true);
}

private static class SetUpRemotePing extends MasterToSlaveCallable<Void, IOException> {
Expand All @@ -99,18 +99,20 @@ public SetUpRemotePing(int pingInterval) {
}

public Void call() throws IOException {
setUpPingForChannel(Channel.current(), pingInterval);
setUpPingForChannel(Channel.current(), pingInterval, false);
return null;
}
}

private static void setUpPingForChannel(final Channel channel, int interval) {
private static void setUpPingForChannel(final Channel channel, int interval, final boolean analysis) {
LOGGER.log(FINE, "setting up ping on {0} at interval {1}m", new Object[] {channel.getName(), interval});
final AtomicBoolean isInClosed = new AtomicBoolean(false);
final PingThread t = new PingThread(channel, interval * 60 * 1000) {
@Override
protected void onDead(Throwable cause) {
try {
for (PingFailureAnalyzer pfa : PingFailureAnalyzer.all()) {
pfa.onPingFailure(channel,cause);
if (analysis) {
analyze(cause);
}
if (isInClosed.get()) {
LOGGER.log(FINE,"Ping failed after the channel "+channel.getName()+" is already partially closed.",cause);
Expand All @@ -122,6 +124,14 @@ protected void onDead(Throwable cause) {
LOGGER.log(SEVERE,"Failed to terminate the channel "+channel.getName(),e);
}
}
/** Keep in a separate method so we do not even try to do class loading on {@link PingFailureAnalyzer} from an agent JVM. */
private void analyze(Throwable cause) throws IOException {
for (PingFailureAnalyzer pfa : PingFailureAnalyzer.all()) {
pfa.onPingFailure(channel,cause);
Copy link
Member

Choose a reason for hiding this comment

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

If you modify this code, please add handling of Runtime Exceptions

Copy link
Member Author

Choose a reason for hiding this comment

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

It would be nice, though (a) there are no implementations to test, as noted in the description, (b) I spent some time testing the patch as written and would have to spend more time retesting with any other modifications.

}
}
@Deprecated
@Override
protected void onDead() {
onDead(null);
}
Expand Down