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