diff --git a/server/src/main/java/org/apache/druid/discovery/BrokerClient.java b/server/src/main/java/org/apache/druid/discovery/BrokerClient.java index a64c8d670e4..bdee9b8dfe4 100644 --- a/server/src/main/java/org/apache/druid/discovery/BrokerClient.java +++ b/server/src/main/java/org/apache/druid/discovery/BrokerClient.java @@ -33,7 +33,6 @@ import org.jboss.netty.channel.ChannelException; import org.jboss.netty.handler.codec.http.HttpMethod; import org.jboss.netty.handler.codec.http.HttpResponseStatus; -import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; @@ -88,10 +87,6 @@ public class BrokerClient throw DruidException.forPersona(DruidException.Persona.OPERATOR) .ofCategory(DruidException.Category.RUNTIME_FAILURE) .build("Request to broker failed due to failed response status: [%s]", responseStatus); - } else if (responseStatus.getCode() != HttpServletResponse.SC_OK) { - throw DruidException.forPersona(DruidException.Persona.OPERATOR) - .ofCategory(DruidException.Category.RUNTIME_FAILURE) - .build("Request to broker failed due to failed response code: [%s]", responseStatus.getCode()); } return fullResponseHolder.getContent(); }, @@ -99,6 +94,9 @@ public class BrokerClient if (throwable instanceof ExecutionException) { return throwable.getCause() instanceof IOException || throwable.getCause() instanceof ChannelException; } + if (throwable instanceof DruidException) { + return ((DruidException) throwable).getCategory() == DruidException.Category.RUNTIME_FAILURE; + } return throwable instanceof IOE; }, MAX_RETRIES diff --git a/server/src/test/java/org/apache/druid/discovery/BrokerClientTest.java b/server/src/test/java/org/apache/druid/discovery/BrokerClientTest.java index 333882a4305..de03877a9b0 100644 --- a/server/src/test/java/org/apache/druid/discovery/BrokerClientTest.java +++ b/server/src/test/java/org/apache/druid/discovery/BrokerClientTest.java @@ -101,7 +101,7 @@ public class BrokerClientTest extends BaseJettyTest } @Test - public void testError() throws Exception + public void testRetryableError() throws Exception { DruidNodeDiscovery druidNodeDiscovery = EasyMock.createMock(DruidNodeDiscovery.class); EasyMock.expect(druidNodeDiscovery.getAllNodes()).andReturn(ImmutableList.of(discoveryDruidNode)).anyTimes(); @@ -121,6 +121,26 @@ public class BrokerClientTest extends BaseJettyTest Assert.assertEquals("hello", brokerClient.sendQuery(request)); } + @Test + public void testNonRetryableError() throws Exception + { + DruidNodeDiscovery druidNodeDiscovery = EasyMock.createMock(DruidNodeDiscovery.class); + EasyMock.expect(druidNodeDiscovery.getAllNodes()).andReturn(ImmutableList.of(discoveryDruidNode)).anyTimes(); + + DruidNodeDiscoveryProvider druidNodeDiscoveryProvider = EasyMock.createMock(DruidNodeDiscoveryProvider.class); + EasyMock.expect(druidNodeDiscoveryProvider.getForNodeRole(NodeRole.BROKER)).andReturn(druidNodeDiscovery); + + EasyMock.replay(druidNodeDiscovery, druidNodeDiscoveryProvider); + + BrokerClient brokerClient = new BrokerClient( + httpClient, + druidNodeDiscoveryProvider + ); + + Request request = brokerClient.makeRequest(HttpMethod.POST, "/simple/error"); + Assert.assertEquals("", brokerClient.sendQuery(request)); + } + @Path("/simple") public static class SimpleResource { @@ -150,5 +170,13 @@ public class BrokerClientTest extends BaseJettyTest return Response.status(504).build(); } } + + @POST + @Path("/error") + @Produces(MediaType.APPLICATION_JSON) + public Response error() + { + return Response.status(404).build(); + } } }