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

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

liubao68 closed pull request #889: [SCB-888] avoid switch SCBEngine status to 
up in the wrong time
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/889
 
 
   

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 dcd877f40..863d839fe 100644
--- a/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
+++ b/core/src/main/java/org/apache/servicecomb/core/SCBEngine.java
@@ -21,6 +21,7 @@
 import java.util.Comparator;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicLong;
 
 import javax.ws.rs.core.Response.Status;
@@ -51,11 +52,16 @@
 import com.google.common.eventbus.AllowConcurrentEvents;
 import com.google.common.eventbus.EventBus;
 import com.google.common.eventbus.Subscribe;
+import com.netflix.config.DynamicPropertyFactory;
 
 // TODO: should not depend on spring, that will make integration more flexible
 public class SCBEngine {
   private static final Logger LOGGER = 
LoggerFactory.getLogger(SCBEngine.class);
 
+  static final String CFG_KEY_WAIT_UP_TIMEOUT = 
"servicecomb.boot.waitUp.timeoutInMilliseconds";
+
+  static final long DEFAULT_WAIT_UP_TIMEOUT = 10_000;
+
   private ProducerProviderManager producerProviderManager;
 
   private ConsumerProviderManager consumerProviderManager;
@@ -192,8 +198,10 @@ public synchronized void init() {
     if (SCBStatus.DOWN.equals(status)) {
       try {
         doInit();
-        status = SCBStatus.UP;
-      } catch (Exception e) {
+        waitStatusUp();
+      } catch (TimeoutException e) {
+        LOGGER.warn("{}", e.getMessage());
+      } catch (Throwable e) {
         destroy();
         status = SCBStatus.FAILED;
         throw new IllegalStateException("ServiceComb init failed.", e);
@@ -201,7 +209,6 @@ public synchronized void init() {
     }
   }
 
-
   private void doInit() throws Exception {
     status = SCBStatus.STARTING;
 
@@ -242,7 +249,7 @@ private void doInit() throws Exception {
    * even some step throw exception, must catch it and go on, otherwise 
shutdown process will be broken.
    */
   public synchronized void destroy() {
-    if (SCBStatus.UP.equals(status)) {
+    if (SCBStatus.UP.equals(status) || SCBStatus.STARTING.equals(status)) {
       LOGGER.info("ServiceComb is closing now...");
       doDestroy();
       status = SCBStatus.DOWN;
@@ -318,4 +325,47 @@ public MicroserviceMeta getProducerMicroserviceMeta() {
   public void setProducerMicroserviceMeta(MicroserviceMeta 
producerMicroserviceMeta) {
     this.producerMicroserviceMeta = producerMicroserviceMeta;
   }
+
+  /**
+   * better to subscribe EventType.AFTER_REGISTRY by BootListener<br>
+   * but in some simple scenes, just block and wait is enough.
+   */
+  public void waitStatusUp() throws InterruptedException, TimeoutException {
+    long msWait = 
DynamicPropertyFactory.getInstance().getLongProperty(CFG_KEY_WAIT_UP_TIMEOUT, 
DEFAULT_WAIT_UP_TIMEOUT)
+        .get();
+    waitStatusUp(msWait);
+  }
+
+  /**
+   * better to subscribe EventType.AFTER_REGISTRY by BootListener<br>
+   * but in some simple scenes, just block and wait is enough.
+   */
+  public void waitStatusUp(long msWait) throws InterruptedException, 
TimeoutException {
+    if (msWait <= 0) {
+      LOGGER.info("Give up waiting for status up, wait timeout 
milliseconds={}.", msWait);
+      return;
+    }
+
+    LOGGER.info("Waiting for status up. timeout: {}ms", msWait);
+    long start = System.currentTimeMillis();
+    for (; ; ) {
+      SCBStatus currentStatus = getStatus();
+      switch (currentStatus) {
+        case DOWN:
+        case FAILED:
+          throw new IllegalStateException("Failed to wait status up, real 
status: " + currentStatus);
+        case UP:
+          LOGGER.info("Status already changed to up.");
+          return;
+        default:
+          break;
+      }
+
+      TimeUnit.MILLISECONDS.sleep(100);
+      if (System.currentTimeMillis() - start > msWait) {
+        throw new TimeoutException(
+            String.format("Timeout to wait status up, timeout: %dms, last 
status: %s", msWait, currentStatus));
+      }
+    }
+  }
 }
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 ea6371b46..fcc0e0f53 100644
--- a/core/src/test/java/org/apache/servicecomb/core/TestSCBEngine.java
+++ b/core/src/test/java/org/apache/servicecomb/core/TestSCBEngine.java
@@ -28,6 +28,7 @@
 import org.apache.servicecomb.core.provider.producer.ProducerProviderManager;
 import org.apache.servicecomb.core.transport.TransportManager;
 import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
+import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
 import org.apache.servicecomb.foundation.vertx.VertxUtils;
 import org.apache.servicecomb.serviceregistry.RegistryUtils;
 import org.apache.servicecomb.serviceregistry.consumer.AppManager;
@@ -81,9 +82,11 @@ void destroyConfigCenterConfigurationSource() {
     engine.setTransportManager(transportManager);
     engine.setSchemaListenerManager(schemaListenerManager);
 
+    ArchaiusUtils.setProperty(SCBEngine.CFG_KEY_WAIT_UP_TIMEOUT, 0);
     engine.init();
+    ArchaiusUtils.updateProperty(SCBEngine.CFG_KEY_WAIT_UP_TIMEOUT, null);
 
-    Assert.assertEquals(SCBStatus.UP, engine.getStatus());
+    Assert.assertEquals(SCBStatus.STARTING, engine.getStatus());
 
     engine.destroy();
 
diff --git 
a/demo/demo-pojo/pojo-tests/src/test/java/org/apache/servicecomb/demo/integration/PojoReferenceIntegrationTest.java
 
b/demo/demo-pojo/pojo-tests/src/test/java/org/apache/servicecomb/demo/integration/PojoReferenceIntegrationTest.java
index 0a3c4b25e..9706e33c0 100644
--- 
a/demo/demo-pojo/pojo-tests/src/test/java/org/apache/servicecomb/demo/integration/PojoReferenceIntegrationTest.java
+++ 
b/demo/demo-pojo/pojo-tests/src/test/java/org/apache/servicecomb/demo/integration/PojoReferenceIntegrationTest.java
@@ -17,6 +17,7 @@
 
 package org.apache.servicecomb.demo.integration;
 
+import static 
org.apache.servicecomb.serviceregistry.client.LocalServiceRegistryClientImpl.LOCAL_REGISTRY_FILE_KEY;
 import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertThat;
 
@@ -31,6 +32,7 @@
 
   @BeforeClass
   public static void setUp() throws Exception {
+    System.setProperty(LOCAL_REGISTRY_FILE_KEY, "notExistJustForceLocal");
     SomePojoTestMain.main(new String[0]);
   }
 
diff --git 
a/integration-tests/jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/RawJaxrsIntegrationTest.java
 
b/integration-tests/jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/RawJaxrsIntegrationTest.java
index 179f2e8d4..74fb18e08 100644
--- 
a/integration-tests/jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/RawJaxrsIntegrationTest.java
+++ 
b/integration-tests/jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/RawJaxrsIntegrationTest.java
@@ -17,12 +17,15 @@
 
 package org.apache.servicecomb.demo.jaxrs.tests;
 
+import static 
org.apache.servicecomb.serviceregistry.client.LocalServiceRegistryClientImpl.LOCAL_REGISTRY_FILE_KEY;
+
 import org.junit.BeforeClass;
 
 public class RawJaxrsIntegrationTest extends JaxrsIntegrationTestBase {
 
   @BeforeClass
   public static void setUp() throws Exception {
+    System.setProperty(LOCAL_REGISTRY_FILE_KEY, "notExistJustForceLocal");
     JaxrsTestMain.main(new String[0]);
   }
 }
diff --git 
a/integration-tests/spring-jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/JaxrsSpringIntegrationTest.java
 
b/integration-tests/spring-jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/JaxrsSpringIntegrationTest.java
index 8bb37bd86..834dff515 100644
--- 
a/integration-tests/spring-jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/JaxrsSpringIntegrationTest.java
+++ 
b/integration-tests/spring-jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/JaxrsSpringIntegrationTest.java
@@ -17,6 +17,7 @@
 
 package org.apache.servicecomb.demo.jaxrs.tests;
 
+import static 
org.apache.servicecomb.serviceregistry.client.LocalServiceRegistryClientImpl.LOCAL_REGISTRY_FILE_KEY;
 import static org.junit.Assert.assertEquals;
 
 import org.junit.AfterClass;
@@ -34,6 +35,7 @@
   @BeforeClass
   public static void setUp() {
     System.setProperty("property.test5", "from_system_property");
+    System.setProperty(LOCAL_REGISTRY_FILE_KEY, "notExistJustForceLocal");
   }
 
   @AfterClass
diff --git 
a/integration-tests/spring-jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/JaxrsSpringMain.java
 
b/integration-tests/spring-jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/JaxrsSpringMain.java
index 0eeaf7e16..3917fdde5 100644
--- 
a/integration-tests/spring-jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/JaxrsSpringMain.java
+++ 
b/integration-tests/spring-jaxrs-tests/src/test/java/org/apache/servicecomb/demo/jaxrs/tests/JaxrsSpringMain.java
@@ -17,6 +17,8 @@
 
 package org.apache.servicecomb.demo.jaxrs.tests;
 
+import static 
org.apache.servicecomb.serviceregistry.client.LocalServiceRegistryClientImpl.LOCAL_REGISTRY_FILE_KEY;
+
 import org.apache.servicecomb.springboot.starter.provider.EnableServiceComb;
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
@@ -25,6 +27,7 @@
 @EnableServiceComb
 public class JaxrsSpringMain {
   public static void main(final String[] args) throws Exception {
+    System.setProperty(LOCAL_REGISTRY_FILE_KEY, "notExistJustForceLocal");
     SpringApplication.run(JaxrsSpringMain.class, args);
   }
 }


 

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


> switch SCBEngine status to up in the wrong time
> -----------------------------------------------
>
>                 Key: SCB-888
>                 URL: https://issues.apache.org/jira/browse/SCB-888
>             Project: Apache ServiceComb
>          Issue Type: Bug
>          Components: Java-Chassis
>            Reporter: wujimin
>            Assignee: wujimin
>            Priority: Major
>             Fix For: java-chassis-1.1.0
>
>




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

Reply via email to