rpuch commented on code in PR #4649:
URL: https://github.com/apache/ignite-3/pull/4649#discussion_r1822587848
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -2189,19 +2190,20 @@ private CompletableFuture<Void>
handleChangePendingAssignmentEvent(
localServicesStartFuture = localPartitionsVv.get(revision)
// TODO https://issues.apache.org/jira/browse/IGNITE-20957
Revisit this code
.thenComposeAsync(
- partitionSet -> inBusyLock(busyLock, () ->
getOrCreatePartitionStorages(tbl, singlePartitionIdSet)),
+ unused -> inBusyLock(
+ busyLock,
+ () -> getOrCreatePartitionStorages(tbl,
singlePartitionIdSet)
+ .thenRun(() ->
localPartsByTableId.compute(
+ replicaGrpId.tableId(),
+ (tableId, oldPartitionSet)
-> extendPartitionSet(oldPartitionSet, partitionId)
+ ))
+ ),
ioExecutor
)
.thenComposeAsync(unused -> inBusyLock(busyLock, () -> {
- if (!isRecovery) {
Review Comment:
Judging by the comment, the author thought that, during node start (aka
recovery), to the moment the pending assignment changes start being applied,
the current assignments set is already recovered. It turned out that this is
not true: during recovery, some previous assignments set is picked up from the
Metastorage, and then missing pending assignment changes are applied on top. So
this `if` was actually breaking the rebalance recovery (namely,
`ItRebalanceRecoveryTest`)
--
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]
