You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "sergeyuttsel (via GitHub)" <gi...@apache.org> on 2023/04/26 13:19:10 UTC

[GitHub] [ignite-3] sergeyuttsel opened a new pull request, #1986: IGNITE-18755 Do not save the empty data nodes list to avoid the distribution zone without a data nodes.

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

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


-- 
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 #1986: IGNITE-18755 Do not save the empty data nodes list to avoid the distribution zone without a data nodes. Fixed exceptions in test logs of DistributionZoneManagerScaleUpTest, DistributionZoneManagerConfigurationChangesTest, DistributionZoneManagerWatchListenerTest.

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


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##########
@@ -1595,6 +1598,11 @@ CompletableFuture<Void> saveDataNodesToMetaStorageOnScaleDown(int zoneId, long r
                         .thenApply(StatementResult::getAsBoolean)
                         .thenCompose(invokeResult -> inBusyLock(busyLock, () -> {
                             if (invokeResult) {
+                                if (newDataNodes.isEmpty()) {

Review Comment:
   How it's supposed to work?
   Let's say that we've prepared new dataNodesAndTriggerKeyUpd with new revision and **same** newDataNodesBytes.
   Are we going to actually update dataNodes with same value? Are we going to consider such same to same change as new rebalance trigger?



-- 
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 #1986: IGNITE-18755 Do not save the empty data nodes list to avoid the distribution zone without a data nodes. Fixed exceptions in test logs of DistributionZoneManagerScaleUpTest, DistributionZoneManagerConfigurationChangesTest, DistributionZoneManagerWatchListenerTest.

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


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##########
@@ -1595,6 +1598,11 @@ CompletableFuture<Void> saveDataNodesToMetaStorageOnScaleDown(int zoneId, long r
                         .thenApply(StatementResult::getAsBoolean)
                         .thenCompose(invokeResult -> inBusyLock(busyLock, () -> {
                             if (invokeResult) {
+                                if (newDataNodes.isEmpty()) {
+                                    LOG.debug("Data nodes for a zone was not updated because"

Review Comment:
   Should we still update trigger 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] sergeyuttsel closed pull request #1986: IGNITE-18755 Do not save the empty data nodes list to avoid the distribution zone without a data nodes. Fixed exceptions in test logs of DistributionZoneManagerScaleUpTest, DistributionZoneManagerConfigurationChangesTest, DistributionZoneManagerWatchListenerTest.

Posted by "sergeyuttsel (via GitHub)" <gi...@apache.org>.
sergeyuttsel closed pull request #1986: IGNITE-18755 Do not save the empty data nodes list to avoid the distribution zone without a data nodes. Fixed exceptions in test logs of DistributionZoneManagerScaleUpTest, DistributionZoneManagerConfigurationChangesTest, DistributionZoneManagerWatchListenerTest.
URL: https://github.com/apache/ignite-3/pull/1986


-- 
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 #1986: IGNITE-18755 Do not save the empty data nodes list to avoid the distribution zone without a data nodes. Fixed exceptions in test logs of DistributionZoneManagerScaleUpTest, DistributionZoneManagerConfigurationChangesTest, DistributionZoneManagerWatchListenerTest.

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


##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##########
@@ -1595,6 +1598,11 @@ CompletableFuture<Void> saveDataNodesToMetaStorageOnScaleDown(int zoneId, long r
                         .thenApply(StatementResult::getAsBoolean)
                         .thenCompose(invokeResult -> inBusyLock(busyLock, () -> {
                             if (invokeResult) {
+                                if (newDataNodes.isEmpty()) {

Review Comment:
   1. Same to same a data nodes change will not trigger a new rebalance because RebalanceUtil#updatePendingAssignmentsKeys filters the same assignments.
   2. I think we can not save the same data nodes, but it will require more changes in the code. Actually a meta storage invoke with the same data nodes can not complete successfully and `saveDataNodesToMetaStorageOnScaleDown` can be repeated again but with a different set of nodes.



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