You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/12/12 15:36:47 UTC

[GitHub] [ignite-3] alievmirza opened a new pull request, #1436: IGNITE-18087 Populate DistributionZoneManager with CMG listeners to logical topology events

alievmirza opened a new pull request, #1436:
URL: https://github.com/apache/ignite-3/pull/1436

   https://issues.apache.org/jira/browse/IGNITE-18087


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] sanpwc merged pull request #1436: IGNITE-18087 Populate DistributionZoneManager with CMG listeners to logical topology events

Posted by GitBox <gi...@apache.org>.
sanpwc merged PR #1436:
URL: https://github.com/apache/ignite-3/pull/1436


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] sergeyuttsel commented on a diff in pull request #1436: IGNITE-18087 Populate DistributionZoneManager with CMG listeners to logical topology events

Posted by GitBox <gi...@apache.org>.
sergeyuttsel commented on code in PR #1436:
URL: https://github.com/apache/ignite-3/pull/1436#discussion_r1048355394


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZonesUtil.java:
##########
@@ -36,19 +37,48 @@ class DistributionZonesUtil {
     /** Key prefix for zone's data nodes. */
     private static final String DISTRIBUTION_ZONE_DATA_NODES_PREFIX = "distributionZone.dataNodes.";
 
+    /** Key prefix for zones' logical topology nodes. */
+    private static final String DISTRIBUTION_ZONES_LOGICAL_TOPOLOGY_PREFIX = "distributionZones.logicalTopology";
+
+    /** Key prefix for zones' logical topology version. */
+    private static final String DISTRIBUTION_ZONES_LOGICAL_TOPOLOGY_VERSION_PREFIX = "distributionZones.logicalTopologyVersion";

Review Comment:
   Not a prefix.



##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZonesUtil.java:
##########
@@ -36,19 +37,48 @@ class DistributionZonesUtil {
     /** Key prefix for zone's data nodes. */
     private static final String DISTRIBUTION_ZONE_DATA_NODES_PREFIX = "distributionZone.dataNodes.";
 
+    /** Key prefix for zones' logical topology nodes. */
+    private static final String DISTRIBUTION_ZONES_LOGICAL_TOPOLOGY_PREFIX = "distributionZones.logicalTopology";
+
+    /** Key prefix for zones' logical topology version. */
+    private static final String DISTRIBUTION_ZONES_LOGICAL_TOPOLOGY_VERSION_PREFIX = "distributionZones.logicalTopologyVersion";
+
     /** The key, needed for processing the event about zones' update was triggered only once. */
-    private static final ByteArray ZONES_CHANGE_TRIGGER_KEY = new ByteArray("distributionZones.change.trigger");
+    private static final ByteArray DISTRIBUTION_ZONES_CHANGE_TRIGGER_KEY = new ByteArray("distributionZones.change.trigger");
 
     /** ByteArray representation of {@link DistributionZonesUtil#DISTRIBUTION_ZONE_DATA_NODES_PREFIX}. */
     static ByteArray zoneDataNodesKey(int zoneId) {
         return new ByteArray(DISTRIBUTION_ZONE_DATA_NODES_PREFIX + zoneId);
     }
 
+    /** ByteArray representation of {@link DistributionZonesUtil#DISTRIBUTION_ZONES_LOGICAL_TOPOLOGY_PREFIX}. */
+    private static final ByteArray DISTRIBUTION_ZONE_LOGICAL_TOPOLOGY_KEY = new ByteArray(DISTRIBUTION_ZONES_LOGICAL_TOPOLOGY_PREFIX);
+
+    /** ByteArray representation of {@link DistributionZonesUtil#DISTRIBUTION_ZONES_LOGICAL_TOPOLOGY_VERSION_PREFIX}. */
+    private static final ByteArray DISTRIBUTION_ZONES_LOGICAL_TOPOLOGY_VERSION_KEY =

Review Comment:
   Need to move "private static final" fields on top.



##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##########
@@ -373,4 +404,92 @@ private void updateMetaStorageOnZoneDelete(int zoneId, long revision) {
             busyLock.leaveBusy();
         }
     }
+
+    /**
+     * Updates {@link DistributionZonesUtil#zonesLogicalTopologyKey()} and {@link DistributionZonesUtil#zonesLogicalTopologyVersionKey()}
+     * in meta storage.
+     *
+     * @param newTopology Logical topology snapshot.
+     * @param topologyLeap Flag that indicates whether this updates was trigger by
+     *                     {@link LogicalTopologyEventListener#onTopologyLeap(LogicalTopologySnapshot)} or not.
+     */
+    private void updateMetaStorageKeys(LogicalTopologySnapshot newTopology, boolean topologyLeap) {
+        Set<String> topologyFromCmg = newTopology.nodes().stream().map(ClusterNode::name).collect(Collectors.toSet());
+
+        Condition updateCondition;
+
+        if (topologyLeap) {
+            updateCondition = value(zonesLogicalTopologyVersionKey()).lt(ByteUtils.longToBytes(newTopology.version()));
+        } else {
+            // This condition may be stronger, as far as we receive topology events one by one.
+            updateCondition = value(zonesLogicalTopologyVersionKey()).eq(ByteUtils.longToBytes(newTopology.version() - 1));
+        }
+
+        If iff = If.iif(
+                updateCondition,
+                updateLogicalTopologyAndVersion(topologyFromCmg, newTopology.version()),
+                ops().yield(false)
+        );
+
+        metaStorageManager.invoke(iff).thenAccept(res -> {
+            if (res.getAsBoolean()) {
+                LOG.debug(
+                        "Distribution zones' logical topology and version keys were updated [topology = {}, version = {}]",
+                        Arrays.toString(topologyFromCmg.toArray()),
+                        newTopology.version()
+                );
+            } else {
+                LOG.debug(
+                        "Failed to update distribution zones' logical topology and version keys [topology = {}, version = {}]",
+                        Arrays.toString(topologyFromCmg.toArray()),
+                        newTopology.version()
+                );
+            }
+        });
+    }
+
+    /**
+     * Initialises {@link DistributionZonesUtil#zonesLogicalTopologyKey()} and
+     * {@link DistributionZonesUtil#zonesLogicalTopologyVersionKey()} from meta storage on the start of {@link DistributionZoneManager}.
+     */
+    private void initMetaStorageKeysOnStart() {
+        logicalTopologyService.logicalTopologyOnLeader().thenAccept(snapshot -> {
+            long topologyVersionFromCmg = snapshot.version();
+
+            byte[] topVerFromMetastorage;
+
+            try {
+                topVerFromMetastorage = metaStorageManager.get(zonesLogicalTopologyVersionKey()).get().value();
+            } catch (InterruptedException | ExecutionException e) {
+                throw new RuntimeException(e);

Review Comment:
   Need to use IgniteInternalException or other.



##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZonesUtil.java:
##########
@@ -36,19 +37,48 @@ class DistributionZonesUtil {
     /** Key prefix for zone's data nodes. */
     private static final String DISTRIBUTION_ZONE_DATA_NODES_PREFIX = "distributionZone.dataNodes.";
 
+    /** Key prefix for zones' logical topology nodes. */
+    private static final String DISTRIBUTION_ZONES_LOGICAL_TOPOLOGY_PREFIX = "distributionZones.logicalTopology";

Review Comment:
   it isn't a prefix. It's just a DISTRIBUTION_ZONES_LOGICAL_TOPOLOGY or DISTRIBUTION_ZONES_LOGICAL_TOPOLOGY_KEY



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1436: IGNITE-18087 Populate DistributionZoneManager with CMG listeners to logical topology events

Posted by GitBox <gi...@apache.org>.
sanpwc commented on code in PR #1436:
URL: https://github.com/apache/ignite-3/pull/1436#discussion_r1050732738


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##########
@@ -294,11 +330,9 @@ private void updateMetaStorageOnZoneCreate(int zoneId, long revision) {
             // so we do not react on a stale events
             CompoundCondition triggerKeyCondition = triggerKeyCondition(revision);
 
-            Set<String> nodesConsistentIds = clusterNodes.stream().map(ClusterNode::name).collect(Collectors.toSet());
-
-            byte[] logicalTopologyBytes = ByteUtils.toBytes(nodesConsistentIds);
+            Set<String> nodesConsistentIds = logicalTopology.stream().map(ClusterNode::name).collect(Collectors.toSet());

Review Comment:
   Let's either add an assert that we don't expect empty logicalToplogy or add the corresponding check.



##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##########
@@ -74,21 +82,45 @@ public class DistributionZoneManager implements IgniteComponent {
     /** Busy lock to stop synchronously. */
     private final IgniteSpinBusyLock busyLock = new IgniteSpinBusyLock();
 
+    /** Logical topology service to track topology changes. */
+    private final LogicalTopologyService logicalTopologyService;
+
+    /** Listener for a topology events. */
+    private final LogicalTopologyEventListener topologyEventListener = new LogicalTopologyEventListener() {
+        @Override
+        public void onAppeared(ClusterNode appearedNode, LogicalTopologySnapshot newTopology) {
+            updateMetaStorageKeys(newTopology, false);

Review Comment:
   Confusing naming, I'd rather use updateLogicalTopologyInMetaStorage.



##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##########
@@ -235,12 +267,16 @@ public CompletableFuture<Void> dropZone(String name) {
     @Override
     public void start() {
         zonesConfiguration.distributionZones().listenElements(new ZonesConfigurationListener());
+
+        logicalTopologyService.addEventListener(topologyEventListener);
+
+        initMetaStorageKeysOnStart();
     }
 
     /** {@inheritDoc} */
     @Override
     public void stop() throws Exception {
-
+        logicalTopologyService.removeEventListener(topologyEventListener);

Review Comment:
   Busy locks are missing here and there.



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1436: IGNITE-18087 Populate DistributionZoneManager with CMG listeners to logical topology events

Posted by GitBox <gi...@apache.org>.
sanpwc commented on code in PR #1436:
URL: https://github.com/apache/ignite-3/pull/1436#discussion_r1050501543


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##########
@@ -74,21 +82,45 @@ public class DistributionZoneManager implements IgniteComponent {
     /** Busy lock to stop synchronously. */
     private final IgniteSpinBusyLock busyLock = new IgniteSpinBusyLock();
 
+    /** Logical topology service to track topology changes. */
+    private final LogicalTopologyService logicalTopologyService;
+
+    /** Listener for a topology events. */
+    private final LogicalTopologyEventListener topologyEventListener = new LogicalTopologyEventListener() {
+        @Override
+        public void onAppeared(ClusterNode appearedNode, LogicalTopologySnapshot newTopology) {
+            updateMetaStorageKeys(newTopology, false);

Review Comment:
   Confusing naming, I'd rather use updateLogicalTopologyInMetaStorage.



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org