-
-
Notifications
You must be signed in to change notification settings - Fork 273
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.
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
[JENKINS-72163] Retry on initial connection failure occurs in one entrypoint but not the other #675
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -694,6 +694,7 @@ | |
} | ||
events.onDisconnect(); | ||
while (true) { | ||
// TODO refactor various sleep statements into a common method | ||
TimeUnit.SECONDS.sleep(10); | ||
// Unlike JnlpAgentEndpointResolver, we do not use $jenkins/tcpSlaveAgentListener/, as that will be a 404 if the TCP port is disabled. | ||
URL ping = new URL(hudsonUrl, "login"); | ||
|
@@ -758,11 +759,18 @@ | |
final JnlpAgentEndpoint endpoint; | ||
try { | ||
endpoint = resolver.resolve(); | ||
} catch (Exception e) { | ||
if (Boolean.getBoolean(Engine.class.getName() + ".nonFatalJnlpAgentEndpointResolutionExceptions")) { | ||
events.status("Could not resolve JNLP agent endpoint", e); | ||
} catch (IOException e) { | ||
if (!noReconnect) { | ||
events.status("Could not locate server among " + candidateUrls + "; waiting 10 seconds before retry", e); | ||
// TODO refactor various sleep statements into a common method | ||
TimeUnit.SECONDS.sleep(10); | ||
continue; | ||
} else { | ||
events.error(e); | ||
if (Boolean.getBoolean(Engine.class.getName() + ".nonFatalJnlpAgentEndpointResolutionExceptions")) { | ||
events.status("Could not resolve JNLP agent endpoint", e); | ||
Comment on lines
+769
to
+770
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to readers excluding the author (not a review): this property derives from #449, where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not sure why scare quotes were used here, but yes a call to |
||
} else { | ||
events.error(e); | ||
} | ||
} | ||
return; | ||
} | ||
|
@@ -891,6 +899,7 @@ | |
|
||
private void onConnectionRejected(String greeting) throws InterruptedException { | ||
events.status("reconnect rejected, sleeping 10s: ", new Exception("The server rejected the connection: " + greeting)); | ||
// TODO refactor various sleep statements into a common method | ||
TimeUnit.SECONDS.sleep(10); | ||
} | ||
|
||
|
@@ -913,6 +922,7 @@ | |
if(retry++>10) { | ||
throw e; | ||
} | ||
// TODO refactor various sleep statements into a common method | ||
TimeUnit.SECONDS.sleep(10); | ||
events.status(msg+" (retrying:"+retry+")",e); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor cleanup while I was here. The original code was catching any exception type, including
InterruptedException
(which it made no attempt to handle). Following generic advice such as that given on this page, prefer specific exceptions in catch blocks.