You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2022/06/23 08:28:54 UTC

[servicecomb-java-chassis] branch master updated (e1493d8fc -> 3f2493eea)

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

liubao pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git


    from e1493d8fc [SCB-2594] [optimize] use enhanced for loop (#3112)
     new d1b3b2e71 [SCB-2596]retry add failAfterMaxAttempts property
     new 3f2493eea [SCB-2596]retry add retryOnSame property

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../governance/handler/BulkheadHandler.java        |  2 +-
 .../governance/handler/CircuitBreakerHandler.java  |  2 +-
 .../handler/InstanceIsolationHandler.java          |  2 +-
 .../governance/handler/RateLimitingHandler.java    |  2 +-
 .../governance/handler/RetryHandler.java           |  3 +-
 .../servicecomb/governance/policy/RetryPolicy.java | 39 ++++++++--
 .../servicecomb/governance/RetryHandlerTest.java   | 83 ++++++++++++++++++++++
 7 files changed, 122 insertions(+), 11 deletions(-)
 create mode 100644 governance/src/test/java/org/apache/servicecomb/governance/RetryHandlerTest.java


[servicecomb-java-chassis] 01/02: [SCB-2596]retry add failAfterMaxAttempts property

Posted by li...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git

commit d1b3b2e71051f3157f833c2bd0df5542d0f19e93
Author: liubao <bi...@qq.com>
AuthorDate: Thu Jun 23 15:17:43 2022 +0800

    [SCB-2596]retry add failAfterMaxAttempts property
---
 .../governance/handler/BulkheadHandler.java        |  2 +-
 .../governance/handler/CircuitBreakerHandler.java  |  2 +-
 .../handler/InstanceIsolationHandler.java          |  2 +-
 .../governance/handler/RateLimitingHandler.java    |  2 +-
 .../governance/handler/RetryHandler.java           |  3 +-
 .../servicecomb/governance/policy/RetryPolicy.java | 27 +++++--
 .../governance/CircuitBreakerHandlerTest.java      | 83 ++++++++++++++++++++++
 7 files changed, 110 insertions(+), 11 deletions(-)

diff --git a/governance/src/main/java/org/apache/servicecomb/governance/handler/BulkheadHandler.java b/governance/src/main/java/org/apache/servicecomb/governance/handler/BulkheadHandler.java
index fea76a617..1b3211940 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/handler/BulkheadHandler.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/handler/BulkheadHandler.java
@@ -49,7 +49,7 @@ public class BulkheadHandler extends AbstractGovernanceHandler<Bulkhead, Bulkhea
   }
 
   @Override
-  protected Bulkhead createProcessor(GovernanceRequest governanceRequest, BulkheadPolicy policy) {
+  public Bulkhead createProcessor(GovernanceRequest governanceRequest, BulkheadPolicy policy) {
     return getBulkhead(policy);
   }
 
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/handler/CircuitBreakerHandler.java b/governance/src/main/java/org/apache/servicecomb/governance/handler/CircuitBreakerHandler.java
index 65f271188..9120ef93e 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/handler/CircuitBreakerHandler.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/handler/CircuitBreakerHandler.java
@@ -55,7 +55,7 @@ public class CircuitBreakerHandler extends AbstractGovernanceHandler<CircuitBrea
   }
 
   @Override
-  protected CircuitBreaker createProcessor(GovernanceRequest governanceRequest, CircuitBreakerPolicy policy) {
+  public CircuitBreaker createProcessor(GovernanceRequest governanceRequest, CircuitBreakerPolicy policy) {
     return getCircuitBreaker(policy);
   }
 
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/handler/InstanceIsolationHandler.java b/governance/src/main/java/org/apache/servicecomb/governance/handler/InstanceIsolationHandler.java
index ade19871d..aba959a65 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/handler/InstanceIsolationHandler.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/handler/InstanceIsolationHandler.java
@@ -83,7 +83,7 @@ public class InstanceIsolationHandler extends AbstractGovernanceHandler<CircuitB
   }
 
   @Override
