alievmirza commented on code in PR #2448:
URL: https://github.com/apache/ignite-3/pull/2448#discussion_r1296811167
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##########
@@ -274,27 +278,19 @@ public DistributionZoneManager(
this.topologyWatchListener = createMetastorageTopologyListener();
- zonesState = new ConcurrentHashMap<>();
-
- logicalTopology = emptySet();
-
- nodesAttributes = new ConcurrentHashMap<>();
-
executor = createZoneManagerExecutor(
Math.min(Runtime.getRuntime().availableProcessors() * 3, 20),
- new
NamedThreadFactory(NamedThreadFactory.threadPrefix(nodeName,
DISTRIBUTION_ZONE_MANAGER_POOL_NAME), LOG)
+ NamedThreadFactory.create(nodeName, "dst-zones-scheduler", LOG)
Review Comment:
Why do we get rid of `DISTRIBUTION_ZONE_MANAGER_POOL_NAME`?
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/rebalance/DistributionZoneRebalanceEngine.java:
##########
@@ -119,19 +102,17 @@ public DistributionZoneRebalanceEngine(
* Starts the rebalance engine by registering corresponding meta storage
and configuration listeners.
*/
public void start() {
- if (!busyLock.enterBusy()) {
- throw new IgniteException(NODE_STOPPING_ERR, new
NodeStoppingException());
- }
-
- try {
-
zonesConfiguration.defaultDistributionZone().replicas().listen(onUpdateReplicas);
-
zonesConfiguration.distributionZones().any().replicas().listen(onUpdateReplicas);
+ IgniteUtils.inBusyLock(busyLock, () -> {
Review Comment:
Why don't we remove this listener in the stop?
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/causalitydatanodes/CausalityDataNodesEngine.java:
##########
@@ -89,7 +89,7 @@ public class CausalityDataNodesEngine {
* zoneId -> (revision -> zoneConfiguration).
* TODO IGNITE-20050 Clean up this map.
*/
- private final ConcurrentHashMap<Integer, ConcurrentSkipListMap<Long,
ZoneConfiguration>> zonesVersionedCfg;
+ private final ConcurrentHashMap<Integer, ConcurrentSkipListMap<Long,
ZoneConfiguration>> zonesVersionedCfg = new ConcurrentHashMap<>();
Review Comment:
The same question, why do we moved this from the constructor?
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##########
@@ -202,7 +205,7 @@ public class DistributionZoneManager implements
IgniteComponent {
* 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;
+ private final Map<Integer, ZoneState> zonesState = new
ConcurrentHashMap<>();
Review Comment:
Why do we moved these lines from the constructor?
--
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]
