Added retries to service level methods that make persistence calls. (#…

…413)

* Added retries to service level methods that make persistence calls.

* Review changes.

* Review changes.

* Review changes.

* Review changes.

* Review changes.

codequality/checkstyle/checkstyle.xml

@@ -33,6 +33,8 @@
          consistency regarding their keys. -->
     <module name="Translation"/>
 
+    <module name="SuppressWarningsFilter" />
+
     <module name="TreeWalker">
         <!-- Checks the style of array type definitions. -->
         <module name="ArrayTypeStyle"/>
@@ -282,6 +284,8 @@
 
         <!-- Checks that a token is surrounded by whitespace. -->
         <module name="WhitespaceAround"/>
+
+        <module name="SuppressWarningsHolder" />
     </module>
 
     <!-- Enable suppression comments -->

genie-core/src/main/java/com/netflix/genie/core/properties/DataServiceRetryProperties.java

@@ -0,0 +1,46 @@
+/*
+ *
+ *  Copyright 2016 Netflix, Inc.
+ *
+ *     Licensed under the Apache License, Version 2.0 (the "License");
+ *     you may not use this file except in compliance with the License.
+ *     You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *     Unless required by applicable law or agreed to in writing, software
+ *     distributed under the License is distributed on an "AS IS" BASIS,
+ *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *     See the License for the specific language governing permissions and
+ *     limitations under the License.
+ *
+ */
+package com.netflix.genie.core.properties;
+
+import lombok.Getter;
+import lombok.Setter;
+
+/**
+ * All properties related to data service retry template in Genie.
+ *
+ * @author amajumdar
+ * @since 3.0.0
+ */
+@Getter
+@Setter
+public class DataServiceRetryProperties {
+    /**
+     * Default to 5 retries.
+     */
+    private int noOfRetries = 5;
+
+    /**
+     * Default to 100 ms.
+     */
+    private long initialInterval = 100L;
+
+    /**
+     * Defaults to 30000 ms.
+     */
+    private long maxInterval = 30000L;
+}

genie-war/src/main/java/com/netflix/genie/GenieWar.java

@@ -23,15 +23,13 @@
 import org.springframework.boot.autoconfigure.session.SessionAutoConfiguration;
 import org.springframework.boot.builder.SpringApplicationBuilder;
 import org.springframework.boot.context.web.SpringBootServletInitializer;
-import org.springframework.retry.annotation.EnableRetry;
 
 /**
  * A class that serves to set up Spring Boot within a servlet container rather than an embedded one.
  *
  * @author tgianos
  * @since 3.0.0
  */
