New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SCB-986] Cache instances result in CseDiscoveryClient #990
Changes from 1 commit
44dd425
0d4159c
c749e11
0f7e052
0f38802
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ | |
import java.util.List; | ||
import java.util.Map; | ||
|
||
import org.apache.servicecomb.core.Const; | ||
import org.apache.servicecomb.foundation.common.cache.VersionedCache; | ||
import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx; | ||
import org.apache.servicecomb.foundation.common.net.URIEndpointObject; | ||
|
@@ -29,12 +30,18 @@ | |
import org.apache.servicecomb.serviceregistry.client.ServiceRegistryClient; | ||
import org.apache.servicecomb.serviceregistry.definition.DefinitionConst; | ||
import org.apache.servicecomb.serviceregistry.discovery.DiscoveryContext; | ||
import org.apache.servicecomb.serviceregistry.discovery.DiscoveryFilter; | ||
import org.apache.servicecomb.serviceregistry.discovery.DiscoveryTree; | ||
import org.apache.servicecomb.serviceregistry.discovery.DiscoveryTreeNode; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.cloud.client.DefaultServiceInstance; | ||
import org.springframework.cloud.client.ServiceInstance; | ||
import org.springframework.cloud.client.discovery.DiscoveryClient; | ||
|
||
public class CseDiscoveryClient implements DiscoveryClient { | ||
private static final Logger LOGGER = LoggerFactory.getLogger(CseDiscoveryClient.class); | ||
|
||
private Map<String, DiscoveryTree> discoveryTrees = new ConcurrentHashMapEx<>(); | ||
|
||
@Override | ||
|
@@ -44,24 +51,55 @@ public String description() { | |
|
||
@Override | ||
public List<ServiceInstance> getInstances(final String serviceId) { | ||
class InstanceDiscoveryFilter implements DiscoveryFilter { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. put class InstanceDiscoveryFilter in "getInstances", make "getInstances" logic not clear There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will move to a separate class |
||
@Override | ||
public int getOrder() { | ||
return Short.MAX_VALUE; | ||
} | ||
|
||
@Override | ||
public DiscoveryTreeNode discovery(DiscoveryContext context, DiscoveryTreeNode parent) { | ||
return parent.children() | ||
.computeIfAbsent(context.getInputParameters(), etn -> createDiscoveryTreeNode(context, parent)); | ||
} | ||
|
||
@SuppressWarnings("unchecked") | ||
protected DiscoveryTreeNode createDiscoveryTreeNode(DiscoveryContext context, | ||
DiscoveryTreeNode parent) { | ||
String serviceName = context.getInputParameters(); | ||
List<ServiceInstance> instances = new ArrayList<>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not align? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will fix |
||
for (MicroserviceInstance instance : ((Map<String, MicroserviceInstance>) parent.data()).values()) { | ||
for (String endpoint : instance.getEndpoints()) { | ||
String scheme = endpoint.split(":", 2)[0]; | ||
if (!scheme.equalsIgnoreCase(Const.RESTFUL)) { | ||
LOGGER.info("Endpoint {} is not supported in Spring Cloud, ignoring.", endpoint); | ||
continue; | ||
} | ||
URIEndpointObject uri = new URIEndpointObject(endpoint); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. zuul can not support highway |
||
instances.add(new DefaultServiceInstance(serviceId, uri.getHostOrIp(), uri.getPort(), uri.isSslEnabled())); | ||
} | ||
} | ||
return new DiscoveryTreeNode() | ||
.subName(parent, serviceName) | ||
.data(instances); | ||
} | ||
}; | ||
|
||
DiscoveryContext context = new DiscoveryContext(); | ||
context.setInputParameters(serviceId); | ||
|
||
DiscoveryTree discoveryTree = discoveryTrees.computeIfAbsent(serviceId, key -> { | ||
return new DiscoveryTree(); | ||
DiscoveryTree tree = new DiscoveryTree(); | ||
tree.addFilter(new InstanceDiscoveryFilter()); | ||
return tree; | ||
}); | ||
|
||
VersionedCache serversVersionedCache = discoveryTree.discovery(context, | ||
RegistryUtils.getAppId(), | ||
serviceId, | ||
DefinitionConst.VERSION_RULE_ALL); | ||
Map<String, MicroserviceInstance> servers = serversVersionedCache.data(); | ||
List<ServiceInstance> instances = new ArrayList<>(servers.size()); | ||
for (MicroserviceInstance s : servers.values()) { | ||
for (String endpoint : s.getEndpoints()) { | ||
URIEndpointObject uri = new URIEndpointObject(endpoint); | ||
instances.add(new DefaultServiceInstance(serviceId, uri.getHostOrIp(), uri.getPort(), uri.isSslEnabled())); | ||
} | ||
} | ||
return instances; | ||
|
||
return serversVersionedCache.data(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what happened when serversVersionedCache is empty? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's OK for the versionedCache to be empty. As it will not be NULL. When there are associated filters and the children data is null, an Exception will be raised. |
||
} | ||
|
||
@Override | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why need this new dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DiscoveryFilter interface is defined in this package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no , you add new core dependence
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's for Const.RESTFUL