Copilot commented on code in PR #59932:
URL: https://github.com/apache/doris/pull/59932#discussion_r2693977545
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudReplica.java:
##########
@@ -628,12 +625,7 @@ public void checkAndClearSecondaryClusterToBe(String
clusterId, long expireTimes
public List<Backend> getAllPrimaryBes() {
List<Backend> result = new ArrayList<Backend>();
- primaryClusterToBackends.keySet().forEach(clusterId -> {
- List<Long> backendIds = primaryClusterToBackends.get(clusterId);
- if (backendIds == null || backendIds.isEmpty()) {
- return;
- }
- Long beId = backendIds.get(0);
+ primaryClusterToBackend.forEach((clusterId, beId) -> {
if (beId != -1) {
Backend backend = Env.getCurrentSystemInfo().getBackend(beId);
result.add(backend);
Review Comment:
The method adds backends to the result list without checking if the backend
is null. If Env.getCurrentSystemInfo().getBackend(beId) returns null, a null
value will be added to the list. Consider adding a null check before adding to
the result list.
```suggestion
if (backend != null) {
result.add(backend);
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudReplica.java:
##########
@@ -29,33 +29,35 @@
import org.apache.doris.common.DdlException;
import org.apache.doris.common.Pair;
import org.apache.doris.common.util.DebugPointUtil;
+import org.apache.doris.persist.gson.GsonPostProcessable;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.system.Backend;
import com.google.common.base.Strings;
-import com.google.common.collect.Lists;
import com.google.common.hash.HashCode;
import com.google.common.hash.Hashing;
import com.google.gson.annotations.SerializedName;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
-public class CloudReplica extends Replica {
+public class CloudReplica extends Replica implements GsonPostProcessable {
private static final Logger LOG = LogManager.getLogger(CloudReplica.class);
- // In the future, a replica may be mapped to multiple BEs in a cluster,
- // so this value is be list
+ // a replica is mapped to one BE in a cluster, use primaryClusterToBackend
instead of primaryClusterToBackends
Review Comment:
Consider enhancing this comment to explain why this change was made. For
example: "a replica is mapped to one BE in a cluster to reduce memory usage.
Use primaryClusterToBackend instead of primaryClusterToBackends"
```suggestion
// A replica is now mapped to exactly one BE in a cluster to reduce
memory usage and simplify routing.
// Use primaryClusterToBackend instead of primaryClusterToBackends for
the single-backend mapping.
```
--
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]
