diff options
author | Harald Musum <musum@verizonmedia.com> | 2019-10-21 09:28:59 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-10-21 09:28:59 +0200 |
commit | 1f0513a8f0f0d474a6cade2cd483af18ff55e0b9 (patch) | |
tree | 42c21ce482c947ce6b890b2af54402d18f1897bc | |
parent | f76693ebe9d78c06d44d7a868ea0ceafaffc8474 (diff) | |
parent | 5112c053d90aa31bba6736a1831ca3a1e6b47285 (diff) |
Merge pull request #11022 from vespa-engine/hakonhall/avoid-retries-against-config-server-if-valid-http-response-code-is-received
Avoid retries against config server if valid HTTP response code is received
3 files changed, 6 insertions, 7 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java index 7b2a1f9764b..3f6909b8ea8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java @@ -119,7 +119,7 @@ public class ConfigServerApiImpl implements ConfigServerApi { // Failure to communicate with a config server is not abnormal during upgrades if (ConnectionException.isKnownConnectionException(e)) { - logger.info("Failed to connect to " + configServer + " (upgrading?), will try next: " + e.getMessage()); + logger.info("Failed to connect to " + configServer + ", will try next: " + e.getMessage()); } else { logger.warning("Failed to communicate with " + configServer + ", will try next: " + e.getMessage()); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java index a9493d4606e..c618fd480ba 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java @@ -58,8 +58,9 @@ public class HttpException extends ConvergenceException { throw new HttpException(status, message, false); } - // Other errors like server-side errors are assumed to be retryable. - throw new HttpException(status, message, true); + // Other errors like server-side errors are assumed to be NOT retryable, + // in case retries would put additional load on a bogged down server. + throw new HttpException(status, message, false); } public static class NotFoundException extends HttpException { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java index 2cff62afad6..1ed3e5729e5 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java @@ -119,7 +119,7 @@ public class ConfigServerApiImplTest { } @Test - public void testRetriesOnBadHttpResponseCode() { + public void testNoRetriesOnBadHttpResponseCode() { // Client is throwing exception, should be retries. mockReturnCode = 503; try { @@ -129,9 +129,7 @@ public class ConfigServerApiImplTest { // ignore } - String[] log = mockLog.toString().split(" "); - assertThat(log, arrayContainingInAnyOrder( - "GET http://host1:666/path", "GET http://host2:666/path")); + assertLogStringContainsGETForAHost(); } @Test |