Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -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 @@ -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 @@ -142,7 +142,11 @@ public static MicroserviceInstance createMicroserviceInstance(
properties.putAll(BootStrapProperties.readServiceProperties(environment));
properties.putAll(genCasProperties(environment));
microserviceInstance.setProperties(properties);
microserviceInstance.setStatus(MicroserviceInstanceStatus.valueOf(scConfigurationProperties.getInitialStatus()));
if (scConfigurationProperties.isEnableElegantUpDown()){
microserviceInstance.setStatus(MicroserviceInstanceStatus.STARTING);
}else {
microserviceInstance.setStatus(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.

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

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

return microserviceInstance;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ public class SCConfigurationProperties {

private boolean enableSwaggerRegistration = false;

private boolean enableElegantUpDown = false;

/**
* for registration service
* when swagger is different between local with remote serviceCenter. if ignoreSwaggerDifferent is true.
Expand Down Expand Up @@ -171,4 +173,12 @@ public boolean isEnableSwaggerRegistration() {
public void setEnableSwaggerRegistration(boolean enableSwaggerRegistration) {
this.enableSwaggerRegistration = enableSwaggerRegistration;
}

public boolean isEnableElegantUpDown() {
return enableElegantUpDown;
}

public void setEnableElegantUpDown(boolean enableElegantUpDown) {
this.enableElegantUpDown = enableElegantUpDown;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

See above, maybe do not need this modification.

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

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

@Override
public DataCenterInfo getDataCenterInfo() {
return new DataCenterInfo(microserviceInstance.getDataCenterInfo().getName(),
microserviceInstance.getDataCenterInfo().getRegion(),
microserviceInstance.getDataCenterInfo().getAvailableZone());
org.apache.servicecomb.service.center.client.model.DataCenterInfo dataCenterInfo = microserviceInstance.getDataCenterInfo();
if (dataCenterInfo != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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, Setting the initial value on DataCenterProperties

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

}
return new DataCenterInfo();
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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, Setting the initial value on DataCenterProperties
image

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,10 @@ public Microservice getBackendMicroservice() {
public MicroserviceInstance getBackendMicroserviceInstance() {
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 @@ -26,7 +26,7 @@ public ZookeeperDiscoveryInstance(ZookeeperInstance other) {

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

@Override
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 @@ -187,4 +192,12 @@ public String getInstanceId() {
public String getServiceId() {
return serviceId;
}

public MicroserviceInstanceStatus getStatus() {
return status;
}

public void setStatus(MicroserviceInstanceStatus status) {
this.status = status;
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 add getStatus to MicroserviceInstance and force all implementations provide status information.

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().

}
}
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,12 @@ public void init() {
}
zookeeperInstance.setProperties(BootStrapProperties.readServiceProperties(environment));
zookeeperInstance.setVersion(BootStrapProperties.readServiceVersion(environment));

if (zookeeperRegistryProperties.isEnableElegantUpDown()){
zookeeperInstance.setStatus(MicroserviceInstanceStatus.STARTING);
}else {
zookeeperInstance.setStatus(MicroserviceInstanceStatus.UP);
}
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 +163,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 +195,19 @@ public ZookeeperRegistrationInstance getMicroserviceInstance() {

@Override
public boolean updateMicroserviceInstanceStatus(MicroserviceInstanceStatus status) {
// not support yet
return true;
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);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Why use registerService when Down?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

has been changed to updateService()

return true;
}

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

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


}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public class ZookeeperRegistryProperties {

private boolean enableSwaggerRegistration = false;

private boolean enableElegantUpDown = false;

public boolean isEnabled() {
return enabled;
}
Expand Down Expand Up @@ -96,4 +98,12 @@ public String getAuthenticationInfo() {
public void setAuthenticationInfo(String authenticationInfo) {
this.authenticationInfo = authenticationInfo;
}

public boolean isEnableElegantUpDown() {
return enableElegantUpDown;
}

public void setEnableElegantUpDown(boolean enableElegantUpDown) {
this.enableElegantUpDown = enableElegantUpDown;
}
}
Loading