Submit a feature that applies graceful up and down #4670

chenshouye168 · 2025-01-06T06:21:29Z

Modify the zookeeper and service-center registry code to be able to modify the state to support graceful application up and down.

liubao68 · 2025-01-06T06:25:53Z

core/src/main/java/org/apache/servicecomb/core/SCBEngine.java

    discoveryManager.run();
    // ensure can invoke services in AFTER_REGISTRY
-    registrationManager.updateMicroserviceInstanceStatus(MicroserviceInstanceStatus.UP);
+//    registrationManager.updateMicroserviceInstanceStatus(MicroserviceInstanceStatus.UP);


Do not comment this statement. Add a configuration to support not update status to UP when service is registered.

Checked the code, only service-center implements this method, the other registry just return true, the initialization status to properties/yml inside, the default is true, the registration status is also true, I think it will not affect the compatibility of this line of code to remove!

OK. Delete this line.

liubao68 · 2025-01-06T07:00:55Z

...ions/foundation-registry/src/main/java/org/apache/servicecomb/registry/DiscoveryManager.java

+      if (instance.getHistoryStatus() == HistoryStatus.CURRENT
+              && instance.getMicroserviceInstanceStatus() == MicroserviceInstanceStatus.UP) {
        result.add(instance);
        continue;
      }


Non UP status should added to list. InstanceStatusDiscoveryFilter will handle status priority.

I've removed this filter.

liubao68 · 2025-01-06T07:04:17Z

...istry/src/main/java/org/apache/servicecomb/registry/discovery/StatefulDiscoveryInstance.java

