BFD-3746: Improve support for RDS autoscaling using AWS JDBC Wrapper (#…

…2487)
CMSgov · Dec 2, 2024 · a142293 · a142293
1 parent 2ddc3b3
commit a142293
Show file tree

Hide file tree

Showing 33 changed files with 2,090 additions and 189 deletions.
diff --git a/...erver/war/commons/RetryOnRDSFailover.java → ...RetryOnFailoverOrConnectionException.java b/...erver/war/commons/RetryOnRDSFailover.java → ...RetryOnFailoverOrConnectionException.java
@@ -7,6 +7,7 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import org.hibernate.exception.JDBCConnectionException;
 import org.springframework.core.annotation.AliasFor;
 import org.springframework.retry.annotation.Backoff;
 import org.springframework.retry.annotation.Retryable;
@@ -15,27 +16,32 @@
 /**
  * Annotation encapsulating common parameters for the {@link Retryable} spring-retry annotation for
  * use with {@link Search} and {@link Read} annotated resource provider methods that need to retry
- * on {@link FailoverSQLException}s.
+ * on {@link FailoverSQLException}s or {@link JDBCConnectionException}s.
  *
  * <p>Some providers wrap checked {@link Exception}s thrown by asynchronous tasks in {@link
  * RuntimeException}s, thus using a simple "retryFor" expression matching on {@link
- * FailoverSQLException} is not possible for all provider operations. Instead, a SpEL expression is
- * used which calls {@link SpringRetryUtils#shouldRetryIfFailover(Exception)} which will enable
- * retries for {@link Exception}s that are {@link FailoverSQLException}s or {@link Exception}s with
- * a root cause of {@link FailoverSQLException}.
+ * FailoverSQLException} or {@link JDBCConnectionException} is not possible for all provider
+ * operations. Instead, a SpEL expression is used which calls {@link
+ * SpringRetryUtils#shouldRetryIfFailoverOrConnectionException(Exception)} which will enable retries
+ * for {@link Exception}s that are {@link FailoverSQLException}s or {@link JDBCConnectionException}s
+ * or {@link Exception}s with inner causes including either.
  */
 @Target({ElementType.METHOD, ElementType.TYPE})
 @Retention(RetentionPolicy.RUNTIME)
 @Documented
