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

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

WillemJiang closed pull request #769: [SCB-667] fix gracefully shutdown is not 
work in some case
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/769
 
 
   

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/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
 
b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
index 81e6fc925..cb4413b47 100644
--- 
a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
+++ 
b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/AbstractRestInvocation.java
@@ -108,7 +108,13 @@ public String getContext(String key) {
   }
 
   protected void scheduleInvocation() {
-    createInvocation();
+    try {
+      createInvocation();
+    } catch (IllegalStateException e) {
+      sendFailResponse(e);
+      return;
+    }
+
     invocation.onStart();
     OperationMeta operationMeta = restOperationMeta.getOperationMeta();
 
@@ -161,7 +167,6 @@ public void invoke() {
 
   protected Response prepareInvoke() throws Throwable {
     this.initProduceProcessor();
-
     this.setContext();
     invocation.getHandlerContext().put(RestConst.REST_REQUEST, requestEx);
 
diff --git 
a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
 
b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
index 6e5df508f..e8d4d7f08 100644
--- 
a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
+++ 
b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
@@ -36,6 +36,8 @@
 import org.apache.servicecomb.core.Endpoint;
 import org.apache.servicecomb.core.Handler;
 import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.core.SCBEngine;
+import org.apache.servicecomb.core.SCBStatus;
 import org.apache.servicecomb.core.definition.MicroserviceMeta;
 import org.apache.servicecomb.core.definition.MicroserviceMetaManager;
 import org.apache.servicecomb.core.definition.OperationMeta;
@@ -121,11 +123,13 @@ protected void createInvocation() {
   @BeforeClass
   public static void classSetup() {
     EventManager.eventBus = new EventBus();
+    SCBEngine.getInstance().setStatus(SCBStatus.UP);
   }
 
   @AfterClass
   public static void classTeardown() {
     EventManager.eventBus = new EventBus();
+    SCBEngine.getInstance().setStatus(SCBStatus.DOWN);
   }
 
   @Before
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 4d2f15702..aa42ded56 100644
--- a/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
+++ b/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
@@ -273,11 +273,11 @@ private void validAllInvocationFinished() throws 
InterruptedException {
     }
   }
 
-  protected void ensureStatusUp() {
+  public void ensureStatusUp() {
     SCBStatus currentStatus = getStatus();
     if (!SCBStatus.UP.equals(currentStatus)) {
       throw new IllegalStateException(
-          "System is starting and not ready for remote calls or shutting down 
in progress, STATUS = " + currentStatus);
+          "The request is rejected, as the service cannot process the request 
due to STATUS = " + currentStatus);
     }
   }
 
diff --git 
a/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationFactory.java
 
b/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationFactory.java
index 8410d935d..8c72f9355 100644
--- 
a/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationFactory.java
+++ 
b/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationFactory.java
@@ -20,6 +20,7 @@
 import org.apache.servicecomb.core.Const;
 import org.apache.servicecomb.core.Endpoint;
 import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.core.SCBEngine;
 import org.apache.servicecomb.core.definition.MicroserviceMeta;
 import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.core.definition.SchemaMeta;
@@ -68,6 +69,7 @@ public static Invocation forConsumer(ReferenceConfig 
referenceConfig, String ope
   public static Invocation forProvider(Endpoint endpoint,
       OperationMeta operationMeta,
       Object[] swaggerArguments) {
+    SCBEngine.getInstance().ensureStatusUp();
     return new Invocation(endpoint,
         operationMeta,
         swaggerArguments);
diff --git 
a/core/src/main/java/org/apache/servicecomb/core/unittest/UnitTestMeta.java 
b/core/src/main/java/org/apache/servicecomb/core/unittest/UnitTestMeta.java
index 832ea6436..fd1eece78 100644
--- a/core/src/main/java/org/apache/servicecomb/core/unittest/UnitTestMeta.java
+++ b/core/src/main/java/org/apache/servicecomb/core/unittest/UnitTestMeta.java
@@ -61,6 +61,10 @@
 
   private ConsumerProviderManager consumerProviderManager;
 
+  public ConsumerProviderManager getConsumerProviderManager() {
+    return consumerProviderManager;
+  }
+
   private ConsumerSchemaFactory consumerSchemaFactory;
 
   private SchemaLoader schemaLoader = new SchemaLoader() {
diff --git 
a/core/src/test/java/org/apache/servicecomb/core/TestInvocationFactory.java 
b/core/src/test/java/org/apache/servicecomb/core/TestInvocationFactory.java
index 7767ad1ad..b91cad1c1 100644
--- a/core/src/test/java/org/apache/servicecomb/core/TestInvocationFactory.java
+++ b/core/src/test/java/org/apache/servicecomb/core/TestInvocationFactory.java
@@ -36,6 +36,7 @@ public static void setUp() {
     ServiceRegistry serviceRegistry = ServiceRegistryFactory.createLocal();
     serviceRegistry.init();
     RegistryUtils.setServiceRegistry(serviceRegistry);
+    SCBEngine.getInstance().setStatus(SCBStatus.UP);
   }
 
   @Test
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 6b72f566b..89ffe806c 100644
--- a/core/src/test/java/org/apache/servicecomb/core/TestSCBEngine.java
+++ b/core/src/test/java/org/apache/servicecomb/core/TestSCBEngine.java
@@ -106,7 +106,7 @@ public void createReferenceConfigForInvoke_down(@Mocked 
ConsumerProviderManager
 
     expectedException.expect(IllegalStateException.class);
     expectedException.expectMessage(
-        Matchers.is("System is starting and not ready for remote calls or 
shutting down in progress, STATUS = DOWN"));
+        Matchers.is("The request is rejected, as the service cannot process 
the request due to STATUS = DOWN"));
     engine.createReferenceConfigForInvoke(null, null, null);
   }
 
@@ -128,7 +128,7 @@ public void getReferenceConfigForInvoke_down(@Mocked 
ConsumerProviderManager con
 
     expectedException.expect(IllegalStateException.class);
     expectedException.expectMessage(
-        Matchers.is("System is starting and not ready for remote calls or 
shutting down in progress, STATUS = DOWN"));
+        Matchers.is("The request is rejected, as the service 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 784a8a66c..ef0b021a5 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
@@ -126,7 +126,7 @@ public void testSyncInvoke_4param_NotReady() {
 
     expectedException.expect(IllegalStateException.class);
     expectedException.expectMessage(
-        Matchers.is("System is starting and not ready for remote calls or 
shutting down in progress, STATUS = DOWN"));
+        Matchers.is("The request is rejected, as the service cannot process 
the request due to STATUS = DOWN"));
     InvokerUtils.syncInvoke("ms", "schemaId", "opName", null);
   }
 
@@ -136,7 +136,7 @@ public void testSyncInvoke_6param_NotReady() {
 
     expectedException.expect(IllegalStateException.class);
     expectedException.expectMessage(
-        Matchers.is("System is starting and not ready for remote calls or 
shutting down in progress, STATUS = DOWN"));
+        Matchers.is("The request is rejected, as the service cannot process 
the request due to STATUS = DOWN"));
 
     InvokerUtils.syncInvoke("ms", "latest", "rest", "schemaId", "opName", 
null);
   }
diff --git 
a/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/Invoker.java
 
b/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/Invoker.java
index d4d279c34..05cd2fd2c 100644
--- 
a/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/Invoker.java
+++ 
b/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/Invoker.java
@@ -57,6 +57,7 @@ public InvokerMeta(ReferenceConfig referenceConfig,
 
     // initialized and meta not changed
     public boolean isValid() {
+      SCBEngine.getInstance().ensureStatusUp();
       return swaggerConsumer != null && microserviceMeta == 
referenceConfig.getMicroserviceMeta();
     }
   }
diff --git 
a/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CseClientHttpRequest.java
 
b/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CseClientHttpRequest.java
index 1e85f5a39..ec9add3f1 100644
--- 
a/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CseClientHttpRequest.java
+++ 
b/providers/provider-springmvc/src/main/java/org/apache/servicecomb/provider/springmvc/reference/CseClientHttpRequest.java
@@ -29,8 +29,8 @@
 import org.apache.servicecomb.common.rest.definition.RestOperationMeta;
 import org.apache.servicecomb.common.rest.locator.OperationLocator;
 import org.apache.servicecomb.common.rest.locator.ServicePathManager;
-import org.apache.servicecomb.core.CseContext;
 import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.core.SCBEngine;
 import org.apache.servicecomb.core.definition.MicroserviceMeta;
 import org.apache.servicecomb.core.invocation.InvocationFactory;
 import org.apache.servicecomb.core.provider.consumer.InvokerUtils;
@@ -149,8 +149,7 @@ public ClientHttpResponse execute() {
   protected RequestMeta createRequestMeta(String httpMethod, URI uri) {
     String microserviceName = uri.getAuthority();
 
-    ReferenceConfig referenceConfig = 
CseContext.getInstance().getConsumerProviderManager()
-        .getReferenceConfig(microserviceName);
+    ReferenceConfig referenceConfig = 
SCBEngine.getInstance().getReferenceConfigForInvoke(microserviceName);
 
     MicroserviceMeta microserviceMeta = referenceConfig.getMicroserviceMeta();
     ServicePathManager servicePathManager = 
ServicePathManager.getServicePathManager(microserviceMeta);
diff --git 
a/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/TestCseClientHttpRequest.java
 
b/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/TestCseClientHttpRequest.java
index de8935dba..aadf55f60 100644
--- 
a/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/TestCseClientHttpRequest.java
+++ 
b/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/TestCseClientHttpRequest.java
@@ -37,21 +37,17 @@
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 
-import mockit.Mock;
-import mockit.MockUp;
-
 public class TestCseClientHttpRequest {
-  SCBEngine scbEngine = new SCBEngine();
+  static UnitTestMeta meta = new UnitTestMeta();
 
   @Before
   public void setup() {
-    new MockUp<SCBEngine>() {
-      @Mock
-      SCBEngine getInstance() {
-        return scbEngine;
-      }
-    };
-    scbEngine.setStatus(SCBStatus.UP);
+    SCBEngine.getInstance().setStatus(SCBStatus.UP);
+    CseContext.getInstance()
+        .getSchemaListenerManager()
+        .setSchemaListenerList(Collections.singletonList(new 
RestEngineSchemaListener()));
+    meta.registerSchema(new SpringmvcSwaggerGeneratorContext(), 
SpringmvcImpl.class);
+    
SCBEngine.getInstance().setConsumerProviderManager(meta.getConsumerProviderManager());
   }
 
   @RequestMapping(path = "SpringmvcImpl")
diff --git 
a/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/async/CseAsyncClientHttpRequestTest.java
 
b/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/async/CseAsyncClientHttpRequestTest.java
index b0e3af909..d45dc37bf 100644
--- 
a/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/async/CseAsyncClientHttpRequestTest.java
+++ 
b/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/async/CseAsyncClientHttpRequestTest.java
@@ -33,7 +33,6 @@
 import 
org.apache.servicecomb.swagger.generator.springmvc.SpringmvcSwaggerGeneratorContext;
 import org.apache.servicecomb.swagger.invocation.Response;
 import org.junit.Assert;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.springframework.http.HttpMethod;
@@ -44,32 +43,17 @@
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 
-import mockit.Mock;
-import mockit.MockUp;
-
 public class CseAsyncClientHttpRequestTest {
   static UnitTestMeta meta = new UnitTestMeta();
 
-  SCBEngine scbEngine = new SCBEngine();
-
-  @Before
-  public void setup() {
-    new MockUp<SCBEngine>() {
-      @Mock
-      SCBEngine getInstance() {
-        return scbEngine;
-      }
-    };
-    scbEngine.setStatus(SCBStatus.UP);
-  }
-
   @BeforeClass
   public static void classSetup() {
     CseContext.getInstance()
         .getSchemaListenerManager()
         .setSchemaListenerList(Collections.singletonList(new 
RestEngineSchemaListener()));
-
     meta.registerSchema(new SpringmvcSwaggerGeneratorContext(), 
CseAsyncClientHttpRequestTestSchema.class);
+    
SCBEngine.getInstance().setConsumerProviderManager(meta.getConsumerProviderManager());
+    SCBEngine.getInstance().setStatus(SCBStatus.UP);
   }
 
   @RequestMapping(path = "CseAsyncClientHttpRequestTestSchema")
diff --git 
a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerInvoke.java
 
b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerInvoke.java
index 3dd600605..fd82cca14 100644
--- 
a/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerInvoke.java
+++ 
b/transports/transport-highway/src/main/java/org/apache/servicecomb/transport/highway/HighwayServerInvoke.java
@@ -158,7 +158,9 @@ private void sendResponse(Map<String, String> context, 
Response response) {
           msgId);
       LOGGER.error(msg, e);
     } finally {
-      invocation.onFinish(response);
+      if (invocation != null) {
+        invocation.onFinish(response);
+      }
     }
   }
 
@@ -166,10 +168,14 @@ private void sendResponse(Map<String, String> context, 
Response response) {
    * start time in queue.
    */
   public void execute() {
-    invocation = InvocationFactory.forProvider(endpoint,
-        operationProtobuf.getOperationMeta(),
-        null);
-    invocation.onStart();
-    operationMeta.getExecutor().execute(() -> runInExecutor());
+    try {
+      invocation = InvocationFactory.forProvider(endpoint,
+          operationProtobuf.getOperationMeta(),
+          null);
+      invocation.onStart();
+      operationMeta.getExecutor().execute(() -> runInExecutor());
+    } catch (IllegalStateException e) {
+      sendResponse(header.getContext(), Response.providerFailResp(e));
+    }
   }
 }
diff --git 
a/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayServerInvoke.java
 
b/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayServerInvoke.java
index 0dc902c81..525b5e0d2 100644
--- 
a/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayServerInvoke.java
+++ 
b/transports/transport-highway/src/test/java/org/apache/servicecomb/transport/highway/TestHighwayServerInvoke.java
@@ -19,6 +19,8 @@
 
 import javax.xml.ws.Holder;
 
+import org.apache.servicecomb.core.SCBEngine;
+import org.apache.servicecomb.core.SCBStatus;
 import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.core.definition.SchemaMeta;
 import org.apache.servicecomb.core.event.InvocationFinishEvent;
@@ -65,11 +67,13 @@ public int add(int x, int y) {
   @BeforeClass
   public static void classSetup() {
     EventManager.eventBus = new EventBus();
+    SCBEngine.getInstance().setStatus(SCBStatus.UP);
   }
 
   @AfterClass
   public static void classTeardown() {
     EventManager.eventBus = new EventBus();
+    SCBEngine.getInstance().setStatus(SCBStatus.DOWN);
   }
 
   @Before


 

----------------------------------------------------------------
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


> gracefully shutdown is not work in some case
> --------------------------------------------
>
>                 Key: SCB-667
>                 URL: https://issues.apache.org/jira/browse/SCB-667
>             Project: Apache ServiceComb
>          Issue Type: Bug
>          Components: Java-Chassis
>    Affects Versions: java-chassis-1.0.0-m1
>            Reporter: yangyongzheng
>            Assignee: yangyongzheng
>            Priority: Major
>             Fix For: java-chassis-1.0.0-m2
>
>
> In some case like consumer never stop sending request to prodiver, privider 
> do not reject new request from consumer and still process them,
> So shutdown will be blocked.
>  



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

Reply via email to