sergeyuttsel commented on code in PR #2873:
URL: https://github.com/apache/ignite-3/pull/2873#discussion_r1406479790


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/causalitydatanodes/CausalityDataNodesEngine.java:
##########
@@ -147,41 +148,32 @@ public CompletableFuture<Set<String>> dataNodes(long 
causalityToken, int zoneId)
                 return CompletableFuture.completedFuture(null);
             }
         }).thenApply(ignored -> inBusyLock(busyLock, () -> {
-            ConcurrentSkipListMap<Long, ZoneConfiguration> versionedCfg = 
zonesVersionedCfg.get(zoneId);
-
-            // Get the latest configuration and configuration revision for a 
given causality token
-            Map.Entry<Long, ZoneConfiguration> zoneLastCfgEntry = 
versionedCfg.floorEntry(causalityToken);
+            CatalogZoneDescriptor zoneDescriptor = 
catalogManager.catalog(catalogVersion).zone(zoneId);
 
-            if (zoneLastCfgEntry == null) {
-                // It means that the zone does not exist on a given causality 
token.
+            if (zoneDescriptor == null) {
+                // It means that the zone does not exist on a given causality 
token or causality token is lower than metastorage recovery
+                // revision.
                 throw new DistributionZoneNotFoundException(zoneId);
             }
 
-            long lastCfgRevision = zoneLastCfgEntry.getKey();
+            Long createRevision = zonesCreateRevision.get(zoneId);
 
-            ZoneConfiguration zoneLastCfg = zoneLastCfgEntry.getValue();
+            long descLastUpdateRevision = zoneDescriptor.updateToken();
 
-            String filter = zoneLastCfg.getFilter();
-
-            boolean isZoneRemoved = zoneLastCfg.getIsRemoved();
-
-            if (isZoneRemoved) {
-                // It means that the zone was removed on a given causality 
token.
-                throw new DistributionZoneNotFoundException(zoneId);
-            }
+            String filter = zoneDescriptor.filter();
 
             // Get revisions of the last scale up and scale down event which 
triggered immediate data nodes recalculation.
-            long lastScaleUpRevision = 
getRevisionsOfLastScaleUpEvent(causalityToken, zoneId);
-            long lastScaleDownRevision = 
getRevisionsOfLastScaleDownEvent(causalityToken, zoneId);
+            long lastScaleUpRevision = 
getRevisionsOfLastScaleUpEvent(causalityToken, catalogVersion, zoneId);
+            long lastScaleDownRevision = 
getRevisionsOfLastScaleDownEvent(causalityToken, catalogVersion, zoneId);
 
-            if (lastCfgRevision == versionedCfg.firstKey()
-                    && lastCfgRevision >= lastScaleUpRevision
-                    && lastCfgRevision >= lastScaleDownRevision
+            if (createRevision != null && 
createRevision.equals(descLastUpdateRevision)
+                    && descLastUpdateRevision >= lastScaleUpRevision
+                    && descLastUpdateRevision >= lastScaleDownRevision
             ) {
                 // It means that the zone was created but the data nodes value 
had not updated yet.
-                // So the data nodes value will be equals to the logical 
topology on the lastCfgRevision.

Review Comment:
   I think we don't need all code related to the augmentation map in the 
CausalityDataNodesEngine#dataNodes method. This code is needed for zones with 
immediate timers. But now the initDataNodesAndTriggerKeysInMetaStorage() watch 
listener updates augmentation and returns the future which is completed when 
the data nodes is updated in MS. So we don't need to add/remove nodes from the 
augmentation map to data nodes from MS.



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