[ 
https://issues.apache.org/jira/browse/SCB-996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16669686#comment-16669686
 ] 

ASF GitHub Bot commented on SCB-996:
------------------------------------

liubao68 closed pull request #975: [SCB-996]When retries fail, return the last 
error
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/975
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
 
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
index 1da2e7c69..ae545eb0e 100644
--- 
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
+++ 
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
@@ -210,6 +210,16 @@ private static void testController(RestTemplate template, 
String microserviceNam
             String.class,
             "hello 中国"));
 
+    try {
+      template.postForObject(prefix + "/controller/sayhello/{name}",
+          null,
+          String.class,
+          "exception");
+      TestMgr.check(true, false);
+    } catch (InvocationException e) {
+      TestMgr.check(e.getStatusCode(), 503);
+    }
+
     HttpHeaders headers = new HttpHeaders();
     headers.add("name", "world");
     @SuppressWarnings("rawtypes")
diff --git 
a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java
 
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java
index 3c68b97b1..46adbe68f 100644
--- 
a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java
+++ 
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/ControllerImpl.java
@@ -22,9 +22,11 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.validation.constraints.Min;
 import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response.Status;
 
 import org.apache.servicecomb.demo.controller.Person;
 import org.apache.servicecomb.provider.rest.common.RestSchema;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.PostMapping;
@@ -44,6 +46,9 @@ public int add(@Min(1) @RequestParam("a") int a, @Min(1) 
@RequestParam("b") int
 
   @PostMapping(path = "/sayhello/{name}")
   public String sayHello(@PathVariable("name") String name) {
+    if ("exception".equals(name)) {
+      throw new InvocationException(Status.SERVICE_UNAVAILABLE, "");
+    }
     return "hello " + name;
   }
 
diff --git 
a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java
 
b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java
index c6aa4206f..fd3190f6b 100644
--- 
a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java
+++ 
b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java
@@ -243,8 +243,8 @@ public static String getStringProperty(String defaultValue, 
String... keys) {
   }
 
   public int getContinuousFailureThreshold(String microservice) {
-    final int defaultValue = 2;
-    String p = getStringProperty("2",
+    final int defaultValue = 5;
+    String p = getStringProperty("5",
         PROP_ROOT + microservice + "." + FILTER_ISOLATION + 
FILTER_CONTINUOUS_FAILURE_THRESHOLD,
         PROP_ROOT + FILTER_ISOLATION + FILTER_CONTINUOUS_FAILURE_THRESHOLD);
     try {
diff --git 
a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java
 
b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java
index 707651da2..d36c55242 100644
--- 
a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java
+++ 
b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java
@@ -70,7 +70,8 @@
   public static final String SERVICECOMB_SERVER_ENDPOINT = "scb-endpoint";
 
   public static final boolean supportDefinedEndpoint =
-      
DynamicPropertyFactory.getInstance().getBooleanProperty("servicecomb.loadbalance.userDefinedEndpoint.enabled",
 false).get();
+      DynamicPropertyFactory.getInstance()
+          
.getBooleanProperty("servicecomb.loadbalance.userDefinedEndpoint.enabled", 
false).get();
 
   // just a wrapper to make sure in retry mode to choose a different server.
   class RetryLoadBalancer implements ILoadBalancer {
@@ -317,14 +318,29 @@ public void 
onExecutionSuccess(ExecutionContext<Invocation> context, Response re
       @Override
       public void onExecutionFailed(ExecutionContext<Invocation> context, 
Throwable finalException,
           ExecutionInfo info) {
+        LOGGER.error("Invoke all server failed. Operation {}, e={}",
+            context.getRequest().getInvocationQualifiedName(), 
finalException.toString());
         if (orginExecutor != null) {
           orginExecutor.execute(() -> {
-            asyncResp.consumerFail(finalException);
+            fail(finalException);
           });
         } else {
-          asyncResp.consumerFail(finalException);
+          fail(finalException);
         }
       }
+
+      private void fail(Throwable finalException) {
+        int depth = 10;
+        Throwable t = finalException;
+        while (depth-- > 0) {
+          if (t instanceof InvocationException) {
+            asyncResp.consumerFail(t);
+            return;
+          }
+          t = finalException.getCause();
+        }
+        asyncResp.consumerFail(finalException);
+      }
     };
     List<ExecutionListener<Invocation, Response>> listeners = new 
ArrayList<>(0);
     listeners.add(listener);


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> When retries fail, return the last error
> ----------------------------------------
>
>                 Key: SCB-996
>                 URL: https://issues.apache.org/jira/browse/SCB-996
>             Project: Apache ServiceComb
>          Issue Type: Improvement
>          Components: Java-Chassis
>            Reporter: liubao
>            Assignee: liubao
>            Priority: Major
>
> Two improvements
>  # when retry all servers failed, return the last exception to caller, not 
> return the wrapped 490 error
>  # change isolation default value of continues error from 2 to 5, because in 
> retried mode, it'll be easily reach 2 when configured retry on same. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to