-  protected CircuitBreaker createProcessor(GovernanceRequest governanceRequest, CircuitBreakerPolicy policy) {
+  public CircuitBreaker createProcessor(GovernanceRequest governanceRequest, CircuitBreakerPolicy policy) {
     return getCircuitBreaker(governanceRequest, policy);
   }
 
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/handler/RateLimitingHandler.java b/governance/src/main/java/org/apache/servicecomb/governance/handler/RateLimitingHandler.java
index aa3ee9f2b..ab5795bb0 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/handler/RateLimitingHandler.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/handler/RateLimitingHandler.java
@@ -48,7 +48,7 @@ public class RateLimitingHandler extends AbstractGovernanceHandler<RateLimiter,
   }
 
   @Override
-  protected RateLimiter createProcessor(GovernanceRequest governanceRequest, RateLimitingPolicy policy) {
+  public RateLimiter createProcessor(GovernanceRequest governanceRequest, RateLimitingPolicy policy) {
     return getRateLimiter(policy);
   }
 
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/handler/RetryHandler.java b/governance/src/main/java/org/apache/servicecomb/governance/handler/RetryHandler.java
index 42898c58b..a47545e2f 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/handler/RetryHandler.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/handler/RetryHandler.java
@@ -55,7 +55,7 @@ public class RetryHandler extends AbstractGovernanceHandler<Retry, RetryPolicy>
   }
 
   @Override