-@Retryable(exceptionExpression = SpringRetryUtils.SHOULD_RETRY_IF_FAILOVER_EXCEPTION_EXPRESSION)
-public @interface RetryOnRDSFailover {
+@Retryable(
+    exceptionExpression =
+        SpringRetryUtils.SHOULD_RETRY_IF_FAILOVER_OR_CONNECTION_EXCEPTION_EXPRESSION)
+public @interface RetryOnFailoverOrConnectionException {
   /**
    * Alias for {@link Retryable#backoff()}.
    *
-   * @return a default {@link Backoff} with a delay of 5000 milliseconds
+   * @return a default {@link Backoff} with a starting delay of 300 ms increasing up to 5 seconds
+   *     with some jitter to reduce the likelihood of a "thundering herd"
    */
   @AliasFor(annotation = Retryable.class, attribute = "backoff")
-  Backoff backoff() default @Backoff(delay = 5000);
+  Backoff backoff() default
+      @Backoff(delay = 1000, multiplier = 1.2, maxDelay = 3000, random = true);
 
   /**
    * Alias for {@link Retryable#stateful()}.
@@ -48,10 +54,10 @@
   /**
    * Alias for {@link Retryable#maxAttempts()}.
    *
-   * @return a default of {@code 3} attempts
+   * @return a default of {@code 10} attempts
    */
   @AliasFor(annotation = Retryable.class, attribute = "maxAttempts")
-  int maxAttempts() default 3;
+  int maxAttempts() default 5;
 
   /**
    * Alias for {@link Retryable#listeners()}.

diff --git a/...-server/bfd-server-war/src/main/java/gov/cms/bfd/server/war/commons/SpringRetryUtils.java b/...-server/bfd-server-war/src/main/java/gov/cms/bfd/server/war/commons/SpringRetryUtils.java
@@ -1,6 +1,9 @@
 package gov.cms.bfd.server.war.commons;
 
 import org.apache.commons.lang3.exception.ExceptionUtils;
+import org.hibernate.exception.JDBCConnectionException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.retry.annotation.Retryable;
 import software.amazon.jdbc.plugin.failover.FailoverSQLException;
 
@@ -10,24 +13,38 @@
  * operation can be retried.
  */
 public final class SpringRetryUtils {
+  /** Logger for this class. */
+  static final Logger LOGGER = LoggerFactory.getLogger(SpringRetryUtils.class);
+
+  /** Private constructor stopping instantiation of this class. */
+  private SpringRetryUtils() {
+    throw new UnsupportedOperationException();
+  }
+
   /**
    * Constant SpEL expression used for {@link Retryable#exceptionExpression()}s that need to invoke
-   * the {@link #shouldRetryIfFailover(Exception)} method.
+   * the {@link #shouldRetryIfFailoverOrConnectionException(Exception)} method.
    */
-  public static final String SHOULD_RETRY_IF_FAILOVER_EXCEPTION_EXPRESSION =
-      "T(gov.cms.bfd.server.war.commons.SpringRetryUtils).shouldRetryIfFailover(#root)";
+  public static final String SHOULD_RETRY_IF_FAILOVER_OR_CONNECTION_EXCEPTION_EXPRESSION =
+      "T(gov.cms.bfd.server.war.commons.SpringRetryUtils).shouldRetryIfFailoverOrConnectionException(#root)";
 
   /**
    * Returns if a given {@link Exception} should be retried or not provided that it is an instance
-   * of {@link FailoverSQLException} or its "root cause" (innermost {@link Throwable}) is an
-   * instance of {@link FailoverSQLException}.
+   * of {@link FailoverSQLException} or {@link JDBCConnectionException} or if any of its inner
+   * causes ({@link Throwable}s) are themselves instances of the aforementioned exceptions.
    *
-   * @param ex the {@link Exception} to check for a root cause of {@link FailoverSQLException}
-   * @return {@code true} if the root cause of the given {@link Exception} is a {@link
-   *     FailoverSQLException}, {@code false} otherwise
+   * @param ex the {@link Exception} to check
+   * @return {@code true} if the given {@link Exception} is a {@link FailoverSQLException}, {@link
+   *     JDBCConnectionException}, or any of its causes are either. {@code false} otherwise
    */
-  public static boolean shouldRetryIfFailover(Exception ex) {
-    return ex instanceof FailoverSQLException
-        || ExceptionUtils.getRootCause(ex) instanceof FailoverSQLException;
+  public static boolean shouldRetryIfFailoverOrConnectionException(Exception ex) {
+    LOGGER.warn("Retrying operation due to exception.", ex);
+    return ex instanceof JDBCConnectionException
+        || ex instanceof FailoverSQLException
+        || ExceptionUtils.getThrowableList(ex).stream()
+            .anyMatch(
+                innerEx ->
+                    innerEx instanceof JDBCConnectionException
+                        || innerEx instanceof FailoverSQLException);
   }
 }
diff --git a/...ver-war/src/main/java/gov/cms/bfd/server/war/r4/providers/R4CoverageResourceProvider.java b/...ver-war/src/main/java/gov/cms/bfd/server/war/r4/providers/R4CoverageResourceProvider.java
@@ -31,7 +31,7 @@
 import gov.cms.bfd.server.war.commons.Profile;
 import gov.cms.bfd.server.war.commons.ProfileConstants;
 import gov.cms.bfd.server.war.commons.QueryUtils;
-import gov.cms.bfd.server.war.commons.RetryOnRDSFailover;
+import gov.cms.bfd.server.war.commons.RetryOnFailoverOrConnectionException;
 import jakarta.persistence.EntityManager;
 import jakarta.persistence.NoResultException;
 import jakarta.persistence.PersistenceContext;
@@ -141,7 +141,7 @@ public Class<? extends IBaseResource> getResourceType() {
    */
   @Read(version = false)
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Coverage read(@IdParam IdType coverageId) {
     if (coverageId == null) {
       throw new InvalidRequestException("Missing required coverage ID");
@@ -243,7 +243,7 @@ public Coverage read(@IdParam IdType coverageId) {
    */
   @Search
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Bundle searchByBeneficiary(
       @RequiredParam(name = Coverage.SP_BENEFICIARY)
           @Description(

diff --git a/...main/java/gov/cms/bfd/server/war/r4/providers/R4ExplanationOfBenefitResourceProvider.java b/...main/java/gov/cms/bfd/server/war/r4/providers/R4ExplanationOfBenefitResourceProvider.java
@@ -33,7 +33,7 @@
 import gov.cms.bfd.server.war.commons.LoggingUtils;
 import gov.cms.bfd.server.war.commons.OffsetLinkBuilder;
 import gov.cms.bfd.server.war.commons.OpenAPIContentProvider;
-import gov.cms.bfd.server.war.commons.RetryOnRDSFailover;
+import gov.cms.bfd.server.war.commons.RetryOnFailoverOrConnectionException;
 import jakarta.persistence.EntityManager;
 import jakarta.persistence.NoResultException;
 import jakarta.persistence.PersistenceContext;
@@ -192,7 +192,7 @@ public Class<? extends IBaseResource> getResourceType() {
   @SuppressWarnings({"rawtypes", "unchecked"})
   @Read(version = false)
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public ExplanationOfBenefit read(@IdParam IdType eobId, RequestDetails requestDetails) {
 
     Matcher eobIdMatcher =
@@ -282,7 +282,7 @@ metricRegistry, getClass().getSimpleName(), "query", "eob_by_id")) {
    */
   @Search
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Bundle findByPatient(
       @RequiredParam(name = ExplanationOfBenefit.SP_PATIENT)
           @Description(

diff --git a/...rver-war/src/main/java/gov/cms/bfd/server/war/r4/providers/R4PatientResourceProvider.java b/...rver-war/src/main/java/gov/cms/bfd/server/war/r4/providers/R4PatientResourceProvider.java
@@ -40,7 +40,7 @@
 import gov.cms.bfd.server.war.commons.PatientLinkBuilder;
 import gov.cms.bfd.server.war.commons.QueryUtils;
 import gov.cms.bfd.server.war.commons.RequestHeaders;
-import gov.cms.bfd.server.war.commons.RetryOnRDSFailover;
+import gov.cms.bfd.server.war.commons.RetryOnFailoverOrConnectionException;
 import gov.cms.bfd.server.war.commons.TransformerConstants;
 import jakarta.persistence.EntityManager;
 import jakarta.persistence.NoResultException;
@@ -169,7 +169,7 @@ public Class<? extends IBaseResource> getResourceType() {
    */
   @Read(version = false)
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Patient read(@IdParam IdType patientId, RequestDetails requestDetails) {
     if (patientId == null || patientId.getIdPart() == null) {
       throw new InvalidRequestException("Missing required patient ID");
@@ -235,7 +235,7 @@ public Patient read(@IdParam IdType patientId, RequestDetails requestDetails) {
    */
   @Search
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Bundle searchByLogicalId(
       @RequiredParam(name = Patient.SP_RES_ID)
           @Description(
@@ -355,7 +355,7 @@ public Bundle searchByLogicalId(
    */
   @Search
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Bundle searchByCoverageContract(
       // This is very explicit as a place holder until this kind
       // of relational search is more common.
@@ -685,7 +685,7 @@ private TypedQuery<Beneficiary> queryBeneficiariesByIds(List<String> ids) {
    */
   @Search
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Bundle searchByIdentifier(
       @RequiredParam(name = Patient.SP_IDENTIFIER)
           @Description(

diff --git a/...war/src/main/java/gov/cms/bfd/server/war/r4/providers/pac/AbstractR4ResourceProvider.java b/...war/src/main/java/gov/cms/bfd/server/war/r4/providers/pac/AbstractR4ResourceProvider.java
@@ -28,7 +28,7 @@
 import gov.cms.bfd.server.war.commons.AbstractResourceProvider;
 import gov.cms.bfd.server.war.commons.OffsetLinkBuilder;
 import gov.cms.bfd.server.war.commons.OpenAPIContentProvider;
-import gov.cms.bfd.server.war.commons.RetryOnRDSFailover;
+import gov.cms.bfd.server.war.commons.RetryOnFailoverOrConnectionException;
 import gov.cms.bfd.server.war.r4.providers.TransformerUtilsV2;
 import gov.cms.bfd.server.war.r4.providers.pac.common.ClaimDao;
 import gov.cms.bfd.server.war.r4.providers.pac.common.ResourceTransformer;
@@ -202,7 +202,7 @@ void setResourceType() {
    */
   @Read
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public T read(@IdParam IdType claimId, RequestDetails requestDetails) {
     if (claimId == null) {
       throw new InvalidRequestException("Resource ID can not be null");
@@ -394,7 +394,7 @@ private void logMbiIdentifiersToMdc(Mbi mbi) {
    */
   @Search
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Bundle findByPatient(
       @RequiredParam(name = "mbi")
           @Description(

diff --git a/...ver-war/src/main/java/gov/cms/bfd/server/war/stu3/providers/CoverageResourceProvider.java b/...ver-war/src/main/java/gov/cms/bfd/server/war/stu3/providers/CoverageResourceProvider.java
@@ -28,7 +28,7 @@
 import gov.cms.bfd.server.war.commons.OffsetLinkBuilder;
 import gov.cms.bfd.server.war.commons.OpenAPIContentProvider;
 import gov.cms.bfd.server.war.commons.QueryUtils;
-import gov.cms.bfd.server.war.commons.RetryOnRDSFailover;
+import gov.cms.bfd.server.war.commons.RetryOnFailoverOrConnectionException;
 import jakarta.persistence.EntityManager;
 import jakarta.persistence.NoResultException;
 import jakarta.persistence.PersistenceContext;
@@ -126,7 +126,7 @@ public Class<? extends IBaseResource> getResourceType() {
    */
   @Read(version = false)
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Coverage read(@IdParam IdType coverageId) {
     if (coverageId == null) {
       throw new InvalidRequestException("Missing required coverage ID");
@@ -200,7 +200,7 @@ public Coverage read(@IdParam IdType coverageId) {
    */
   @Search
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Bundle searchByBeneficiary(
       @RequiredParam(name = Coverage.SP_BENEFICIARY)
           @Description(

diff --git a/...main/java/gov/cms/bfd/server/war/stu3/providers/ExplanationOfBenefitResourceProvider.java b/...main/java/gov/cms/bfd/server/war/stu3/providers/ExplanationOfBenefitResourceProvider.java
@@ -30,7 +30,7 @@
 import gov.cms.bfd.server.war.commons.LoggingUtils;
 import gov.cms.bfd.server.war.commons.OffsetLinkBuilder;
 import gov.cms.bfd.server.war.commons.OpenAPIContentProvider;
-import gov.cms.bfd.server.war.commons.RetryOnRDSFailover;
+import gov.cms.bfd.server.war.commons.RetryOnFailoverOrConnectionException;
 import jakarta.persistence.EntityManager;
 import jakarta.persistence.NoResultException;
 import jakarta.persistence.PersistenceContext;
@@ -187,7 +187,7 @@ public Class<? extends IBaseResource> getResourceType() {
   @SuppressWarnings({"rawtypes", "unchecked"})
   @Read(version = false)
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public ExplanationOfBenefit read(@IdParam IdType eobId, RequestDetails requestDetails) {
 
     if (eobId == null) {
@@ -276,7 +276,7 @@ metricRegistry, getClass().getSimpleName(), "query", "eob_by_id")) {
    */
   @Search
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Bundle findByPatient(
       @RequiredParam(name = ExplanationOfBenefit.SP_PATIENT)
           @Description(

diff --git a/...rver-war/src/main/java/gov/cms/bfd/server/war/stu3/providers/PatientResourceProvider.java b/...rver-war/src/main/java/gov/cms/bfd/server/war/stu3/providers/PatientResourceProvider.java
@@ -40,7 +40,7 @@
 import gov.cms.bfd.server.war.commons.PatientLinkBuilder;
 import gov.cms.bfd.server.war.commons.QueryUtils;
 import gov.cms.bfd.server.war.commons.RequestHeaders;
-import gov.cms.bfd.server.war.commons.RetryOnRDSFailover;
+import gov.cms.bfd.server.war.commons.RetryOnFailoverOrConnectionException;
 import gov.cms.bfd.server.war.commons.TransformerConstants;
 import jakarta.persistence.EntityManager;
 import jakarta.persistence.NoResultException;
@@ -156,7 +156,7 @@ public Class<? extends IBaseResource> getResourceType() {
    */
   @Read(version = false)
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Patient read(@IdParam IdType patientId, RequestDetails requestDetails) {
     if (patientId == null || patientId.getIdPart() == null) {
       throw new InvalidRequestException("Missing required patient ID");
@@ -220,7 +220,7 @@ public Patient read(@IdParam IdType patientId, RequestDetails requestDetails) {
    */
   @Search
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Bundle searchByCoverageContract(
       // This is very explicit as a place holder until this kind
       // of relational search is more common.
@@ -285,7 +285,7 @@ public Bundle searchByCoverageContract(
    */
   @Search
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Bundle searchByLogicalId(
       @RequiredParam(name = Patient.SP_RES_ID)
           @Description(
@@ -741,7 +741,7 @@ private List<Beneficiary> queryBeneficiariesByIdsWithBeneficiaryMonthlys(List<Lo
    */
   @Search
   @Trace
-  @RetryOnRDSFailover
+  @RetryOnFailoverOrConnectionException
   public Bundle searchByIdentifier(
       @RequiredParam(name = Patient.SP_IDENTIFIER)
           @Description(shortDefinition = "The patient identifier to search for")