You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "rpuch (via GitHub)" <gi...@apache.org> on 2023/06/27 14:36:31 UTC

[GitHub] [ignite-3] rpuch opened a new pull request, #2263: IGNITE-19843 Unify code that triggers rebalance

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

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


-- 
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 #2263: IGNITE-19843 Unify code that triggers rebalance

Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #2263:
URL: https://github.com/apache/ignite-3/pull/2263#discussion_r1246223324


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/rebalance/DistributionZoneRebalanceEngine.java:
##########
@@ -153,80 +149,48 @@ public CompletableFuture<Void> onUpdate(WatchEvent evt) {
                 }
 
                 try {
-                    byte[] dataNodesBytes = evt.entryEvent().newEntry().value();
+                    Set<Node> dataNodes = parseDataNodes(evt.entryEvent().newEntry().value());
 
-                    if (dataNodesBytes == null) {
+                    if (dataNodes == null) {
                         //The zone was removed so data nodes was removed too.
                         return completedFuture(null);
                     }
 
-                    NamedConfigurationTree<TableConfiguration, TableView, TableChange> tables = tablesConfiguration.tables();
-
                     int zoneId = extractZoneId(evt.entryEvent().newEntry().key());
 
-                    Set<Node> dataNodes = dataNodes(ByteUtils.fromBytes(dataNodesBytes));
-
-                    for (int i = 0; i < tables.value().size(); i++) {
-                        TableView tableView = tables.value().get(i);
+                    DistributionZoneView zoneConfig =
+                            getZoneById(zonesConfiguration, zoneId).value();
 
-                        int tableZoneId = tableView.zoneId();
+                    Set<String> filteredDataNodes = filterDataNodes(
+                            dataNodes,
+                            zoneConfig.filter(),
+                            distributionZoneManager.nodesAttributes()
+                    );
 
-                        DistributionZoneConfiguration distributionZoneConfiguration =
-                                getZoneById(zonesConfiguration, tableZoneId);
+                    if (filteredDataNodes.isEmpty()) {
+                        return completedFuture(null);
+                    }
 
-                        Set<String> filteredDataNodes = filterDataNodes(
-                                dataNodes,
-                                distributionZoneConfiguration.filter().value(),
-                                distributionZoneManager.nodesAttributes()
+                    for (TableView tableConfig : findTablesByZoneId(zoneId)) {
+                        CompletableFuture<?>[] partitionFutures = RebalanceUtil.triggerAllTablePartitionsRebalance(
+                                tableConfig,
+                                zoneConfig,
+                                filteredDataNodes,
+                                evt.entryEvent().newEntry().revision(),
+                                metaStorageManager
                         );
 
-                        if (filteredDataNodes.isEmpty()) {
-                            continue;
-                        }
-
-                        if (zoneId == tableZoneId) {
-                            TableConfiguration tableCfg = tables.get(tableView.name());
-
-                            int tableId = tableCfg.id().value();
-
-                            CompletableFuture<List<Set<Assignment>>> tableAssignmentsFut = tableAssignments(
-                                    metaStorageManager,
-                                    tableId,
-                                    distributionZoneConfiguration.partitions().value()
-                            );
-
-                            tableAssignmentsFut.thenAccept(tableAssignments -> {
-
-                                for (int part = 0; part < distributionZoneConfiguration.partitions().value(); part++) {
-
-                                    TablePartitionId replicaGrpId = new TablePartitionId(tableId, part);
-
-                                    int replicas = distributionZoneConfiguration.replicas().value();
-
-                                    int partId = part;
-
-                                    updatePendingAssignmentsKeys(
-                                            tableView.name(),
-                                            replicaGrpId,
-                                            filteredDataNodes,
-                                            replicas,
-                                            evt.entryEvent().newEntry().revision(),
-                                            metaStorageManager,
-                                            partId,
-                                            tableAssignments.isEmpty() ? Collections.emptySet() : tableAssignments.get(partId)
-                                    ).exceptionally(e -> {
-                                        LOG.error(
-                                                "Exception on updating assignments for [table={}, partition={}]", e, tableView.name(),
-                                                partId
-                                        );
-
-                                        return null;
-                                    });
+                        Set<Throwable> exceptions = newSetFromMap(new ConcurrentHashMap<>());
+                        for (int partId = 0; partId < partitionFutures.length; partId++) {
+                            int finalPartId = partId;
 
+                            partitionFutures[partId].exceptionally(e -> {
+                                if (exceptions.add(e)) {

Review Comment:
   Do we use exceptions set anywhere? Or it's used only to check whether given exception was previously added and thus skip log record duplication?



-- 
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 #2263: IGNITE-19843 Unify code that triggers rebalance

Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #2263:
URL: https://github.com/apache/ignite-3/pull/2263#discussion_r1246223324


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/rebalance/DistributionZoneRebalanceEngine.java:
##########
@@ -153,80 +149,48 @@ public CompletableFuture<Void> onUpdate(WatchEvent evt) {
                 }
 
                 try {
-                    byte[] dataNodesBytes = evt.entryEvent().newEntry().value();
+                    Set<Node> dataNodes = parseDataNodes(evt.entryEvent().newEntry().value());
 
-                    if (dataNodesBytes == null) {
+                    if (dataNodes == null) {
                         //The zone was removed so data nodes was removed too.
                         return completedFuture(null);
                     }
 
-                    NamedConfigurationTree<TableConfiguration, TableView, TableChange> tables = tablesConfiguration.tables();
-
                     int zoneId = extractZoneId(evt.entryEvent().newEntry().key());
 
-                    Set<Node> dataNodes = dataNodes(ByteUtils.fromBytes(dataNodesBytes));
-
-                    for (int i = 0; i < tables.value().size(); i++) {
-                        TableView tableView = tables.value().get(i);
+                    DistributionZoneView zoneConfig =
+                            getZoneById(zonesConfiguration, zoneId).value();
 
-                        int tableZoneId = tableView.zoneId();
+                    Set<String> filteredDataNodes = filterDataNodes(
+                            dataNodes,
+                            zoneConfig.filter(),
+                            distributionZoneManager.nodesAttributes()
+                    );
 
-                        DistributionZoneConfiguration distributionZoneConfiguration =
-                                getZoneById(zonesConfiguration, tableZoneId);
+                    if (filteredDataNodes.isEmpty()) {
+                        return completedFuture(null);
+                    }
 
-                        Set<String> filteredDataNodes = filterDataNodes(
-                                dataNodes,
-                                distributionZoneConfiguration.filter().value(),
-                                distributionZoneManager.nodesAttributes()
+                    for (TableView tableConfig : findTablesByZoneId(zoneId)) {
+                        CompletableFuture<?>[] partitionFutures = RebalanceUtil.triggerAllTablePartitionsRebalance(
+                                tableConfig,
+                                zoneConfig,
+                                filteredDataNodes,
+                                evt.entryEvent().newEntry().revision(),
+                                metaStorageManager
                         );
 
-                        if (filteredDataNodes.isEmpty()) {
-                            continue;
-                        }
-
-                        if (zoneId == tableZoneId) {
-                            TableConfiguration tableCfg = tables.get(tableView.name());
-
-                            int tableId = tableCfg.id().value();
-
-                            CompletableFuture<List<Set<Assignment>>> tableAssignmentsFut = tableAssignments(
-                                    metaStorageManager,
-                                    tableId,
-                                    distributionZoneConfiguration.partitions().value()
-                            );
-
-                            tableAssignmentsFut.thenAccept(tableAssignments -> {
-
-                                for (int part = 0; part < distributionZoneConfiguration.partitions().value(); part++) {
-
-                                    TablePartitionId replicaGrpId = new TablePartitionId(tableId, part);
-
-                                    int replicas = distributionZoneConfiguration.replicas().value();
-
-                                    int partId = part;
-
-                                    updatePendingAssignmentsKeys(
-                                            tableView.name(),
-                                            replicaGrpId,
-                                            filteredDataNodes,
-                                            replicas,
-                                            evt.entryEvent().newEntry().revision(),
-                                            metaStorageManager,
-                                            partId,
-                                            tableAssignments.isEmpty() ? Collections.emptySet() : tableAssignments.get(partId)
-                                    ).exceptionally(e -> {
-                                        LOG.error(
-                                                "Exception on updating assignments for [table={}, partition={}]", e, tableView.name(),
-                                                partId
-                                        );
-
-                                        return null;
-                                    });
+                        Set<Throwable> exceptions = newSetFromMap(new ConcurrentHashMap<>());
+                        for (int partId = 0; partId < partitionFutures.length; partId++) {
+                            int finalPartId = partId;
 
+                            partitionFutures[partId].exceptionally(e -> {
+                                if (exceptions.add(e)) {

Review Comment:
   Do we use exceptions set anywhere? Or it's used only to check whether given exception was previously added and thus skip log record duplication? If the latter is correct please add corresponding comment or even extract it to the helper method.



-- 
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 #2263: IGNITE-19843 Unify code that triggers rebalance

Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #2263:
URL: https://github.com/apache/ignite-3/pull/2263#discussion_r1246223324


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/rebalance/DistributionZoneRebalanceEngine.java:
##########
@@ -153,80 +149,48 @@ public CompletableFuture<Void> onUpdate(WatchEvent evt) {
                 }
 
                 try {
-                    byte[] dataNodesBytes = evt.entryEvent().newEntry().value();
+                    Set<Node> dataNodes = parseDataNodes(evt.entryEvent().newEntry().value());
 
-                    if (dataNodesBytes == null) {
+                    if (dataNodes == null) {
                         //The zone was removed so data nodes was removed too.
                         return completedFuture(null);
                     }
 
-                    NamedConfigurationTree<TableConfiguration, TableView, TableChange> tables = tablesConfiguration.tables();
-
                     int zoneId = extractZoneId(evt.entryEvent().newEntry().key());
 
-                    Set<Node> dataNodes = dataNodes(ByteUtils.fromBytes(dataNodesBytes));
-
-                    for (int i = 0; i < tables.value().size(); i++) {
-                        TableView tableView = tables.value().get(i);
+                    DistributionZoneView zoneConfig =
+                            getZoneById(zonesConfiguration, zoneId).value();
 
-                        int tableZoneId = tableView.zoneId();
+                    Set<String> filteredDataNodes = filterDataNodes(
+                            dataNodes,
+                            zoneConfig.filter(),
+                            distributionZoneManager.nodesAttributes()
+                    );
 
-                        DistributionZoneConfiguration distributionZoneConfiguration =
-                                getZoneById(zonesConfiguration, tableZoneId);
+                    if (filteredDataNodes.isEmpty()) {
+                        return completedFuture(null);
+                    }
 
-                        Set<String> filteredDataNodes = filterDataNodes(
-                                dataNodes,
-                                distributionZoneConfiguration.filter().value(),
-                                distributionZoneManager.nodesAttributes()
+                    for (TableView tableConfig : findTablesByZoneId(zoneId)) {
+                        CompletableFuture<?>[] partitionFutures = RebalanceUtil.triggerAllTablePartitionsRebalance(
+                                tableConfig,
+                                zoneConfig,
+                                filteredDataNodes,
+                                evt.entryEvent().newEntry().revision(),
+                                metaStorageManager
                         );
 
-                        if (filteredDataNodes.isEmpty()) {
-                            continue;
-                        }
-
-                        if (zoneId == tableZoneId) {
-                            TableConfiguration tableCfg = tables.get(tableView.name());
-
-                            int tableId = tableCfg.id().value();
-
-                            CompletableFuture<List<Set<Assignment>>> tableAssignmentsFut = tableAssignments(
-                                    metaStorageManager,
-                                    tableId,
-                                    distributionZoneConfiguration.partitions().value()
-                            );
-
-                            tableAssignmentsFut.thenAccept(tableAssignments -> {
-
-                                for (int part = 0; part < distributionZoneConfiguration.partitions().value(); part++) {
-
-                                    TablePartitionId replicaGrpId = new TablePartitionId(tableId, part);
-
-                                    int replicas = distributionZoneConfiguration.replicas().value();
-
-                                    int partId = part;
-
-                                    updatePendingAssignmentsKeys(
-                                            tableView.name(),
-                                            replicaGrpId,
-                                            filteredDataNodes,
-                                            replicas,
-                                            evt.entryEvent().newEntry().revision(),
-                                            metaStorageManager,
-                                            partId,
-                                            tableAssignments.isEmpty() ? Collections.emptySet() : tableAssignments.get(partId)
-                                    ).exceptionally(e -> {
-                                        LOG.error(
-                                                "Exception on updating assignments for [table={}, partition={}]", e, tableView.name(),
-                                                partId
-                                        );
-
-                                        return null;
-                                    });
+                        Set<Throwable> exceptions = newSetFromMap(new ConcurrentHashMap<>());
+                        for (int partId = 0; partId < partitionFutures.length; partId++) {
+                            int finalPartId = partId;
 
+                            partitionFutures[partId].exceptionally(e -> {
+                                if (exceptions.add(e)) {

Review Comment:
   Do we use exceptions set anywhere? 



-- 
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] rpuch commented on a diff in pull request #2263: IGNITE-19843 Unify code that triggers rebalance

Posted by "rpuch (via GitHub)" <gi...@apache.org>.
rpuch commented on code in PR #2263:
URL: https://github.com/apache/ignite-3/pull/2263#discussion_r1246418299


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/rebalance/DistributionZoneRebalanceEngine.java:
##########
@@ -153,80 +149,48 @@ public CompletableFuture<Void> onUpdate(WatchEvent evt) {
                 }
 
                 try {
-                    byte[] dataNodesBytes = evt.entryEvent().newEntry().value();
+                    Set<Node> dataNodes = parseDataNodes(evt.entryEvent().newEntry().value());
 
-                    if (dataNodesBytes == null) {
+                    if (dataNodes == null) {
                         //The zone was removed so data nodes was removed too.
                         return completedFuture(null);
                     }
 
-                    NamedConfigurationTree<TableConfiguration, TableView, TableChange> tables = tablesConfiguration.tables();
-
                     int zoneId = extractZoneId(evt.entryEvent().newEntry().key());
 
-                    Set<Node> dataNodes = dataNodes(ByteUtils.fromBytes(dataNodesBytes));
-
-                    for (int i = 0; i < tables.value().size(); i++) {
-                        TableView tableView = tables.value().get(i);
+                    DistributionZoneView zoneConfig =
+                            getZoneById(zonesConfiguration, zoneId).value();
 
-                        int tableZoneId = tableView.zoneId();
+                    Set<String> filteredDataNodes = filterDataNodes(
+                            dataNodes,
+                            zoneConfig.filter(),
+                            distributionZoneManager.nodesAttributes()
+                    );
 
-                        DistributionZoneConfiguration distributionZoneConfiguration =
-                                getZoneById(zonesConfiguration, tableZoneId);
+                    if (filteredDataNodes.isEmpty()) {
+                        return completedFuture(null);
+                    }
 
-                        Set<String> filteredDataNodes = filterDataNodes(
-                                dataNodes,
-                                distributionZoneConfiguration.filter().value(),
-                                distributionZoneManager.nodesAttributes()
+                    for (TableView tableConfig : findTablesByZoneId(zoneId)) {
+                        CompletableFuture<?>[] partitionFutures = RebalanceUtil.triggerAllTablePartitionsRebalance(
+                                tableConfig,
+                                zoneConfig,
+                                filteredDataNodes,
+                                evt.entryEvent().newEntry().revision(),
+                                metaStorageManager
                         );
 
-                        if (filteredDataNodes.isEmpty()) {
-                            continue;
-                        }
-
-                        if (zoneId == tableZoneId) {
-                            TableConfiguration tableCfg = tables.get(tableView.name());
-
-                            int tableId = tableCfg.id().value();
-
-                            CompletableFuture<List<Set<Assignment>>> tableAssignmentsFut = tableAssignments(
-                                    metaStorageManager,
-                                    tableId,
-                                    distributionZoneConfiguration.partitions().value()
-                            );
-
-                            tableAssignmentsFut.thenAccept(tableAssignments -> {
-
-                                for (int part = 0; part < distributionZoneConfiguration.partitions().value(); part++) {
-
-                                    TablePartitionId replicaGrpId = new TablePartitionId(tableId, part);
-
-                                    int replicas = distributionZoneConfiguration.replicas().value();
-
-                                    int partId = part;
-
-                                    updatePendingAssignmentsKeys(
-                                            tableView.name(),
-                                            replicaGrpId,
-                                            filteredDataNodes,
-                                            replicas,
-                                            evt.entryEvent().newEntry().revision(),
-                                            metaStorageManager,
-                                            partId,
-                                            tableAssignments.isEmpty() ? Collections.emptySet() : tableAssignments.get(partId)
-                                    ).exceptionally(e -> {
-                                        LOG.error(
-                                                "Exception on updating assignments for [table={}, partition={}]", e, tableView.name(),
-                                                partId
-                                        );
-
-                                        return null;
-                                    });
+                        Set<Throwable> exceptions = newSetFromMap(new ConcurrentHashMap<>());
+                        for (int partId = 0; partId < partitionFutures.length; partId++) {
+                            int finalPartId = partId;
 
+                            partitionFutures[partId].exceptionally(e -> {
+                                if (exceptions.add(e)) {

Review Comment:
   I added a couple of comments to clarify the situation



-- 
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] ibessonov merged pull request #2263: IGNITE-19843 Unify code that triggers rebalance

Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov merged PR #2263:
URL: https://github.com/apache/ignite-3/pull/2263


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