-  protected Retry createProcessor(GovernanceRequest governanceRequest, RetryPolicy policy) {
+  public Retry createProcessor(GovernanceRequest governanceRequest, RetryPolicy policy) {
     return getRetry(policy);
   }
 
@@ -67,6 +67,7 @@ public class RetryHandler extends AbstractGovernanceHandler<Retry, RetryPolicy>
         .retryOnResult(response -> retryExtension.isRetry(retryPolicy.getRetryOnResponseStatus(), response))
         .retryOnException(e -> retryExtension.isRetry(e))
         .intervalFunction(getIntervalFunction(retryPolicy))
+        .failAfterMaxAttempts(retryPolicy.isFailAfterMaxAttempts())
         .build();
 
     RetryRegistry registry = RetryRegistry.of(config);
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java b/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java
index cd6bbc3c6..a4b786ba0 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java
@@ -16,13 +16,13 @@
  */
 package org.apache.servicecomb.governance.policy;
 
-import org.apache.commons.lang3.StringUtils;
-import org.springframework.util.CollectionUtils;
-
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.commons.lang3.StringUtils;
+import org.springframework.util.CollectionUtils;
+
 public class RetryPolicy extends AbstractPolicy {
 
   public static final int DEFAULT_MAX_ATTEMPTS = 3;
@@ -39,23 +39,29 @@ public class RetryPolicy extends AbstractPolicy {
 
   private static final String DEFAULT_RETRY_STRATEGY = "FixedInterval";
 
-  //最多尝试次数
+  //max retry attempts
   private int maxAttempts = DEFAULT_MAX_ATTEMPTS;
 
-  //每次重试尝试等待的时间。
+  //wait duration for each retry
   private String waitDuration = DEFAULT_WAIT_DURATION.toString();
 
-  //需要重试的http status, 逗号分隔
+  //status code that need retry
   private List<String> retryOnResponseStatus = new ArrayList<>();
 
+  //retry strategy
   private String retryStrategy = DEFAULT_RETRY_STRATEGY;
 
+  // initial interval for backoff retry
   private String initialInterval = INITIAL_INTERVAL.toString();
 
+  // multiplier for backoff retry
   private float multiplier = MULTIPLIER;
 
+  // randomization factor for backoff retry
   private double randomizationFactor = RANDOMIZATION_FACTOR;
 
+  private boolean failAfterMaxAttempts = false;
+
   public List<String> getRetryOnResponseStatus() {
     if (CollectionUtils.isEmpty(retryOnResponseStatus)) {
       this.retryOnResponseStatus.add(DEFAULT_RETRY_ON_RESPONSE_STATUS);
@@ -118,6 +124,15 @@ public class RetryPolicy extends AbstractPolicy {
     this.randomizationFactor = randomizationFactor;
   }
 
+  public boolean isFailAfterMaxAttempts() {
+    return failAfterMaxAttempts;
+  }
+
+  public RetryPolicy setFailAfterMaxAttempts(boolean failAfterMaxAttempts) {
+    this.failAfterMaxAttempts = failAfterMaxAttempts;
+    return this;
+  }
+
   @Override
   public boolean isValid() {
     if (maxAttempts < 1) {
diff --git a/governance/src/test/java/org/apache/servicecomb/governance/CircuitBreakerHandlerTest.java b/governance/src/test/java/org/apache/servicecomb/governance/CircuitBreakerHandlerTest.java
new file mode 100644
index 000000000..80c4e61be
--- /dev/null
+++ b/governance/src/test/java/org/apache/servicecomb/governance/CircuitBreakerHandlerTest.java
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.servicecomb.governance;
+
+import org.apache.servicecomb.governance.handler.RetryHandler;
+import org.apache.servicecomb.governance.handler.ext.RetryExtension;
+import org.apache.servicecomb.governance.marker.GovernanceRequest;
+import org.apache.servicecomb.governance.policy.RetryPolicy;
+import org.apache.servicecomb.governance.properties.RetryProperties;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import io.github.resilience4j.retry.MaxRetriesExceededException;
+import io.github.resilience4j.retry.Retry;
+
+public class CircuitBreakerHandlerTest {
+  @Test
+  public void testNotFailAfterMaxAttemptsWhenThrow() {
+    RetryExtension retryExtension = Mockito.mock(RetryExtension.class);
+    RetryProperties retryProperties = Mockito.mock(RetryProperties.class);
+    GovernanceRequest governanceRequest = Mockito.mock(GovernanceRequest.class);
+    Mockito.when(retryExtension.isRetry(Mockito.any())).thenReturn(true);
+
+    RetryPolicy retryPolicy = new RetryPolicy();
+    retryPolicy.setName("test");
+    retryPolicy.setFailAfterMaxAttempts(false);
+    RetryHandler retryHandler = new RetryHandler(retryProperties, retryExtension);
+
+    Retry retry = retryHandler.createProcessor(governanceRequest, retryPolicy);
+    Assertions.assertThrows(IllegalStateException.class, () -> retry.<Integer>executeCheckedSupplier(() -> {
+      throw new IllegalStateException();
+    }));
+  }
+
+  @Test
+  public void testFailAfterMaxAttemptsWhenThrow() {
+    RetryExtension retryExtension = Mockito.mock(RetryExtension.class);
+    RetryProperties retryProperties = Mockito.mock(RetryProperties.class);
+    GovernanceRequest governanceRequest = Mockito.mock(GovernanceRequest.class);
+    Mockito.when(retryExtension.isRetry(Mockito.any())).thenReturn(true);
+
+    RetryPolicy retryPolicy = new RetryPolicy();
+    retryPolicy.setName("test");
+    retryPolicy.setFailAfterMaxAttempts(true);
+    RetryHandler retryHandler = new RetryHandler(retryProperties, retryExtension);
+
+    Retry retry = retryHandler.createProcessor(governanceRequest, retryPolicy);
+    Assertions.assertThrows(IllegalStateException.class, () -> retry.<Integer>executeCheckedSupplier(() -> {
+      throw new IllegalStateException();
+    }));
+  }
+
+  @Test
+  public void testFailAfterMaxAttemptsOnResult() {
+    RetryExtension retryExtension = Mockito.mock(RetryExtension.class);
+    RetryProperties retryProperties = Mockito.mock(RetryProperties.class);
+    GovernanceRequest governanceRequest = Mockito.mock(GovernanceRequest.class);
+    Mockito.when(retryExtension.isRetry(Mockito.any(), Mockito.any())).thenReturn(true);
+
+    RetryPolicy retryPolicy = new RetryPolicy();
+    retryPolicy.setName("test");
+    retryPolicy.setFailAfterMaxAttempts(true);
+    RetryHandler retryHandler = new RetryHandler(retryProperties, retryExtension);
+
+    Retry retry = retryHandler.createProcessor(governanceRequest, retryPolicy);
+    Assertions.assertThrows(MaxRetriesExceededException.class, () -> retry.<Integer>executeCheckedSupplier(() -> -1));
+  }
+}


[servicecomb-java-chassis] 02/02: [SCB-2596]retry add retryOnSame property

Posted by li...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git

commit 3f2493eea9a579d4c5f4d633fc9b6c61659761ae
Author: liubao <bi...@qq.com>
AuthorDate: Thu Jun 23 15:38:40 2022 +0800

    [SCB-2596]retry add retryOnSame property
---
 .../servicecomb/governance/policy/RetryPolicy.java       | 16 ++++++++++++++--
 ...cuitBreakerHandlerTest.java => RetryHandlerTest.java} |  2 +-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java b/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java
index a4b786ba0..ce534ae89 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java
@@ -60,8 +60,13 @@ public class RetryPolicy extends AbstractPolicy {
   // randomization factor for backoff retry
   private double randomizationFactor = RANDOMIZATION_FACTOR;
 
+  // if throw an MaxRetriesExceededException if retry condition is based on result
   private boolean failAfterMaxAttempts = false;
 
+  // if retry on the same instance. This property is not directly used in
+  // RetryHandler, but used for loadbalancers
+  private int retryOnSame = 0;
+
   public List<String> getRetryOnResponseStatus() {
     if (CollectionUtils.isEmpty(retryOnResponseStatus)) {
       this.retryOnResponseStatus.add(DEFAULT_RETRY_ON_RESPONSE_STATUS);
@@ -128,9 +133,16 @@ public class RetryPolicy extends AbstractPolicy {
     return failAfterMaxAttempts;
   }
 
-  public RetryPolicy setFailAfterMaxAttempts(boolean failAfterMaxAttempts) {
+  public void setFailAfterMaxAttempts(boolean failAfterMaxAttempts) {
     this.failAfterMaxAttempts = failAfterMaxAttempts;
-    return this;
+  }
+
+  public int getRetryOnSame() {
+    return retryOnSame;
+  }
+
+  public void setRetryOnSame(int retryOnSame) {
+    this.retryOnSame = retryOnSame;
   }
 
   @Override
diff --git a/governance/src/test/java/org/apache/servicecomb/governance/CircuitBreakerHandlerTest.java b/governance/src/test/java/org/apache/servicecomb/governance/RetryHandlerTest.java
similarity index 98%
rename from governance/src/test/java/org/apache/servicecomb/governance/CircuitBreakerHandlerTest.java
rename to governance/src/test/java/org/apache/servicecomb/governance/RetryHandlerTest.java
index 80c4e61be..c33cfb653 100644
--- a/governance/src/test/java/org/apache/servicecomb/governance/CircuitBreakerHandlerTest.java
+++ b/governance/src/test/java/org/apache/servicecomb/governance/RetryHandlerTest.java
@@ -28,7 +28,7 @@ import org.mockito.Mockito;
 import io.github.resilience4j.retry.MaxRetriesExceededException;
 import io.github.resilience4j.retry.Retry;
 
-public class CircuitBreakerHandlerTest {
+public class RetryHandlerTest {
   @Test
   public void testNotFailAfterMaxAttemptsWhenThrow() {
     RetryExtension retryExtension = Mockito.mock(RetryExtension.class);