+    this.microserviceInstanceStatus = discoveryInstance.getStatus();
  }

  public MicroserviceInstanceStatus getMicroserviceInstanceStatus() {


Maybe we can delete method getMicroserviceInstanceStatus and use getStatus for all appearances.

yes, the 'getMicroserviceInstanceStatus' method has been added to the MicroserviceInstance class.,as getStatus().

liubao68 · 2025-01-06T07:06:35Z

...try-service-center/src/main/java/org/apache/servicecomb/registry/sc/MicroserviceHandler.java

+    if (scConfigurationProperties.isEnableElegantUpDown()){
+      microserviceInstance.setStatus(MicroserviceInstanceStatus.STARTING);
+    }else {
+      microserviceInstance.setStatus(MicroserviceInstanceStatus.UP);
+    }


Maybe do not need this modification. Users can configure initial status to STARTING when need elegant up down.

don`t need it ,have removed

liubao68 · 2025-01-06T07:07:03Z

...rvice-center/src/main/java/org/apache/servicecomb/registry/sc/SCConfigurationProperties.java

+  public boolean isEnableElegantUpDown() {
+    return enableElegantUpDown;
+  }
+
+  public void setEnableElegantUpDown(boolean enableElegantUpDown) {
+    this.enableElegantUpDown = enableElegantUpDown;
+  }


See above, maybe do not need this modification.

don`t need it ,have removed

liubao68 · 2025-01-06T07:08:39Z

...try-service-center/src/main/java/org/apache/servicecomb/registry/sc/SCDiscoveryInstance.java

+    org.apache.servicecomb.service.center.client.model.DataCenterInfo dataCenterInfo = microserviceInstance.getDataCenterInfo();
+    if (dataCenterInfo != null) {
+      return new DataCenterInfo(microserviceInstance.getDataCenterInfo().getName(),
+              microserviceInstance.getDataCenterInfo().getRegion(),
+              microserviceInstance.getDataCenterInfo().getAvailableZone());
+    }
+    return new DataCenterInfo();


When DataCenterInfo is null? Can we add a fix to register DataCenterInfo?

don`t need it ,have removed, Setting the initial value on DataCenterProperties

liubao68 · 2025-01-06T07:09:06Z

...registry-service-center/src/main/java/org/apache/servicecomb/registry/sc/SCRegistration.java

+  public MicroserviceInstance getLatestMicroserviceInstance() {
+    MicroserviceInstance latestInstance = serviceCenterClient.getMicroserviceInstance(microserviceInstance.getServiceId(),
+            microserviceInstance.getInstanceId());
+    microserviceInstance.setStatus(latestInstance.getStatus());
+    return latestInstance;
+  }


This method is never used.

This method is used to externally query the latest instance state, and when the warm-up function updates the instance state, only the instance state that is STARTING can be updated to UP.

liubao68 · 2025-01-06T07:17:30Z

...try-zookeeper/src/main/java/org/apache/servicecomb/registry/zookeeper/ZookeeperInstance.java

+  public MicroserviceInstanceStatus getStatus() {
+    return status;
+  }
+
+  public void setStatus(MicroserviceInstanceStatus status) {
+    this.status = status;


Maybe we can add getStatus to MicroserviceInstance and force all implementations provide status information.

yes, the 'getMicroserviceInstanceStatus' method has been added to the MicroserviceInstance class.,as getStatus().

liubao68 · 2025-01-06T07:18:55Z

...zookeeper/src/main/java/org/apache/servicecomb/registry/zookeeper/ZookeeperRegistration.java

+  public ZookeeperInstance getZookeeperInstance(){
+    try {
+      ServiceInstance<ZookeeperInstance> zkInstance = dis.queryForInstance(instance.getName(), instance.getId());
+      if (zkInstance != null){
+        return zkInstance.getPayload();
+      }
+    } catch (Exception e) {
+      throw new RuntimeException(e);
+    }
+    return null;
+  }


This method never used.

This method is used to externally query the latest instance state, and when the warm-up function updates the instance state, only the instance state that is STARTING can be updated to UP

don`t need it ,remove

liubao68 · 2025-01-06T07:19:49Z

...zookeeper/src/main/java/org/apache/servicecomb/registry/zookeeper/ZookeeperRegistration.java

+      this.instance.getPayload().setStatus(status);
+      try {
+          if (status == MicroserviceInstanceStatus.UP){
+              dis.updateService(instance);
+          }
+          if (status == MicroserviceInstanceStatus.DOWN){
+              dis.registerService(instance);
+          }
+      } catch (Exception e){
+          throw new IllegalStateException(e);
+      }
+


Why use registerService when Down?

has been changed to updateService()

liubao68 · 2025-01-06T07:21:14Z

...zookeeper/src/main/java/org/apache/servicecomb/registry/zookeeper/ZookeeperRegistration.java

+    }else {
+      zookeeperInstance.setStatus(MicroserviceInstanceStatus.UP);
+    }
    try {


Should use initialstatus, do not need isEnableElegantUpDown

don`t need it ,have removed

chengyouling · 2025-01-06T11:59:04Z

...try-service-center/src/main/java/org/apache/servicecomb/registry/sc/SCDiscoveryInstance.java

-        microserviceInstance.getDataCenterInfo().getRegion(),
-        microserviceInstance.getDataCenterInfo().getAvailableZone());
+    org.apache.servicecomb.service.center.client.model.DataCenterInfo dataCenterInfo = microserviceInstance.getDataCenterInfo();
+    if (dataCenterInfo != null) {


Maybe you can init DataCenterInfo when init MicroserviceInstance, so that not need this modification.

don`t need it ,have removed, Setting the initial value on DataCenterProperties

… and down functions.

liubao68 · 2025-01-09T09:06:29Z

...gistry-nacos/src/main/java/org/apache/servicecomb/registry/nacos/NacosDiscoveryInstance.java

  public MicroserviceInstanceStatus getStatus() {
-    return instance.isEnabled() ? MicroserviceInstanceStatus.UP : MicroserviceInstanceStatus.DOWN;
+    if (instance.isEnabled()){
+      return MicroserviceInstanceStatus.valueOf(instance.getMetadata().get(NacosConst.NACOS_STATUS));


instance.getMetadata().get(NacosConst.NACOS_STATUS maybe null for old versions instance.

The new version will definitely have status in metadata, so Returns up if status is not found in the old version in metadata.

liubao68 · 2025-01-09T09:07:18Z

...stry-nacos/src/main/java/org/apache/servicecomb/registry/nacos/NacosDiscoveryProperties.java

+  public String getInitialStatus() {
+    return initialStatus;
+  }
+
+  public void setInitialStatus(String initialStatus) {
+    this.initialStatus = initialStatus;
+  }
 }


Use common configration servicecomb.instance.initialStatus, see SC how to read it.

Nacos,ZK,ServiceCenter has used common configration servicecomb.instance.initialStatus

liubao68 · 2025-01-09T09:09:13Z

...ry/registry-nacos/src/main/java/org/apache/servicecomb/registry/nacos/NacosRegistration.java

+      if (MicroserviceInstanceStatus.UP == status){
+        instance.getMetadata().put(NacosConst.NACOS_STATUS, MicroserviceInstanceStatus.UP.name());
+        namingService.registerInstance(nacosRegistrationInstance.getServiceName(),
+                nacosRegistrationInstance.getApplication(), instance);
+      }
+      if (MicroserviceInstanceStatus.DOWN == status){
+        instance.getMetadata().put(NacosConst.NACOS_STATUS, MicroserviceInstanceStatus.DOWN.name());
+        namingService.deregisterInstance(nacosRegistrationInstance.getServiceName(),
+                nacosRegistrationInstance.getApplication(), instance);
+      }
+      return true;
+    } catch (Exception e) {
+      throw new RuntimeException(e);
+    }


How about update to other status? like TESTING?

ok, Instance status equal DOWN to exec 'deregisterInstance()' method, the other exec 'registerInstance()'

…ization time.

liubao68 · 2025-01-09T12:57:52Z

...ions/foundation-registry/src/main/java/org/apache/servicecomb/registry/api/Registration.java

 * This is the core service registration interface. <br/>
 */
-public interface Registration<R extends RegistrationInstance> extends SPIEnabled, SPIOrder, LifeCycle {
+public interface Registration<R extends MicroserviceInstance> extends SPIEnabled, SPIOrder, LifeCycle {


Why made this change?

don`r need method 'getnitialStatus()' and 'getReadyStatus()'

liubao68 · 2025-01-09T12:59:07Z

...gistry-nacos/src/main/java/org/apache/servicecomb/registry/nacos/NacosDiscoveryInstance.java

-    return instance.isEnabled() ? MicroserviceInstanceStatus.UP : MicroserviceInstanceStatus.DOWN;
+    if (instance.isEnabled()) {
+      String instanceStatus = instance.getMetadata().get(NacosConst.NACOS_STATUS);
+      if (StringUtils.isBlank(instanceStatus)) {


Should be isEmpty?

U said "instance.getMetadata().get(NacosConst.NACOS_STATUS maybe null for old versions instance." before,so i do it .

liubao68 · 2025-01-09T13:01:33Z

...ry/registry-nacos/src/main/java/org/apache/servicecomb/registry/nacos/NacosRegistration.java

+      if (MicroserviceInstanceStatus.DOWN == status) {
+        instance.getMetadata().put(NacosConst.NACOS_STATUS, MicroserviceInstanceStatus.DOWN.name());
+        namingService.deregisterInstance(nacosRegistrationInstance.getServiceName(),
+            nacosRegistrationInstance.getApplication(), instance);
+      } else {
+        instance.getMetadata().put(NacosConst.NACOS_STATUS, status.name());
+        namingService.registerInstance(nacosRegistrationInstance.getServiceName(),
+            nacosRegistrationInstance.getApplication(), instance);
+      }
+      return true;
+    } catch (Exception e) {
+      throw new RuntimeException(e);
+    }


Whatever status, update. No need to check status. Will registerInstance cause two instances? If Nacos do not support update status, maybe we can not use this feature for nacos.

Nacos register Instance again ,old instance will be covery. For Down, just deregister instance.

It's better only set status, do not deregister instance.

liubao68 · 2025-01-09T13:02:30Z

...try-service-center/src/main/java/org/apache/servicecomb/registry/sc/SCDiscoveryInstance.java

+      return new DataCenterInfo(microserviceInstance.getDataCenterInfo().getName(),
+              microserviceInstance.getDataCenterInfo().getRegion(),
+              microserviceInstance.getDataCenterInfo().getAvailableZone());


Seams nothing changed, but format is wrong .

it`s changed

…thod 'getInitialStatus()' and 'getReadyStatus()'

…rationInstance

… to replace customizable settings

chenshouye168 added 2 commits January 3, 2025 00:07

elegant up or down for service-center

83b1585

elegant up or down for registry-zookeeper

8d5c0bb

liubao68 reviewed Jan 6, 2025

View reviewed changes

chengyouling reviewed Jan 6, 2025

View reviewed changes

chenshouye168 added 3 commits January 9, 2025 15:43

Modify the code according to the review code and add nacos adapted up…

8c5d8e4

… and down functions.

Merge branch 'apache:master' into master

cae38de

remove 'getMicroserviceInstanceStatus' method,use getStatus()

ff52d42

liubao68 reviewed Jan 9, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master'

7ca6e02

liubao68 reviewed Jan 9, 2025

View reviewed changes

chenshouye168 added 7 commits January 9, 2025 17:52

don`t need this method to select instance status,delete it

004aa32

getInitialStatus(),getReadyStatus() not used,so delete it

a76ae7d

delete this line,because already read in the configuration at initial…

2d6e04b

…ization time.

getInitialStatus(),getReadyStatus() not used,so delete it

860b5d4

NacosInstanceStatus Add some conditional judgments

1352218

Restore the original state

c100c8a

update getMicroserviceInstanceStatus() to 'getStatus()'

fc48c8f

liubao68 reviewed Jan 9, 2025

View reviewed changes

chenshouye168 added 9 commits January 10, 2025 09:40

getInitialStatus(),getReadyStatus() not used,so don`t implement it

83339e8

getDataCenterInfo() NullPointException handle

95bb8bc

Setting ServiceInstance initialStatus

89b84d5

change RegistrationInstance to MicroserviceInstance,cuz don`t need me…

568c627

…thod 'getInitialStatus()' and 'getReadyStatus()'

Merge branch 'apache:master' into master

2a0801f

all registry center interface rollback MicroserviceInstance to Regist…

e70bcfb

…rationInstance

All states use the registerInstance method

2da0311

use BootStrapProperties.readServiceInstanceInitialStatus(environment)…

911429c

… to replace customizable settings

rat check

b4ecf04

liubao68 merged commit 0668f63 into apache:master Jan 13, 2025
6 checks passed

Submit a feature that applies graceful up and down #4670

Submit a feature that applies graceful up and down #4670

Uh oh!

Conversation

chenshouye168 commented Jan 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liubao68 Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liubao68 Jan 9, 2025 •

edited

Loading