[
https://issues.apache.org/jira/browse/SCB-833?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16579309#comment-16579309
]
ASF GitHub Bot commented on SCB-833:
------------------------------------
liubao68 closed pull request #868: [SCB-833]Provide a retry mechanism to meet
upgrade no interrupt
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/868
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/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
b/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
index 6c5ccb3ac..6e0aefec1 100644
--- a/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
+++ b/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
@@ -20,6 +20,8 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
+import javax.ws.rs.core.Response.Status;
+
import org.apache.servicecomb.config.ConfigUtil;
import org.apache.servicecomb.core.BootListener.BootEvent;
import org.apache.servicecomb.core.BootListener.EventType;
@@ -36,6 +38,7 @@
import org.apache.servicecomb.foundation.vertx.VertxUtils;
import org.apache.servicecomb.serviceregistry.RegistryUtils;
import
org.apache.servicecomb.serviceregistry.task.MicroserviceInstanceRegisterTask;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.util.StringUtils;
@@ -277,8 +280,10 @@ private void validAllInvocationFinished() throws
InterruptedException {
public void ensureStatusUp() {
SCBStatus currentStatus = getStatus();
if (!SCBStatus.UP.equals(currentStatus)) {
- throw new IllegalStateException(
- "The request is rejected, as the service cannot process the request
due to STATUS = " + currentStatus);
+ String message =
+ "The request is rejected. Cannot process the request due to STATUS =
" + currentStatus;
+ LOGGER.warn(message);
+ throw new InvocationException(Status.SERVICE_UNAVAILABLE, message);
}
}
diff --git a/core/src/test/java/org/apache/servicecomb/core/TestSCBEngine.java
b/core/src/test/java/org/apache/servicecomb/core/TestSCBEngine.java
index 89ffe806c..177a46bb3 100644
--- a/core/src/test/java/org/apache/servicecomb/core/TestSCBEngine.java
+++ b/core/src/test/java/org/apache/servicecomb/core/TestSCBEngine.java
@@ -29,6 +29,7 @@
import org.apache.servicecomb.foundation.vertx.VertxUtils;
import org.apache.servicecomb.serviceregistry.RegistryUtils;
import org.apache.servicecomb.serviceregistry.consumer.AppManager;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Rule;
@@ -104,9 +105,10 @@ public void createReferenceConfigForInvoke_down(@Mocked
ConsumerProviderManager
engine.setStatus(SCBStatus.DOWN);
engine.setConsumerProviderManager(consumerProviderManager);
- expectedException.expect(IllegalStateException.class);
+ expectedException.expect(InvocationException.class);
expectedException.expectMessage(
- Matchers.is("The request is rejected, as the service cannot process
the request due to STATUS = DOWN"));
+ Matchers
+ .is("InvocationException: code=503;msg=CommonExceptionData
[message=The request is rejected. Cannot process the request due to STATUS =
DOWN]"));
engine.createReferenceConfigForInvoke(null, null, null);
}
@@ -126,9 +128,10 @@ public void getReferenceConfigForInvoke_down(@Mocked
ConsumerProviderManager con
engine.setStatus(SCBStatus.DOWN);
engine.setConsumerProviderManager(consumerProviderManager);
- expectedException.expect(IllegalStateException.class);
+ expectedException.expect(InvocationException.class);
expectedException.expectMessage(
- Matchers.is("The request is rejected, as the service cannot process
the request due to STATUS = DOWN"));
+ Matchers
+ .is("InvocationException: code=503;msg=CommonExceptionData
[message=The request is rejected. Cannot process the request due to STATUS =
DOWN]"));
engine.getReferenceConfigForInvoke(null);
}
}
diff --git
a/core/src/test/java/org/apache/servicecomb/core/provider/consumer/TestInvokerUtils.java
b/core/src/test/java/org/apache/servicecomb/core/provider/consumer/TestInvokerUtils.java
index ef0b021a5..04e32b20c 100644
---
a/core/src/test/java/org/apache/servicecomb/core/provider/consumer/TestInvokerUtils.java
+++
b/core/src/test/java/org/apache/servicecomb/core/provider/consumer/TestInvokerUtils.java
@@ -124,9 +124,10 @@ Object syncInvoke(Invocation invocation) {
public void testSyncInvoke_4param_NotReady() {
scbEngine.setStatus(SCBStatus.DOWN);
- expectedException.expect(IllegalStateException.class);
+ expectedException.expect(InvocationException.class);
expectedException.expectMessage(
- Matchers.is("The request is rejected, as the service cannot process
the request due to STATUS = DOWN"));
+ Matchers
+ .is("InvocationException: code=503;msg=CommonExceptionData
[message=The request is rejected. Cannot process the request due to STATUS =
DOWN]"));
InvokerUtils.syncInvoke("ms", "schemaId", "opName", null);
}
@@ -134,9 +135,10 @@ public void testSyncInvoke_4param_NotReady() {
public void testSyncInvoke_6param_NotReady() {
scbEngine.setStatus(SCBStatus.DOWN);
- expectedException.expect(IllegalStateException.class);
+ expectedException.expect(InvocationException.class);
expectedException.expectMessage(
- Matchers.is("The request is rejected, as the service cannot process
the request due to STATUS = DOWN"));
+ Matchers
+ .is("InvocationException: code=503;msg=CommonExceptionData
[message=The request is rejected. Cannot process the request due to STATUS =
DOWN]"));
InvokerUtils.syncInvoke("ms", "latest", "rest", "schemaId", "opName",
null);
}
diff --git
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstRestTemplateSpringmvc.java
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstRestTemplateSpringmvc.java
index 2525f8757..af27ccdf3 100644
---
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstRestTemplateSpringmvc.java
+++
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/CodeFirstRestTemplateSpringmvc.java
@@ -137,17 +137,13 @@ private void testUpload(RestTemplate template, String
cseUrlPrefix) throws IOExc
String result = testRestTemplateUpload(template, cseUrlPrefix, file1,
someFile);
TestMgr.check(expect, result);
- {
- MultiValueMap<String, Object> map = new LinkedMultiValueMap<>();
- map.add("file1", new FileSystemResource(file1));
-
- result = template.postForObject(
- cseUrlPrefix + "/upload1",
- new HttpEntity<>(map),
- String.class);
+ MultiValueMap<String, Object> map = new LinkedMultiValueMap<>();
+ map.add("file1", new FileSystemResource(file1));
- System.out.println(result);
- }
+ result = template.postForObject(
+ cseUrlPrefix + "/upload1",
+ new HttpEntity<>(map),
+ String.class);
result = uploadPartAndFile.fileUpload(new FilePart(null, file1), someFile);
TestMgr.check(expect, result);
@@ -228,6 +224,9 @@ private void testResponseEntity(String microserviceName,
RestTemplate template,
TestMgr.check("h1v " + srcName,
responseEntity.getHeaders().getFirst("h1"));
TestMgr.check("h2v " + srcName,
responseEntity.getHeaders().getFirst("h2"));
checkStatusCode(microserviceName, 202, responseEntity.getStatusCode());
+
+ int retryResult = template.getForObject(cseUrlPrefix +
"retrySuccess?a=2&b=3", Integer.class);
+ TestMgr.check(retryResult, 5);
}
protected void testCodeFirstTestForm(RestTemplate template, String
cseUrlPrefix) {
diff --git
a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
index 01a21587f..74f8b1161 100644
---
a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
+++
b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/CodeFirstSpringmvc.java
@@ -22,6 +22,8 @@
import java.util.Date;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.Part;
@@ -87,6 +89,8 @@
public class CodeFirstSpringmvc {
private static final Logger LOGGER =
LoggerFactory.getLogger(CodeFirstSpringmvc.class);
+ private AtomicInteger firstInovcation = new AtomicInteger(2);
+
private String _fileUpload(MultipartFile file1, Part file2) {
try (InputStream is1 = file1.getInputStream(); InputStream is2 =
file2.getInputStream()) {
String content1 = IOUtils.toString(is1);
@@ -104,6 +108,14 @@ private String _fileUpload(MultipartFile file1, Part
file2) {
}
}
+ @GetMapping(path = "/retrySuccess")
+ public int retrySuccess(@RequestParam("a") int a, @RequestParam("b") int b) {
+ if (firstInovcation.getAndDecrement() > 0) {
+ throw new InvocationException(Status.SERVICE_UNAVAILABLE, "try again
later.");
+ }
+ return a + b;
+ }
+
@PostMapping(path = "/upload1", produces = MediaType.TEXT_PLAIN_VALUE)
public String fileUpload1(@RequestPart(name = "file1") MultipartFile file1)
throws IOException {
try (InputStream is = file1.getInputStream()) {
diff --git
a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/DefaultRetryExtensionsFactory.java
b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/DefaultRetryExtensionsFactory.java
index 546c3e33e..118fe0c45 100644
---
a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/DefaultRetryExtensionsFactory.java
+++
b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/DefaultRetryExtensionsFactory.java
@@ -16,8 +16,13 @@
*/
package org.apache.servicecomb.loadbalance;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.SocketTimeoutException;
import java.util.Collection;
+import java.util.List;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
import org.springframework.stereotype.Component;
import com.google.common.collect.Lists;
@@ -40,15 +45,31 @@ public boolean isSupport(String key, String value) {
return ACCEPT_KEYS.contains(key) && ACCEPT_VALUES.contains(value);
}
+ @SuppressWarnings("unchecked")
public RetryHandler createRetryHandler(String retryName, String
microservice) {
- RetryHandler handler = new DefaultLoadBalancerRetryHandler(
+ return new DefaultLoadBalancerRetryHandler(
Configuration.INSTANCE.getRetryOnSame(microservice),
Configuration.INSTANCE.getRetryOnNext(microservice), true) {
+ private List<Class<? extends Throwable>> retriable = Lists
+ .newArrayList(new Class[] {ConnectException.class,
SocketTimeoutException.class, IOException.class});
+
@Override
public boolean isRetriableException(Throwable e, boolean sameServer) {
- return Utils.isPresentAsCause(e, getRetriableExceptions());
+ boolean retriable = Utils.isPresentAsCause(e,
getRetriableExceptions());
+ if (!retriable) {
+ if (e instanceof InvocationException) {
+ if (((InvocationException) e).getStatusCode() == 503) {
+ return true;
+ }
+ }
+ }
+ return retriable;
+ }
+
+ @Override
+ protected List<Class<? extends Throwable>> getRetriableExceptions() {
+ return this.retriable;
}
};
- return handler;
}
}
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 9750f1947..7d6394a46 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
@@ -44,6 +44,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.IRule;
import com.netflix.loadbalancer.Server;
import com.netflix.loadbalancer.reactive.ExecutionContext;
@@ -59,6 +60,60 @@
*
*/
public class LoadbalanceHandler implements Handler {
+ // just a wrapper to make sure in retry mode to choose a different server.
+ class RetryLoadBalancer implements ILoadBalancer {
+ // Enough times to make sure to choose a different server in high volume.
+ static final int COUNT = 17;
+
+ Server lastServer = null;
+
+ ILoadBalancer delegate;
+
+ RetryLoadBalancer(ILoadBalancer delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public void addServers(List<Server> newServers) {
+ delegate.addServers(newServers);
+ }
+
+ @Override
+ public Server chooseServer(Object key) {
+ for (int i = 0; i < COUNT; i++) {
+ Server s = delegate.chooseServer(key);
+ if (s != null && !s.equals(lastServer)) {
+ lastServer = s;
+ break;
+ }
+ }
+
+ return lastServer;
+ }
+
+
+ @Override
+ public void markServerDown(Server server) {
+ delegate.markServerDown(server);
+ }
+
+ @Override
+ @Deprecated
+ public List<Server> getServerList(boolean availableOnly) {
+ return delegate.getServerList(availableOnly);
+ }
+
+ @Override
+ public List<Server> getReachableServers() {
+ return delegate.getReachableServers();
+ }
+
+ @Override
+ public List<Server> getAllServers() {
+ return delegate.getAllServers();
+ }
+ }
+
private static final Logger LOGGER =
LoggerFactory.getLogger(LoadbalanceHandler.class);
private static final ExecutorService RETRY_POOL =
Executors.newCachedThreadPool(new ThreadFactory() {
@@ -190,15 +245,22 @@ public void
onStartWithServer(ExecutionContext<Invocation> context,
@Override
public void onExceptionWithServer(ExecutionContext<Invocation> context,
Throwable exception,
ExecutionInfo info) {
- LOGGER.error("onExceptionWithServer operation {}; msg {}; server {}",
+ LOGGER.error("Invoke server failed. Operation {}; server {}; {}-{} msg
{}",
context.getRequest().getInvocationQualifiedName(),
- exception.getMessage(),
- context.getRequest().getEndpoint());
+ context.getRequest().getEndpoint(),
+ info.getNumberOfPastServersAttempted(),
+ info.getNumberOfPastAttemptsOnServer(),
+ exception.getMessage());
}
@Override
public void onExecutionSuccess(ExecutionContext<Invocation> context,
Response response,
ExecutionInfo info) {
+ if (info.getNumberOfPastServersAttempted() > 0 ||
info.getNumberOfPastAttemptsOnServer() > 0) {
+ LOGGER.error("Invoke server success. Operation {}; server {}",
+ context.getRequest().getInvocationQualifiedName(),
+ context.getRequest().getEndpoint());
+ }
if (orginExecutor != null) {
orginExecutor.execute(() -> {
asyncResp.complete(response);
@@ -225,7 +287,7 @@ public void onExecutionFailed(ExecutionContext<Invocation>
context, Throwable fi
ExecutionContext<Invocation> context = new ExecutionContext<>(invocation,
null, null, null);
LoadBalancerCommand<Response> command =
LoadBalancerCommand.<Response>builder()
- .withLoadBalancer(chosenLB)
+ .withLoadBalancer(new RetryLoadBalancer(chosenLB))
.withServerLocator(invocation)
.withRetryHandler(ExtensionsManager.createRetryHandler(invocation.getMicroserviceName()))
.withListeners(listeners)
@@ -276,7 +338,8 @@ protected boolean isFailedResponse(Response resp) {
if (resp.isFailed()) {
if (InvocationException.class.isInstance(resp.getResult())) {
InvocationException e = (InvocationException) resp.getResult();
- return e.getStatusCode() ==
ExceptionFactory.CONSUMER_INNER_STATUS_CODE;
+ return e.getStatusCode() == ExceptionFactory.CONSUMER_INNER_STATUS_CODE
+ || e.getStatusCode() == 503;
} else {
return true;
}
diff --git
a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestExtensionsManager.java
b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestExtensionsManager.java
index 539052d88..c0070cabb 100644
---
a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestExtensionsManager.java
+++
b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestExtensionsManager.java
@@ -16,6 +16,7 @@
*/
package org.apache.servicecomb.loadbalance;
+import java.io.IOException;
import java.net.ConnectException;
import java.net.SocketTimeoutException;
import java.util.ArrayList;
@@ -117,9 +118,11 @@ public void testRuleClassName() {
Assert.assertTrue(DefaultLoadBalancerRetryHandler.class.isInstance(retryHandler));
Assert.assertFalse(retryHandler.isRetriableException(new
InvocationException(400, "", ""), false));
Assert.assertFalse(retryHandler.isRetriableException(new
InvocationException(400, "", ""), true));
+ Assert.assertTrue(retryHandler.isRetriableException(new
InvocationException(503, "", ""), true));
Assert.assertTrue(retryHandler.isRetriableException(new
ConnectException(), false));
Assert.assertTrue(retryHandler.isRetriableException(new
ConnectException(), true));
Assert.assertTrue(retryHandler.isRetriableException(new
SocketTimeoutException(), false));
Assert.assertTrue(retryHandler.isRetriableException(new
SocketTimeoutException(), true));
+ Assert.assertTrue(retryHandler.isRetriableException(new IOException(),
true));
}
}
diff --git
a/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerEnvironment.java
b/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerEnvironment.java
index 42852c17d..8a5aa2d08 100644
---
a/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerEnvironment.java
+++
b/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerEnvironment.java
@@ -171,9 +171,9 @@ public SwaggerProducer createProducer(Object
producerInstance, Class<?> swaggerI
Method producerMethod = visibleProducerMethods.getOrDefault(methodName,
null);
if (producerMethod == null) {
// producer未实现契约，非法
- String msg = String.format("swagger method %s:%s not exist in
producer.",
- swaggerIntf.getClass().getName(),
- methodName);
+ String msg = String.format("swagger method %s not exist in producer
%s.",
+ methodName,
+ producerInstance.getClass().getName());
throw new Error(msg);
}
----------------------------------------------------------------
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:
[email protected]
> Provide a retry mechanism to meet upgrade no interrupt
> ------------------------------------------------------
>
> Key: SCB-833
> URL: https://issues.apache.org/jira/browse/SCB-833
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Java-Chassis
> Reporter: liubao
> Assignee: liubao
> Priority: Major
> Fix For: java-chassis-1.1.0
>
>
> see https://bbs.huaweicloud.com/blogs/72a312f09c8911e89fc57ca23e93a89f
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