-@EnableRetry
 @SpringBootApplication(
     exclude = {
         SessionAutoConfiguration.class,

genie-web/src/main/java/com/netflix/genie/GenieWeb.java

@@ -22,7 +22,7 @@
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.boot.autoconfigure.data.redis.RedisAutoConfiguration;
 import org.springframework.boot.autoconfigure.session.SessionAutoConfiguration;
-import org.springframework.retry.annotation.EnableRetry;
+import org.springframework.context.annotation.EnableAspectJAutoProxy;
 
 import java.util.Map;
 
@@ -32,8 +32,8 @@
  * @author tgianos
  * @since 3.0.0
  */
-@EnableRetry
 @SpringBootApplication(exclude = {SessionAutoConfiguration.class, RedisAutoConfiguration.class})
+@EnableAspectJAutoProxy
 public class GenieWeb {
 
     protected static final String SPRING_CONFIG_LOCATION = "spring.config.location";

genie-web/src/main/java/com/netflix/genie/web/aspect/DataServiceRetryAspect.java

@@ -0,0 +1,114 @@
+/*
+ *
+ *  Copyright 2016 Netflix, Inc.
+ *
+ *     Licensed under the Apache License, Version 2.0 (the "License");
+ *     you may not use this file except in compliance with the License.
+ *     You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *     Unless required by applicable law or agreed to in writing, software
+ *     distributed under the License is distributed on an "AS IS" BASIS,
+ *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *     See the License for the specific language governing permissions and
+ *     limitations under the License.
+ *
+ */
+package com.netflix.genie.web.aspect;
+
+import com.google.common.collect.ImmutableMap;
+import com.netflix.genie.common.exceptions.GenieException;
+import com.netflix.genie.common.exceptions.GenieServerException;
+import com.netflix.genie.core.properties.DataServiceRetryProperties;
+import lombok.extern.slf4j.Slf4j;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.Ordered;
+import org.springframework.dao.CannotAcquireLockException;
+import org.springframework.dao.ConcurrencyFailureException;
+import org.springframework.dao.DeadlockLoserDataAccessException;
+import org.springframework.dao.OptimisticLockingFailureException;
+import org.springframework.dao.PessimisticLockingFailureException;
+import org.springframework.dao.QueryTimeoutException;
+import org.springframework.dao.TransientDataAccessResourceException;
+import org.springframework.jdbc.CannotGetJdbcConnectionException;
+import org.springframework.orm.jpa.JpaSystemException;
+import org.springframework.retry.RetryListener;
+import org.springframework.retry.backoff.ExponentialBackOffPolicy;
+import org.springframework.retry.policy.SimpleRetryPolicy;
+import org.springframework.retry.support.RetryTemplate;
+import org.springframework.stereotype.Component;
+
+import javax.validation.ConstraintViolationException;
+
+/**
+ * Aspect implementation of retrying the data service methods on certain failures.
+ * @author amajumdar
+ * @since 3.0.0
+ */
+@Aspect
+@Component
+@Slf4j
+public class DataServiceRetryAspect implements Ordered {
+    private final RetryTemplate retryTemplate;
+
+    /**
+     * Constructor.
+     * @param dataServiceRetryProperties retry properties
+     */
+    @Autowired
+    public DataServiceRetryAspect(final DataServiceRetryProperties dataServiceRetryProperties) {
+        retryTemplate = new RetryTemplate();
+        retryTemplate.setRetryPolicy(new SimpleRetryPolicy(dataServiceRetryProperties.getNoOfRetries(),
+            new ImmutableMap.Builder<Class<? extends Throwable>, Boolean>()
+                .put(CannotGetJdbcConnectionException.class, true)
+                .put(CannotAcquireLockException.class, true)
+                .put(DeadlockLoserDataAccessException.class, true)
+                .put(OptimisticLockingFailureException.class, true)
+                .put(PessimisticLockingFailureException.class, true)
+                .put(ConcurrencyFailureException.class, true)
+                // Will this work for cases where the write queries timeout on the client?
+                .put(QueryTimeoutException.class, true)
+                .put(TransientDataAccessResourceException.class, true)
+                .put(JpaSystemException.class, true)
+                .build()));
+        final ExponentialBackOffPolicy backOffPolicy = new ExponentialBackOffPolicy();
+        backOffPolicy.setInitialInterval(dataServiceRetryProperties.getInitialInterval());
+        backOffPolicy.setMaxInterval(dataServiceRetryProperties.getMaxInterval());
+        retryTemplate.setBackOffPolicy(backOffPolicy);
+    }
+
+    /**
+     * Sets the retry listeners for the retry template in use.
+     * @param retryListeners retry listeners
+     */
+    public void setRetryListeners(final RetryListener[] retryListeners) {
+        retryTemplate.setListeners(retryListeners);
+    }
+
+    /**
+     * Aspect implementation method of retrying the data service method on certain failures.
+     * @param pjp join point
+     * @return return the data method response
+     * @throws GenieException any exception thrown by the data service method
+     */
+    @Around("com.netflix.genie.web.aspect.SystemArchitecture.dataOperation()")
+    public Object profile(final ProceedingJoinPoint pjp) throws GenieException {
+        try {
+            return retryTemplate.execute(context -> pjp.proceed());
+        } catch (GenieException | ConstraintViolationException e) {
+            throw e;
+        } catch (Throwable e) {
+            throw new GenieServerException(e);
+        }
+    }
+
+    @Override
+    public int getOrder() {
+        // Currently setting this to 0 since we want the retry to happen before the transaction interceptor.
+        return 0;
+    }
+}

genie-web/src/main/java/com/netflix/genie/web/aspect/SystemArchitecture.java

@@ -0,0 +1,79 @@
+/*
+ *
+ *  Copyright 2016 Netflix, Inc.
+ *
+ *     Licensed under the Apache License, Version 2.0 (the "License");
+ *     you may not use this file except in compliance with the License.
+ *     You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *     Unless required by applicable law or agreed to in writing, software
+ *     distributed under the License is distributed on an "AS IS" BASIS,
+ *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *     See the License for the specific language governing permissions and
+ *     limitations under the License.
+ *
+ */
+package com.netflix.genie.web.aspect;
+
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Pointcut;
+import org.springframework.stereotype.Component;
+
+/**
+ * Application pointcut expressions.
+ * @author amajumdar
+ * @since 3.0.0
+ */
+@Aspect
+@Component
+public class SystemArchitecture {
+    /**
+     * A join point is in the resource layer if the method is defined
+     * in a type in the com.netflix.genie.web.controllers package or any sub-package
+     * under that.
+     */
+    @Pointcut("within(com.netflix.genie.web.controllers..*)")
+    public void inResourceLayer() { }
+
+    /**
+     * A join point is in the service layer if the method is defined
+     * in a type in the com.netflix.genie.core.services package or any sub-package
+     * under that.
+     */
+    @Pointcut("within(com.netflix.genie.core.services..*)")
+    public void inServiceLayer() { }
+
+    /**
+     * A join point is in the data service layer if the method is defined
+     * in a type in the com.netflix.genie.core.jpa.services package or any sub-package
+     * under that.
+     */
+    @Pointcut("within(com.netflix.genie.core.jpa.services..*)")
+    public void inDataLayer() { }
+
+    /**
+     * A resource service is the execution of any method defined on a controller.
+     * This definition assumes that interfaces are placed in the
+     * "resources" package, and that implementation types are in sub-packages.
+     */
+    @Pointcut("execution(* com.netflix.genie.web.controllers.*.*(..))")
+    public void resourceOperation() { }
+
+    /**
+     * A service operation is the execution of any method defined on a
+     * service class/interface. This definition assumes that interfaces are placed in the
+     * "service" package, and that implementation types are in sub-packages.
+     */
+    @Pointcut("execution(* com.netflix.genie.core.services.*.*(..))")
+    public void serviceOperation() { }
+
+    /**
+     * A data service operation is the execution of any method defined on a
+     * dao interface. This definition assumes that interfaces are placed in the
+     * "dao" package, and that implementation types are in sub-packages.
+     */
+    @Pointcut("execution(* com.netflix.genie.core.jpa.services.*.*(..))")
+    public void dataOperation() { }
+}

genie-web/src/main/java/com/netflix/genie/web/aspect/package-info.java

@@ -0,0 +1,25 @@
+/*
+ *
+ *  Copyright 2016 Netflix, Inc.
+ *
+ *     Licensed under the Apache License, Version 2.0 (the "License");
+ *     you may not use this file except in compliance with the License.
+ *     You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *     Unless required by applicable law or agreed to in writing, software
+ *     distributed under the License is distributed on an "AS IS" BASIS,
+ *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *     See the License for the specific language governing permissions and
+ *     limitations under the License.
+ *
+ */
+
+/**
+ * Various Spring aspects for Genie web.
+ *
+ * @author amajumdar
+ * @since 3.0.0
+ */
+package com.netflix.genie.web.aspect;

genie-web/src/main/java/com/netflix/genie/web/configs/PropertiesConfig.java

@@ -17,6 +17,7 @@
  */
 package com.netflix.genie.web.configs;
 
+import com.netflix.genie.core.properties.DataServiceRetryProperties;
 import com.netflix.genie.core.properties.JobsProperties;
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.context.annotation.Bean;
@@ -41,4 +42,15 @@ public class PropertiesConfig {
     public JobsProperties jobsProperties() {
         return new JobsProperties();
     }
+
+    /**
+     * All the properties related to configuring data service retries.
+     *
+     * @return The data service retry properties structure
+     */
+    @Bean
+    @ConfigurationProperties("genie.data.service.retry")
+    public DataServiceRetryProperties dataServiceRetryProperties() {
+        return new DataServiceRetryProperties();
+    }
 }