alievmirza commented on code in PR #4318:
URL: https://github.com/apache/ignite-3/pull/4318#discussion_r1742026728


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZonesUtil.java:
##########
@@ -377,21 +397,25 @@ static Update deleteDataNodesAndTriggerKeys(int zoneId, 
long revision) {
     }
 
     /**
-     * Updates logical topology and logical topology version values for zones.
+     * Updates logical topology, its version and (if this is an initial 
version) cluster ID values for zones.
      *
-     * @param logicalTopology Logical topology.
-     * @param topologyVersion Logical topology version.
+     * @param logicalTopology Logical topology snapshot.
      * @return Update command for the meta storage.
      */
-    static Update updateLogicalTopologyAndVersion(Set<LogicalNode> 
logicalTopology, long topologyVersion) {
-        Set<NodeWithAttributes> topologyFromCmg = logicalTopology.stream()
+    static Update 
updateLogicalTopologyAndVersionAndClusterId(LogicalTopologySnapshot 
logicalTopology) {
+        Set<NodeWithAttributes> topologyFromCmg = 
logicalTopology.nodes().stream()
                 .map(n -> new NodeWithAttributes(n.name(), n.id(), 
n.userAttributes(), n.storageProfiles()))
                 .collect(toSet());
 
-        return ops(
-                put(zonesLogicalTopologyVersionKey(), 
longToBytesKeepingOrder(topologyVersion)),
-                put(zonesLogicalTopologyKey(), 
ByteUtils.toBytes(topologyFromCmg))
-        ).yield(true);
+        List<Operation> operations = new ArrayList<>();
+
+        operations.add(put(zonesLogicalTopologyVersionKey(), 
longToBytesKeepingOrder(logicalTopology.version())));
+        operations.add(put(zonesLogicalTopologyKey(), 
ByteUtils.toBytes(topologyFromCmg)));
+        if (logicalTopology.version() == 
LogicalTopologySnapshot.FIRST_VERSION) {

Review Comment:
   it is a bit confusing that we will try to write 
`zonesLogicalTopologyClusterIdKey` even if condition will be
   ```
   updateCondition = 
value(zonesLogicalTopologyVersionKey()).lt(longToBytesKeepingOrder(newTopology.version()));
   ```
   I would separate two updates, depending on the condition 
   ```
   if (newTopology.version() == LogicalTopologySnapshot.FIRST_VERSION) {
   ```



##########
modules/core/src/main/java/org/apache/ignite/internal/util/ByteUtils.java:
##########
@@ -344,4 +345,16 @@ public static List<byte[]> 
toByteArrayList(List<ByteBuffer> byteBufferList) {
 
         return unmodifiableList(result);
     }
+
+    /**
+     * Converts a UUID to bytes.
+     */
+    public static byte[] uuidToBytes(UUID uuid) {

Review Comment:
   why can't we use `org.apache.ignite.internal.util.ByteUtils#toBytes`?



##########
modules/runner/src/testFixtures/java/org/apache/ignite/internal/Cluster.java:
##########
@@ -488,6 +488,15 @@ public Stream<Ignite> runningNodes() {
         return nodes.stream().filter(Objects::nonNull);
     }
 
+    /**
+     * Returns indexes of nodes that are started and not stopped..
+     */
+    public Stream<Integer> runningNodeIndexes() {

Review Comment:
   seems like this method is not used



##########
modules/system-disaster-recovery/src/integrationTest/java/org/apache/ignite/internal/disaster/system/ItCmgDisasterRecoveryTest.java:
##########
@@ -351,4 +352,79 @@ void repeatedRepairWorks() throws Exception {
         // TODO: IGNITE-23096 - remove after the hang is fixed.
         waitTillNodesRestartInProcess(0, 1);
     }
+
+    @Test
+    void dataNodesAreUpdatedCorrectlyAfterClusterReset() throws Exception {
+        cluster.startAndInit(2, paramsBuilder -> {
+            paramsBuilder.cmgNodeNames(nodeNames(0));
+            paramsBuilder.metaStorageNodeNames(nodeNames(1));
+        });
+        waitTillClusterStateIsSavedToVaultOnConductor(1);
+
+        final String zoneName = "TEST_ZONE";
+
+        cluster.node(1).sql().execute(
+                null,
+                "CREATE ZONE " + zoneName + " WITH STORAGE_PROFILES='default', 
"
+                        + "DATA_NODES_AUTO_ADJUST_SCALE_UP=0, 
DATA_NODES_AUTO_ADJUST_SCALE_DOWN=0"
+        );
+
+        int zoneId = igniteImpl(1).catalogManager().zone("TEST_ZONE", 
Long.MAX_VALUE).id();
+
+        waitTillDataNodesBecome(new int[]{0, 1}, zoneId, igniteImpl(1));
+
+        cluster.startNode(2);
+
+        waitTillDataNodesBecome(new int[]{0, 1, 2}, zoneId, igniteImpl(1));
+
+        // This makes the CMG majority go away.
+        cluster.stopNode(0);
+
+        // Now, dataNodes should have become [1, 2], but as there is no CMG 
leader, noone is able to trigger data nodes update.

Review Comment:
   no one
   
   



##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/topology/api/LogicalTopologySnapshot.java:
##########
@@ -34,17 +36,32 @@
 public class LogicalTopologySnapshot implements Serializable {
     private static final long serialVersionUID = 0L;
 
+    /** Version that first topology snapshot will have. */

Review Comment:
   It is not clear, what does "first topology" mean. Since which event it is 
first? 



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