Do not timeout destination calls in lightblue proxy phase

lightblue-platform · Jan 19, 2016 · 3589b10 · 3589b10
1 parent bd0edda
commit 3589b10
Show file tree

Hide file tree

Showing 4 changed files with 160 additions and 9 deletions.
diff --git a/facade/src/main/java/com/redhat/lightblue/migrator/facade/DAOFacadeBase.java b/facade/src/main/java/com/redhat/lightblue/migrator/facade/DAOFacadeBase.java
@@ -175,8 +175,8 @@ public T call() throws Exception {
         }
     }
 
-    private <T> T getWithTimeout(ListenableFuture<T> listenableFuture, String methodName) throws InterruptedException, ExecutionException, TimeoutException {
-        if (timeoutConfiguration.getTimeoutMS(methodName) <= 0) {
+    private <T> T getWithTimeout(ListenableFuture<T> listenableFuture, String methodName, boolean shouldSource) throws InterruptedException, ExecutionException, TimeoutException {
+        if (!shouldSource || timeoutConfiguration.getTimeoutMS(methodName) <= 0) {
             return listenableFuture.get();
         } else {
             return listenableFuture.get(timeoutConfiguration.getTimeoutMS(methodName), TimeUnit.MILLISECONDS);
@@ -233,7 +233,7 @@ public <T> T callDAOReadMethod(final Class<T> returnedType, final String methodN
             // make sure async call to lightblue has completed
             try {
                 log.debug("Calling lightblue {}.{}", implementationName, methodName);
-                lightblueEntity = getWithTimeout(listenableFuture, methodName);
+                lightblueEntity = getWithTimeout(listenableFuture, methodName, LightblueMigration.shouldReadSourceEntity());
             } catch (TimeoutException te) {
                 if (LightblueMigration.shouldReadSourceEntity()) {
                     log.warn("Lightblue call "+implementationName+"."+methodCallToString(methodName, values)+" is taking too long (longer than "+timeoutConfiguration.getTimeoutMS(methodName)+"s). Returning data from legacy.");
@@ -325,7 +325,7 @@ public <T> T callDAOUpdateMethod(final Class<T> returnedType, final String metho
             // make sure asnyc call to lightblue has completed
             log.debug("Calling lightblue {}.{}", implementationName, methodName);
             try {
-                lightblueEntity = getWithTimeout(listenableFuture, methodName);
+                lightblueEntity = getWithTimeout(listenableFuture, methodName, LightblueMigration.shouldWriteSourceEntity());
             } catch (TimeoutException te) {
                 if (LightblueMigration.shouldReadSourceEntity()) {
                     log.warn("Lightblue call "+implementationName+"."+methodCallToString(methodName, values)+" is taking too long (longer than "+timeoutConfiguration.getTimeoutMS(methodName)+"s). Returning data from legacy.");
@@ -426,7 +426,7 @@ public <T> T callDAOCreateSingleMethod(final EntityIdExtractor<T> entityIdExtrac
             ListenableFuture<T> listenableFuture = callLightblueDAO(passIds, method, values);
 
             try {
-                lightblueEntity = getWithTimeout(listenableFuture, methodName);
+                lightblueEntity = getWithTimeout(listenableFuture, methodName, LightblueMigration.shouldWriteSourceEntity());
             } catch (ExecutionException ee) {
                 if (LightblueMigration.shouldReadSourceEntity()) {
                     EntityIdStoreException se = extractEntityIdStoreExceptionIfExists(ee);

diff --git a/facade/src/main/java/com/redhat/lightblue/migrator/facade/ServiceFacade.java b/facade/src/main/java/com/redhat/lightblue/migrator/facade/ServiceFacade.java
@@ -190,8 +190,20 @@ public T call() throws Exception {
         }
     }
 
-    private <T> T getWithTimeout(ListenableFuture<T> listenableFuture, String methodName) throws InterruptedException, ExecutionException, TimeoutException {
-        if (timeoutConfiguration.getTimeoutMS(methodName) <= 0) {
+    /**
+     * Call destination (lightblue) using a timeout in dual read/write phases. Do not use facade
+     * timeout during lightblue proxy phase.
+     *
+     * @param listenableFuture
+     * @param methodName method name is used to read method specific timeout configuration
+     * @param shouldSource true if source operation is supposed to be performed for this call (i.e. this is a dual phase)
+     * @return
+     * @throws InterruptedException
+     * @throws ExecutionException
+     * @throws TimeoutException
+     */
+    private <T> T getWithTimeout(ListenableFuture<T> listenableFuture, String methodName, boolean shouldSource) throws InterruptedException, ExecutionException, TimeoutException {
+        if (!shouldSource || timeoutConfiguration.getTimeoutMS(methodName) <= 0) {
             return listenableFuture.get();
         } else {
             return listenableFuture.get(timeoutConfiguration.getTimeoutMS(methodName), TimeUnit.MILLISECONDS);
@@ -295,12 +307,12 @@ public <T> T callSvcMethod(final FacadeOperation facadeOperation, final boolean
             try {
                 if (callInParallel) {
                     // lightblue was called before source/legacy, now just getting results
-                    lightblueEntity = getWithTimeout(listenableFuture, methodName);
+                    lightblueEntity = getWithTimeout(listenableFuture, methodName, shouldSource(facadeOperation));
                 } else {
                     // call lightblue after source/legacy finished
                     Method method = lightblueSvc.getClass().getMethod(methodName, types);
                     listenableFuture = callLightblueSvc(true, method, values);
-                    lightblueEntity = getWithTimeout(listenableFuture, methodName);
+                    lightblueEntity = getWithTimeout(listenableFuture, methodName, shouldSource(facadeOperation));
                 }
             } catch (TimeoutException te) {
                 if (shouldSource(facadeOperation)) {

diff --git a/facade/src/test/java/com/redhat/lightblue/migrator/facade/DAOFacadeTest.java b/facade/src/test/java/com/redhat/lightblue/migrator/facade/DAOFacadeTest.java
@@ -660,6 +660,89 @@ public Country answer(InvocationOnMock invocation) throws Throwable {
         Assert.assertEquals(pl, returnedCountry);
     }
 
+    @Test
+    public void lightblueTakesLongToRespondOnCreate_ProxyPhase() throws CountryException {
+        Properties p = new Properties();
+        TimeoutConfiguration t = new TimeoutConfiguration(100, CountryDAO.class.getSimpleName(), p);
+        daoFacadeExample.setTimeoutConfiguration(t);
+
+        LightblueMigrationPhase.lightblueProxyPhase(togglzRule);
+
+        final Country pl = new Country(101l, "PL");
+
+        Mockito.when(lightblueDAO.createCountry(Mockito.any(Country.class))).thenAnswer(new Answer<Country>() {
+
+            @Override
+            public Country answer(InvocationOnMock invocation) throws Throwable {
+                Thread.sleep(200);
+                return pl;
+            }
+        });
+
+        Country returnedCountry = facade.createCountry(pl);
+
+        Mockito.verify(lightblueDAO).createCountry(pl);
+        Mockito.verifyZeroInteractions(legacyDAO);
+        Mockito.verifyZeroInteractions(consistencyChecker);
+
+        Assert.assertEquals(pl, returnedCountry);
+    }
+
+    @Test
+    public void lightblueTakesLongToRespondOnUpdate_ProxyPhase() throws CountryException {
+        Properties p = new Properties();
+        TimeoutConfiguration t = new TimeoutConfiguration(100, CountryDAO.class.getSimpleName(), p);
+        daoFacadeExample.setTimeoutConfiguration(t);
+
+        LightblueMigrationPhase.lightblueProxyPhase(togglzRule);
+
+        final Country pl = new Country(101l, "PL");
+
+        Mockito.when(lightblueDAO.updateCountry(Mockito.any(Country.class))).thenAnswer(new Answer<Country>() {
+
+            @Override
+            public Country answer(InvocationOnMock invocation) throws Throwable {
+                Thread.sleep(200);
+                return pl;
+            }
+        });
+
+        Country returnedCountry = facade.updateCountry(pl);
+
+        Mockito.verify(lightblueDAO).updateCountry(pl);
+        Mockito.verifyZeroInteractions(legacyDAO);
+        Mockito.verifyZeroInteractions(consistencyChecker);
+
+        Assert.assertEquals(pl, returnedCountry);
+    }
+
+    @Test
+    public void lightblueTakesLongToRespondOnRead_ProxyPhase() throws CountryException {
+        Properties p = new Properties();
+        TimeoutConfiguration t = new TimeoutConfiguration(100, CountryDAO.class.getSimpleName(), p);
+        daoFacadeExample.setTimeoutConfiguration(t);
+
+        LightblueMigrationPhase.lightblueProxyPhase(togglzRule);
+
+        final Country pl = new Country(101l, "PL");
+
+        Mockito.when(lightblueDAO.getCountry(Mockito.any(String.class))).thenAnswer(new Answer<Country>() {
+
+            @Override
+            public Country answer(InvocationOnMock invocation) throws Throwable {
+                Thread.sleep(200);
+                return pl;
+            }
+        });
+
+        Country returnedCountry = facade.getCountry("PL");
+
+        Mockito.verify(lightblueDAO).getCountry("PL");
+        Mockito.verifyZeroInteractions(legacyDAO);
+        Mockito.verifyZeroInteractions(consistencyChecker);
+
+        Assert.assertEquals(pl, returnedCountry);
+    }
 
     /* legacy failure tests */
 

diff --git a/facade/src/test/java/com/redhat/lightblue/migrator/facade/ServiceFacadeTest.java b/facade/src/test/java/com/redhat/lightblue/migrator/facade/ServiceFacadeTest.java
@@ -693,6 +693,62 @@ public Country answer(InvocationOnMock invocation) throws Throwable {
         Assert.assertEquals(pl, returnedCountry);
     }
 
+    @Test
+    public void lightblueTakesLongToRespondOnCreate_ProxyPhase() throws CountryException {
+        Properties p = new Properties();
+        TimeoutConfiguration t = new TimeoutConfiguration(100, CountryDAO.class.getSimpleName(), p);
+        daoFacade.setTimeoutConfiguration(t);
+
+        LightblueMigrationPhase.lightblueProxyPhase(togglzRule);
+
+        final Country pl = new Country(101l, "PL");
+
+        Mockito.when(lightblueDAO.createCountry(Mockito.any(Country.class))).thenAnswer(new Answer<Country>() {
+
+            @Override
+            public Country answer(InvocationOnMock invocation) throws Throwable {
+                Thread.sleep(200);
+                return pl;
+            }
+        });
+
+        Country returnedCountry = countryDAOProxy.createCountry(pl);
+
+        Mockito.verify(lightblueDAO).createCountry(pl);
+        Mockito.verifyZeroInteractions(legacyDAO);
+        Mockito.verifyZeroInteractions(consistencyChecker);
+
+        Assert.assertEquals(pl, returnedCountry);
+    }
+
+    @Test
+    public void lightblueTakesLongToRespondOnRead_ProxyPhase() throws CountryException {
+        Properties p = new Properties();
+        TimeoutConfiguration t = new TimeoutConfiguration(100, CountryDAO.class.getSimpleName(), p);
+        daoFacade.setTimeoutConfiguration(t);
+
+        LightblueMigrationPhase.lightblueProxyPhase(togglzRule);
+
+        final Country pl = new Country(101l, "PL");
+
+        Mockito.when(lightblueDAO.getCountry(Mockito.any(String.class))).thenAnswer(new Answer<Country>() {
+
+            @Override
+            public Country answer(InvocationOnMock invocation) throws Throwable {
+                Thread.sleep(200);
+                return pl;
+            }
+        });
+
+        Country returnedCountry = countryDAOProxy.getCountry("PL");
+
+        Mockito.verify(lightblueDAO).getCountry("PL");
+        Mockito.verifyZeroInteractions(legacyDAO);
+        Mockito.verifyZeroInteractions(consistencyChecker);
+
+        Assert.assertEquals(pl, returnedCountry);
+    }
+
     /* legacy failure tests */
 
     @Test