Skip to content
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

[ISSUE #3833] optimization ZookeeperMetaService #4780

Merged
merged 1 commit into from
Feb 27, 2024
Merged
Changes from all commits
Commits
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 @@ -47,6 +47,7 @@

import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -69,7 +70,7 @@ public class ZookeeperMetaService implements MetaService {
private String serverAddr;

@Getter
public CuratorFramework zkClient;
private CuratorFramework zkClient;

private ConcurrentMap<String, EventMeshRegisterInfo> eventMeshRegisterInfoMap;

Expand Down Expand Up @@ -184,49 +185,15 @@ public void shutdown() throws MetaException {
public List<EventMeshDataInfo> findEventMeshInfoByCluster(String clusterName) throws MetaException {
List<EventMeshDataInfo> eventMeshDataInfoList = new ArrayList<>();
for (String key : ConfigurationContextUtil.KEYS) {

CommonConfiguration configuration = ConfigurationContextUtil.get(key);
if (Objects.isNull(configuration)) {
continue;
}
String eventMeshName = configuration.getEventMeshName();
try {
String serviceName = eventMeshName.concat("-").concat(key);
String servicePath = formatServicePath(clusterName, serviceName);

List<String> instances = zkClient.getChildren()
.forPath(servicePath);

if (CollectionUtils.isEmpty(instances)) {
continue;
}

for (String endpoint : instances) {
String instancePath = servicePath.concat(ZookeeperConstant.PATH_SEPARATOR).concat(endpoint);

Stat stat = new Stat();
byte[] data;
try {
data = zkClient.getData()
.storingStatIn(stat)
.forPath(instancePath);
} catch (Exception e) {
log.warn("[ZookeeperRegistryService][findEventMeshInfoByCluster] failed for path: {}", instancePath, e);
continue;
}

EventMeshInstance eventMeshInstance = JsonUtils.parseObject(new String(data, StandardCharsets.UTF_8), EventMeshInstance.class);

EventMeshDataInfo eventMeshDataInfo =
new EventMeshDataInfo(clusterName, serviceName, endpoint, stat.getMtime(),
Objects.requireNonNull(eventMeshInstance, "instance must not be Null").getMetaData());

eventMeshDataInfoList.add(eventMeshDataInfo);
}

} catch (Exception e) {
throw new MetaException("ZookeeperRegistry findEventMeshInfoByCluster failed", e);
}
String serviceName = eventMeshName.concat("-").concat(key);

findEventMeshInfo("findEventMeshInfoByCluster", clusterName, serviceName, eventMeshDataInfoList);
}
return eventMeshDataInfoList;
}
Expand All @@ -239,44 +206,49 @@ public List<EventMeshDataInfo> findAllEventMeshInfo() throws MetaException {

String serviceName = entry.getKey();
String clusterName = entry.getValue().getEventMeshClusterName();
try {
String servicePath = formatServicePath(clusterName, serviceName);

List<String> instances = zkClient.getChildren()
.forPath(servicePath);
findEventMeshInfo("findAllEventMeshInfo", clusterName, serviceName, eventMeshDataInfoList);
}
return eventMeshDataInfoList;
}

if (CollectionUtils.isEmpty(instances)) {
continue;
}
private void findEventMeshInfo(String tipTitle, String clusterName,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think tipTitle is needed here just for logging message.

Copy link
Member

Choose a reason for hiding this comment

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

The naming of tipTitle seems to indicate the name of its caller, only findEventMeshInfoByCluster for now. At least the naming of tipTitle is a bit confusing.

Is there any plan to add more callers like findEventMeshInfoBySubmodule and should we keep the tipTitle param? @Alonexc @mxsm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

感谢大佬们指点
Thank you for your guidance. My consideration is that the tipTitle here has two values, "findEventMeshInfoByCluster" and "findAllEventMeshInfo", whose call links are executed at different times. If there is an error, I am concerned that it may differ from the previous log printing rules

String serviceName, List<EventMeshDataInfo> eventMeshDataInfoList) throws MetaException {
try {
String servicePath = formatServicePath(clusterName, serviceName);

for (String endpoint : instances) {
String instancePath = servicePath.concat(ZookeeperConstant.PATH_SEPARATOR).concat(endpoint);
List<String> instances = zkClient.getChildren().forPath(servicePath);

Stat stat = new Stat();
byte[] data;
try {
data = zkClient.getData()
.storingStatIn(stat)
.forPath(instancePath);
} catch (Exception e) {
log.warn("[ZookeeperRegistryService][findAllEventMeshInfo] failed for path: {}", instancePath, e);
continue;
}
if (CollectionUtils.isEmpty(instances)) {
return;
}

EventMeshInstance eventMeshInstance = JsonUtils.parseObject(new String(data, StandardCharsets.UTF_8), EventMeshInstance.class);
for (String endpoint : instances) {
String instancePath = servicePath.concat(ZookeeperConstant.PATH_SEPARATOR).concat(endpoint);

Stat stat = new Stat();
byte[] data;
try {
data = zkClient.getData()
.storingStatIn(stat)
.forPath(instancePath);
} catch (Exception e) {
log.warn("[ZookeeperRegistryService][{}] failed for path: {}", tipTitle, instancePath, e);
continue;
}

EventMeshDataInfo eventMeshDataInfo =
new EventMeshDataInfo(clusterName, serviceName, endpoint, stat.getMtime(),
Objects.requireNonNull(eventMeshInstance, "instance must not be Null").getMetaData());
EventMeshInstance eventMeshInstance = JsonUtils.parseObject(new String(data, StandardCharsets.UTF_8), EventMeshInstance.class);

eventMeshDataInfoList.add(eventMeshDataInfo);
}
EventMeshDataInfo eventMeshDataInfo =
new EventMeshDataInfo(clusterName, serviceName, endpoint, stat.getMtime(),
Objects.requireNonNull(eventMeshInstance, "instance must not be Null").getMetaData());

} catch (Exception e) {
throw new MetaException("ZookeeperRegistry findAllEventMeshInfo failed", e);
eventMeshDataInfoList.add(eventMeshDataInfo);
}

} catch (Exception e) {
throw new MetaException(String.format("ZookeeperRegistry {0} failed", tipTitle), e);
}
return eventMeshDataInfoList;
}

@Override
Expand All @@ -290,7 +262,7 @@ public void registerMetadata(Map<String, String> metadataMap) {

@Override
public Map<String, String> getMetaData(String key, boolean fuzzyEnabled) {
return null;
return new HashMap<>();
}

// todo: to be implemented
Expand Down
Loading