[knox] branch master updated: KNOX-2478 Logging fix (#390)

[mo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

more pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git


The following commit(s) were added to refs/heads/master by this push:
     new 9aa8772  KNOX-2478 Logging fix (#390)
9aa8772 is described below

commit 9aa87729c0f28293432404d0ec0b1746c7ee099c
Author: Sandeep Moré <mo...@gmail.com>
AuthorDate: Fri Dec 4 06:19:53 2020 -0500

    KNOX-2478 Logging fix (#390)
---
 .../gateway/ha/dispatch/DefaultHaDispatch.java     |  4 +-
 .../gateway/ha/dispatch/DefaultHaDispatchTest.java | 67 ++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatch.java b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatch.java
index 0e479b1..5649ad3 100644
--- a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatch.java
+++ b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatch.java
@@ -209,14 +209,13 @@ public class DefaultHaDispatch extends DefaultDispatch {
       outboundResponse.sendError(HttpServletResponse.SC_BAD_GATEWAY, "Service connection error, HA failover disabled");
       return;
     }
-    LOG.failingOverRequest(outboundRequest.getURI().toString());
+    haProvider.markFailedURL(getServiceRole(), outboundRequest.getURI().toString());
     AtomicInteger counter = (AtomicInteger) inboundRequest.getAttribute(FAILOVER_COUNTER_ATTRIBUTE);
     if ( counter == null ) {
       counter = new AtomicInteger(0);
     }
     inboundRequest.setAttribute(FAILOVER_COUNTER_ATTRIBUTE, counter);
     if ( counter.incrementAndGet() <= maxFailoverAttempts ) {
-      haProvider.markFailedURL(getServiceRole(), outboundRequest.getURI().toString());
       setupUrlHashLookup(); // refresh the url hash after failing a url
       //null out target url so that rewriters run again
       inboundRequest.setAttribute(AbstractGatewayFilter.TARGET_REQUEST_URL_ATTRIBUTE_NAME, null);
@@ -232,6 +231,7 @@ public class DefaultHaDispatch extends DefaultDispatch {
           Thread.currentThread().interrupt();
         }
       }
+      LOG.failingOverRequest(outboundRequest.getURI().toString());
       executeRequest(outboundRequest, inboundRequest, outboundResponse);
     } else {
       LOG.maxFailoverAttemptsReached(maxFailoverAttempts, getServiceRole());
diff --git a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatchTest.java b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatchTest.java
index dc9bcf2..7e51904 100644
--- a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatchTest.java
+++ b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatchTest.java
@@ -287,4 +287,71 @@ public class DefaultHaDispatchTest {
     dispatch.setServiceRole(serviceName);
     dispatch.init();
   }
+
+  /**
+   * test a case where a service might want to mark a host failed without retrying
+   * so the next request that comes goes to the other HA host.
+   * This can be achieved by using maxFailoverAttempsValue=0
+   * @throws Exception
+   */
+  @Test
+  public void testMarkedFailedWithoutRetry() throws Exception {
+    String serviceName = "OOZIE";
+    HaDescriptor descriptor = HaDescriptorFactory.createDescriptor();
+    descriptor.addServiceConfig(HaDescriptorFactory.createServiceConfig(serviceName, "true", "0", "1000", null, null, null, null, null, null));
+    HaProvider provider = new DefaultHaProvider(descriptor);
+    URI uri1 = new URI( "http://unreachable-host.invalid" );
+    URI uri2 = new URI( "http://reachable-host.invalid" );
+    ArrayList<String> urlList = new ArrayList<>();
+    urlList.add(uri1.toString());
+    urlList.add(uri2.toString());
+    provider.addHaService(serviceName, urlList);
+    FilterConfig filterConfig = EasyMock.createNiceMock(FilterConfig.class);
+    ServletContext servletContext = EasyMock.createNiceMock(ServletContext.class);
+
+    EasyMock.expect(filterConfig.getServletContext()).andReturn(servletContext).anyTimes();
+    EasyMock.expect(servletContext.getAttribute(HaServletContextListener.PROVIDER_ATTRIBUTE_NAME)).andReturn(provider).anyTimes();
+
+    BasicHttpParams params = new BasicHttpParams();
+
+    HttpUriRequest outboundRequest = EasyMock.createNiceMock(HttpRequestBase.class);
+    EasyMock.expect(outboundRequest.getMethod()).andReturn( "GET" ).anyTimes();
+    EasyMock.expect(outboundRequest.getURI()).andReturn( uri1  ).anyTimes();
+    EasyMock.expect(outboundRequest.getParams()).andReturn( params ).anyTimes();
+
+    HttpServletRequest inboundRequest = EasyMock.createNiceMock(HttpServletRequest.class);
+    EasyMock.expect(inboundRequest.getRequestURL()).andReturn( new StringBuffer(uri2.toString()) ).once();
+    EasyMock.expect(inboundRequest.getAttribute("dispatch.ha.failover.counter")).andReturn(new AtomicInteger(0)).once();
+    EasyMock.expect(inboundRequest.getAttribute("dispatch.ha.failover.counter")).andReturn(new AtomicInteger(1)).once();
+
+    HttpServletResponse outboundResponse = EasyMock.createNiceMock(HttpServletResponse.class);
+    EasyMock.expect(outboundResponse.getOutputStream()).andAnswer( new IAnswer<SynchronousServletOutputStreamAdapter>() {
+      @Override
+      public SynchronousServletOutputStreamAdapter answer() {
+        return new SynchronousServletOutputStreamAdapter() {
+          @Override
+          public void write( int b ) throws IOException {
+            throw new IOException( "unreachable-host.invalid" );
+          }
+        };
+      }
+    }).once();
+    EasyMock.replay(filterConfig, servletContext, outboundRequest, inboundRequest, outboundResponse);
+    Assert.assertEquals(uri1.toString(), provider.getActiveURL(serviceName));
+    DefaultHaDispatch dispatch = new DefaultHaDispatch();
+    HttpClientBuilder builder = HttpClientBuilder.create();
+    CloseableHttpClient client = builder.build();
+    dispatch.setHttpClient(client);
+    dispatch.setHaProvider(provider);
+    dispatch.setServiceRole(serviceName);
+    dispatch.init();
+    try {
+      dispatch.executeRequest(outboundRequest, inboundRequest, outboundResponse);
+    } catch (IOException e) {
+      //this is expected after the failover limit is reached
+    }
+    /* make sure active url list got updated */
+    Assert.assertEquals(uri2.toString(), provider.getActiveURL(serviceName));
+  }
+
 }