sergeyuttsel commented on code in PR #2873:
URL: https://github.com/apache/ignite-3/pull/2873#discussion_r1406280128
##########
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.
Review Comment:
I think that it is impossible to get this state because
initDataNodesAndTriggerKeysInMetaStorage() method returns the future. So when
zone creation event is handled then the data nodes is updated in MS.
So we can remove this block and zonesCreateRevision.
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/causalitydatanodes/CausalityDataNodesEngine.java:
##########
@@ -71,24 +67,23 @@ public class CausalityDataNodesEngine {
/** Meta Storage manager. */
private final MetaStorageManager msManager;
- /** Vault manager. */
- private final VaultManager vaultMgr;
-
/** Distribution zones manager. */
private final DistributionZoneManager distributionZoneManager;
+ /** Catalog manager. */
+ private final CatalogManager catalogManager;
+
/**
* Map with states for distribution zones. States are needed to track
nodes that we want to add or remove from the data nodes,
* schedule and stop scale up and scale down processes.
*/
private final Map<Integer, ZoneState> zonesState;
/**
- * The map which contains configuration changes which trigger zone's data
nodes recalculation.
- * zoneId -> (revision -> zoneConfiguration).
+ * The map which contains zones' create revision. It is updated only if
the current node handled zone's create event.
* TODO IGNITE-20050 Clean up this map.
*/
- private final ConcurrentHashMap<Integer, ConcurrentSkipListMap<Long,
ZoneConfiguration>> zonesVersionedCfg = new ConcurrentHashMap<>();
+ private final ConcurrentHashMap<Integer, Long> zonesCreateRevision = new
ConcurrentHashMap<>();
/** Used to guarantee that the zone will be created before other
components use the zone. */
Review Comment:
Lets update this javadoc. zonesVv used to guarantee that event which is
associated with a causalityToken is handled, not only for a zone creation, but
also other events which updates the zone.
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/causalitydatanodes/CausalityDataNodesEngine.java:
##########
@@ -98,39 +93,45 @@ public class CausalityDataNodesEngine {
*
* @param busyLock Busy lock to stop synchronously.
* @param msManager Meta Storage manager.
- * @param vaultMgr Vault manager.
* @param zonesState Map with states for distribution zones.
* @param distributionZoneManager Distribution zones manager.
Review Comment:
Please update params. registry and catalogManager are missed.
--
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]
