github-actions[bot] commented on code in PR #68833:
URL: https://github.com/apache/doris/pull/68833#discussion_r4229997036
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudInstanceStatusChecker.java:
##########
@@ -517,20 +519,31 @@ private void handleNewVirtualComputeGroup(Cloud.ClusterPB
cluster, List<Cloud.Cl
private void checkSubClusters(List<String> subClusterNames,
Cloud.ClusterPB cluster,
List<Cloud.ClusterPB> computeClustersInPB) {
+ Map<String, Cloud.ClusterPB> computeClustersByName =
computeClustersInPB.stream()
Review Comment:
[P2] Build the compute-name index once per instance snapshot.
`checkSubClusters` rebuilds a map of every physical cluster for every VCG,
though every call in this cycle receives the same `computeClusters` list. With
1,000 VCGs backed by 2,000 distinct compute groups, each 10-second check makes
about two million repeated map insertions and allocates 1,000 maps before the
subgroup lookups. Build the index once in `processVirtualClusters` and pass it
through both VCG paths; use it for the name check in `handleFailedSync` too, so
failed cycles do not retain a full-list scan per missing subgroup.
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudInstanceStatusChecker.java:
##########
@@ -517,20 +519,31 @@ private void handleNewVirtualComputeGroup(Cloud.ClusterPB
cluster, List<Cloud.Cl
private void checkSubClusters(List<String> subClusterNames,
Cloud.ClusterPB cluster,
List<Cloud.ClusterPB> computeClustersInPB) {
+ Map<String, Cloud.ClusterPB> computeClustersByName =
computeClustersInPB.stream()
+ .collect(Collectors.toMap(Cloud.ClusterPB::getClusterName,
computeCluster -> computeCluster));
+ boolean allSubClustersExist = true;
for (String subClusterName : subClusterNames) {
- if (cloudSystemInfoService.getCloudClusterIdByName(subClusterName)
== null) {
+ Cloud.ClusterPB subClusterInMs =
computeClustersByName.get(subClusterName);
+ String subClusterIdInFe =
cloudSystemInfoService.getCloudClusterIdByName(subClusterName);
+ boolean sameSubClusterInFe = subClusterInMs != null
+ && subClusterInMs.getClusterId().equals(subClusterIdInFe);
Review Comment:
[P3] Report subgroup ID mismatches as mismatches. This new equality check
can fail when MS has compute group `p` with a new ID while FE still maps `p` to
its old ID, such as after a physical rename and name reuse before
CloudClusterChecker catches up. `handleFailedSync` then logs `fe mem cant find
p` and later says cluster info cannot be found, although FE did find the name
with the wrong ID. Preserve the FE and MS IDs for the mismatch branch so the
warning identifies the actual reconciliation problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
