Cyrill commented on code in PR #4688: URL: https://github.com/apache/ignite-3/pull/4688#discussion_r1851965589
########## modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/GroupUpdateRequest.java: ########## @@ -254,42 +292,60 @@ private static CompletableFuture<Integer> partitionUpdate( return completedFuture(ASSIGNMENT_NOT_UPDATED.ordinal()); } - Iif invokeClosure; - - if (aliveStableNodes.isEmpty()) { - if (!manualUpdate) { - return completedFuture(ASSIGNMENT_NOT_UPDATED.ordinal()); - } + if (aliveStableNodes.isEmpty() && !manualUpdate) { + return completedFuture(ASSIGNMENT_NOT_UPDATED.ordinal()); + } + if (manualUpdate) { enrichAssignments(partId, aliveDataNodes, replicas, partAssignments); + } - // There are no known nodes with data, which means that we can just put new assignments into pending assignments with "forced" - // flag. - invokeClosure = prepareMsInvokeClosure( - partId, - longToBytesKeepingOrder(revision), - Assignments.forced(partAssignments, assignmentsTimestamp).toBytes(), - null - ); - } else { - Set<Assignment> stableAssignments = Set.copyOf(partAssignments); + Assignment nextAssignment = aliveStableNodes.isEmpty() + ? nextAssignment(partAssignments) + : nextAssignment(localPartitionStateMessageByNode, partAssignments); + + // There are nodes with data, and we set pending assignments to this set of nodes. It'll be the source of peers for + // "resetPeers", and after that new assignments with restored replica factor wil be picked up from planned assignments + // for the case of the manual update, that was triggered by a user. + Iif invokeClosure = prepareMsInvokeClosure( + partId, + longToBytesKeepingOrder(revision), + Assignments.forced(Set.of(nextAssignment), assignmentsTimestamp).toBytes(), + Assignments.toBytes(partAssignments, assignmentsTimestamp) + ); - if (manualUpdate) { - enrichAssignments(partId, aliveDataNodes, replicas, partAssignments); - } + return metaStorageMgr.invoke(invokeClosure).thenApply(StatementResult::getAsInt); + } - // There are nodes with data, and we set pending assignments to this set of nodes. It'll be the source of peers for - // "resetPeers", and after that new assignments with restored replica factor wil be picked up from planned assignments - // for the case of the manual update, that was triggered by a user. - invokeClosure = prepareMsInvokeClosure( - partId, - longToBytesKeepingOrder(revision), - Assignments.forced(stableAssignments, assignmentsTimestamp).toBytes(), - Assignments.toBytes(partAssignments, assignmentsTimestamp) - ); - } + /** + * Returns the first assignment in the lexicographic order. + */ + private static Assignment nextAssignment(Set<Assignment> assignments) { + assert !assignments.isEmpty() : "Alive nodes with data should not be empty"; Review Comment: Changed this. Added todo with IGNITE-23737 to cover the case when there are no available nodes at all -- 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org