Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
83b1585
elegant up or down for service-center
chenshouye168 Jan 2, 2025
8d5c0bb
elegant up or down for registry-zookeeper
chenshouye168 Jan 5, 2025
8c5d8e4
Modify the code according to the review code and add nacos adapted up…
chenshouye168 Jan 9, 2025
cae38de
Merge branch 'apache:master' into master
chenshouye168 Jan 9, 2025
ff52d42
remove 'getMicroserviceInstanceStatus' method,use getStatus()
chenshouye168 Jan 9, 2025
7ca6e02
Merge remote-tracking branch 'origin/master'
chenshouye168 Jan 9, 2025
004aa32
don`t need this method to select instance status,delete it
chenshouye168 Jan 9, 2025
a76ae7d
getInitialStatus(),getReadyStatus() not used,so delete it
chenshouye168 Jan 9, 2025
2d6e04b
delete this line,because already read in the configuration at initial…
chenshouye168 Jan 9, 2025
860b5d4
getInitialStatus(),getReadyStatus() not used,so delete it
chenshouye168 Jan 9, 2025
1352218
NacosInstanceStatus Add some conditional judgments
chenshouye168 Jan 9, 2025
c100c8a
Restore the original state
chenshouye168 Jan 9, 2025
fc48c8f
update getMicroserviceInstanceStatus() to 'getStatus()'
chenshouye168 Jan 9, 2025
83339e8
getInitialStatus(),getReadyStatus() not used,so don`t implement it
chenshouye168 Jan 10, 2025
95bb8bc
getDataCenterInfo() NullPointException handle
chenshouye168 Jan 10, 2025
89b84d5
Setting ServiceInstance initialStatus
chenshouye168 Jan 10, 2025
568c627
change RegistrationInstance to MicroserviceInstance,cuz don`t need me…
chenshouye168 Jan 10, 2025
2a0801f
Merge branch 'apache:master' into master
chenshouye168 Jan 10, 2025
e70bcfb
all registry center interface rollback MicroserviceInstance to Regist…
chenshouye168 Jan 10, 2025
2da0311
All states use the registerInstance method
chenshouye168 Jan 10, 2025
911429c
use BootStrapProperties.readServiceInstanceInitialStatus(environment)…
chenshouye168 Jan 10, 2025
b4ecf04
rat check
chenshouye168 Jan 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ private void doRun() throws Exception {
registrationManager.run();
discoveryManager.run();
// ensure can invoke services in AFTER_REGISTRY
registrationManager.updateMicroserviceInstanceStatus(MicroserviceInstanceStatus.UP);
// registrationManager.updateMicroserviceInstanceStatus(MicroserviceInstanceStatus.UP);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Delete this line.

status = SCBStatus.UP;
triggerEvent(EventType.AFTER_REGISTRY);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@
public class DataCenterProperties {
public static final String PREFIX = "servicecomb.datacenter";

private String name;
private String name = "local";

private String region;
private String region = "local";

private String availableZone;
private String availableZone = "local";

public String getName() {
return name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ private VersionedCache calcAvailableInstance(String application, String serviceN
-> new ConcurrentHashMapEx<>());
List<StatefulDiscoveryInstance> result = new ArrayList<>();
for (StatefulDiscoveryInstance instance : statefulInstances.values()) {
if (instance.getHistoryStatus() == HistoryStatus.CURRENT) {
if (instance.getHistoryStatus() == HistoryStatus.CURRENT
&& instance.getMicroserviceInstanceStatus() == MicroserviceInstanceStatus.UP) {
result.add(instance);
continue;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed this filter.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,10 @@ public interface MicroserviceInstance {
default String getServiceId() {
return "";
}

/**
* Service status(Required): status of this microservice.
* @return
*/
public MicroserviceInstanceStatus getStatus();
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public enum HistoryStatus {

public StatefulDiscoveryInstance(DiscoveryInstance discoveryInstance) {
this.discoveryInstance = discoveryInstance;
this.microserviceInstanceStatus = discoveryInstance.getStatus();
}

public MicroserviceInstanceStatus getMicroserviceInstanceStatus() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import org.apache.servicecomb.registry.api.DataCenterInfo;
import org.apache.servicecomb.registry.api.MicroserviceInstance;
import org.apache.servicecomb.registry.api.MicroserviceInstanceStatus;

public class EtcdInstance implements MicroserviceInstance {
private String serviceId;
Expand Down Expand Up @@ -188,6 +189,11 @@ public String getServiceId() {
return serviceId;
}

@Override
public MicroserviceInstanceStatus getStatus() {
return MicroserviceInstanceStatus.UP;
}

@Override
public String toString() {
return "EtcdInstance{" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ public String getInstanceId() {
return this.instanceId;
}

@Override
public MicroserviceInstanceStatus getStatus() {
return MicroserviceInstanceStatus.UP;
}

@Override
public MicroserviceInstanceStatus getInitialStatus() {
return MicroserviceInstanceStatus.STARTING;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,6 @@ public class NacosConst {
public static final String NAMING_LOAD_CACHE_AT_START = "namingLoadCacheAtStart";

public static final String NACOS_REGISTRY_NAME = "nacos-registry";

public static final String NACOS_STATUS= "nacos-status";
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ public NacosDiscoveryInstance(Instance instance, String application, String serv

@Override
public MicroserviceInstanceStatus getStatus() {
return instance.isEnabled() ? MicroserviceInstanceStatus.UP : MicroserviceInstanceStatus.DOWN;
if (instance.isEnabled()){
return MicroserviceInstanceStatus.valueOf(instance.getMetadata().get(NacosConst.NACOS_STATUS));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} else {
return MicroserviceInstanceStatus.DOWN;
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public class NacosDiscoveryProperties {

private boolean enableSwaggerRegistration = false;

private String initialStatus = "UP";

public String getServerAddr() {
return serverAddr;
}
Expand Down Expand Up @@ -160,4 +162,12 @@ public boolean isEnableSwaggerRegistration() {
public void setEnableSwaggerRegistration(boolean enableSwaggerRegistration) {
this.enableSwaggerRegistration = enableSwaggerRegistration;
}

public String getInitialStatus() {
return initialStatus;
}

public void setInitialStatus(String initialStatus) {
this.initialStatus = initialStatus;
}
}
Copy link
Contributor

@liubao68 liubao68 Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public static Instance createMicroserviceInstance(
metadata.put(NacosConst.PROPERTY_DATACENTER, dataCenterProperties.getName());
metadata.put(NacosConst.PROPERTY_REGION, dataCenterProperties.getRegion());
metadata.put(NacosConst.PROPERTY_ZONE, dataCenterProperties.getAvailableZone());
metadata.put(NacosConst.NACOS_STATUS, properties.getInitialStatus());
if (!StringUtils.isEmpty(environment.getProperty(VERSION_MAPPING)) &&
!StringUtils.isEmpty(environment.getProperty(environment.getProperty(VERSION_MAPPING)))) {
metadata.put("version", environment.getProperty(environment.getProperty(VERSION_MAPPING)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,21 @@ public NacosRegistrationInstance getMicroserviceInstance() {

@Override
public boolean updateMicroserviceInstanceStatus(MicroserviceInstanceStatus status) {
// Do not support Nacos update status now. Because update status will fail
// due to some unknown reasons(Maybe different constrains in register and maintain api).
return true;
try {
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);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about update to other status? like TESTING?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ public String getInstanceId() {
return instance.getInstanceId();
}

@Override
public MicroserviceInstanceStatus getStatus() {
return MicroserviceInstanceStatus.valueOf(instance.getMetadata().get(NacosConst.NACOS_STATUS));
}

@Override
public MicroserviceInstanceStatus getInitialStatus() {
return MicroserviceInstanceStatus.STARTING;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ public static MicroserviceInstance createMicroserviceInstance(
}
microserviceInstance.setHostName(hostName);

DataCenterInfo dataCenterInfo = new DataCenterInfo();
if (StringUtils.isNotEmpty(dataCenterProperties.getName())) {
DataCenterInfo dataCenterInfo = new DataCenterInfo();
dataCenterInfo.setName(dataCenterProperties.getName());
dataCenterInfo.setRegion(dataCenterProperties.getRegion());
dataCenterInfo.setAvailableZone(dataCenterProperties.getAvailableZone());
microserviceInstance.setDataCenterInfo(dataCenterInfo);
}
microserviceInstance.setDataCenterInfo(dataCenterInfo);

HealthCheck healthCheck = new HealthCheck();
healthCheck.setMode(HealthCheckMode.push);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class SCConfigurationProperties {

private boolean autoDiscovery = false;

private String initialStatus = "STARTING";
private String initialStatus = "UP";

private boolean watch = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ public String getVersion() {

@Override
public DataCenterInfo getDataCenterInfo() {
return new DataCenterInfo(microserviceInstance.getDataCenterInfo().getName(),
microserviceInstance.getDataCenterInfo().getRegion(),
microserviceInstance.getDataCenterInfo().getAvailableZone());
return new DataCenterInfo(microserviceInstance.getDataCenterInfo().getName(),
microserviceInstance.getDataCenterInfo().getRegion(),
microserviceInstance.getDataCenterInfo().getAvailableZone());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seams nothing changed, but format is wrong .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it`s changed

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,15 @@ public Microservice getBackendMicroservice() {
public MicroserviceInstance getBackendMicroserviceInstance() {
return microserviceInstance;
}

/**
* Returns the latest instance for checking when updating the instance status from STARTING to UP.
* @return MicroserviceInstance
*/
public MicroserviceInstance getLatestMicroserviceInstance() {
MicroserviceInstance latestInstance = serviceCenterClient.getMicroserviceInstance(microserviceInstance.getServiceId(),
microserviceInstance.getInstanceId());
microserviceInstance.setStatus(latestInstance.getStatus());
return latestInstance;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is never used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ public class SCRegistrationInstance implements RegistrationInstance {

private final ServiceCenterRegistration serviceCenterRegistration;

public SCRegistrationInstance(Microservice microservice,
MicroserviceInstance microserviceInstance,
public SCRegistrationInstance(Microservice microservice, MicroserviceInstance microserviceInstance,
ServiceCenterRegistration serviceCenterRegistration) {
this.microservice = microservice;
this.microserviceInstance = microserviceInstance;
Expand Down Expand Up @@ -106,6 +105,11 @@ public String getServiceId() {
return microservice.getServiceId();
}

@Override
public MicroserviceInstanceStatus getStatus() {
return MicroserviceInstanceStatus.valueOf(microserviceInstance.getStatus().name());
}

@Override
public MicroserviceInstanceStatus getInitialStatus() {
return MicroserviceInstanceStatus.valueOf(microserviceInstance.getStatus().name());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ public String getInstanceId() {
return this.self.getInstanceId();
}

@Override
public MicroserviceInstanceStatus getStatus() {
return MicroserviceInstanceStatus.UP;
}

@Override
public MicroserviceInstanceStatus getInitialStatus() {
return MicroserviceInstanceStatus.STARTING;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@ public ZookeeperDiscoveryInstance(ZookeeperInstance other) {
super(other);
}

@Override
public MicroserviceInstanceStatus getStatus() {
return MicroserviceInstanceStatus.UP;
}

@Override
public String getRegistryName() {
return ZookeeperConst.ZOOKEEPER_REGISTRY_NAME;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import org.apache.servicecomb.registry.api.DataCenterInfo;
import org.apache.servicecomb.registry.api.MicroserviceInstance;
import org.apache.servicecomb.registry.api.MicroserviceInstanceStatus;

public class ZookeeperInstance implements MicroserviceInstance {
private String serviceId;
Expand All @@ -49,6 +50,8 @@ public class ZookeeperInstance implements MicroserviceInstance {

private Map<String, String> properties = new HashMap<>();

private MicroserviceInstanceStatus status;

public ZookeeperInstance() {

}
Expand All @@ -66,6 +69,8 @@ public ZookeeperInstance(ZookeeperInstance other) {
this.endpoints = other.endpoints;
this.schemas = other.schemas;
this.properties = other.properties;
this.status = other.status;

}

public void setServiceId(String serviceId) {
Expand Down Expand Up @@ -116,6 +121,10 @@ public void setProperties(Map<String, String> properties) {
this.properties = properties;
}

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

@Override
public String getEnvironment() {
return this.environment;
Expand Down Expand Up @@ -187,4 +196,9 @@ public String getInstanceId() {
public String getServiceId() {
return serviceId;
}

@Override
public MicroserviceInstanceStatus getStatus() {
return this.status;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ public AppConfigurationEntry[] getAppConfigurationEntry(String name) {

private RegistrationId registrationId;

private ServiceDiscovery<ZookeeperInstance> dis;

@Autowired
@SuppressWarnings("unused")
public void setEnvironment(Environment environment) {
Expand Down Expand Up @@ -125,6 +127,8 @@ public void init() {
}
zookeeperInstance.setProperties(BootStrapProperties.readServiceProperties(environment));
zookeeperInstance.setVersion(BootStrapProperties.readServiceVersion(environment));

zookeeperInstance.setStatus(MicroserviceInstanceStatus.valueOf(zookeeperRegistryProperties.getInitialStatus()));
try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use initialstatus, do not need isEnableElegantUpDown

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don`t need it ,have removed

this.instance = ServiceInstance.<ZookeeperInstance>builder().name(zookeeperInstance.getServiceName())
.id(zookeeperInstance.getInstanceId()).payload(zookeeperInstance).build();
Expand Down Expand Up @@ -155,7 +159,7 @@ public void run() {
client.start();
JsonInstanceSerializer<ZookeeperInstance> serializer =
new JsonInstanceSerializer<>(ZookeeperInstance.class);
ServiceDiscovery<ZookeeperInstance> dis = ServiceDiscoveryBuilder.builder(ZookeeperInstance.class)
dis = ServiceDiscoveryBuilder.builder(ZookeeperInstance.class)
.client(client)
.basePath(basePath + "/" + BootStrapProperties.readApplication(environment))
.serializer(serializer)
Expand Down Expand Up @@ -187,8 +191,13 @@ public ZookeeperRegistrationInstance getMicroserviceInstance() {

@Override
public boolean updateMicroserviceInstanceStatus(MicroserviceInstanceStatus status) {
// not support yet
return true;
this.instance.getPayload().setStatus(status);
try {
dis.updateService(instance);
} catch (Exception e){
throw new IllegalStateException(e);
}
return true;
}

@Override
Expand All @@ -212,4 +221,21 @@ public void addProperty(String key, String value) {
public boolean enabled() {
return zookeeperRegistryProperties.isEnabled();
}

/**
* Returns the latest instance for checking when updating the instance status from STARTING to UP.
* @return ZookeeperInstance
*/
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;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method never used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don`t need it ,remove


}
Loading