alex-plekhanov commented on code in PR #12517:
URL: https://github.com/apache/ignite/pull/12517#discussion_r2719696361


##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/internal/TcpDiscoveryNodesRing.java:
##########
@@ -506,7 +506,10 @@ public void clear() {
             if (filtered.size() < 2)
                 return null;
 
-            Iterator<TcpDiscoveryNode> iter = filtered.iterator();

Review Comment:
   Methodd `TcpDiscoverySpi#forceNextNodeFailure` -> 
`ServerImpl#forceNextNodeFailure` depend on `nextNode`,  these methods are 
never used and can be cleaned up.



##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/internal/MdcAwareNodesComparator.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.spi.discovery.tcp.internal;
+
+import java.util.Comparator;
+
+/** Compares nodes using the Data Center id as a primary factor. */
+public class MdcAwareNodesComparator implements Comparator<TcpDiscoveryNode> {
+    /** */
+    @Override public int compare(TcpDiscoveryNode n1, TcpDiscoveryNode n2) {
+        String n1DcId = n1.dataCenterId() == null ? "" : n1.dataCenterId();
+        String n2DcId = n2.dataCenterId() == null ? "" : n2.dataCenterId();
+
+        int res = n1DcId.compareTo(n2DcId);
+
+        if (res == 0) {

Review Comment:
   Redundant braces



##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/internal/TcpDiscoveryNodesRing.java:
##########
@@ -506,7 +506,10 @@ public void clear() {
             if (filtered.size() < 2)
                 return null;
 
-            Iterator<TcpDiscoveryNode> iter = filtered.iterator();
+            NavigableSet<TcpDiscoveryNode> sorted = new TreeSet<>(new 
MdcAwareNodesComparator());
+            sorted.addAll(filtered);

Review Comment:
   I'm not sure if `TreeSet` always preserves order of added elements if all 
elements are equal according to comparator, so it potentianly can break 
something in non-multi-DC cluster. Let's use sorting only for multi-DC 
environment. Also it's a potentially dangerous feature even for multi-DC, I 
think it's worth to have a flag to disable it explicitely. 



##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/internal/MdcAwareNodesComparator.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.spi.discovery.tcp.internal;
+
+import java.util.Comparator;
+
+/** Compares nodes using the Data Center id as a primary factor. */
+public class MdcAwareNodesComparator implements Comparator<TcpDiscoveryNode> {
+    /** */
+    @Override public int compare(TcpDiscoveryNode n1, TcpDiscoveryNode n2) {
+        String n1DcId = n1.dataCenterId() == null ? "" : n1.dataCenterId();
+        String n2DcId = n2.dataCenterId() == null ? "" : n2.dataCenterId();

Review Comment:
   For multi-DC environment we have a check, that all `dataCenterId` is 
provided. We can ommit nullability check if comparator will be used only for 
multi-DC.



-- 
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]

Reply via email to