rpuch commented on code in PR #2316:
URL: https://github.com/apache/ignite-3/pull/2316#discussion_r1263400164
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -909,27 +909,27 @@ private CompletableFuture<?> createTablePartitionsLocally(
return allOf(futures);
};
- return localPartsByTableIdVv.update(causalityToken, (previous,
throwable) -> inBusyLock(busyLock, () -> {
- return getOrCreatePartitionStorages(table, parts).thenApply(u -> {
- var newValue = new HashMap<>(previous);
+ // NB: all vv.update() calls must be made from the synchronous part of
the method (not in thenCompose()/etc!).
+ CompletableFuture<?> localPartsUpdateFuture =
localPartsByTableIdVv.update(causalityToken,
+ (previous, throwable) -> inBusyLock(busyLock, () -> {
+ return getOrCreatePartitionStorages(table,
parts).thenApply(u -> {
+ var newValue = new HashMap<>(previous);
- newValue.put(tableId, parts);
+ newValue.put(tableId, parts);
- return newValue;
- });
- })).thenCompose(unused -> {
- CompletableFuture<Void> updateAssignmentsFuture =
tablesByIdVv.get(causalityToken).thenComposeAsync(
- tablesById -> inBusyLock(busyLock,
updateAssignmentsClosure),
- ioExecutor
- );
+ return newValue;
+ });
+ }));
- return assignmentsUpdatedVv.update(causalityToken, (token, e) -> {
- if (e != null) {
- return failedFuture(e);
- }
+ return assignmentsUpdatedVv.update(causalityToken, (token, e) -> {
+ if (e != null) {
+ return failedFuture(e);
+ }
- return updateAssignmentsFuture;
- });
+ return localPartsUpdateFuture.thenCompose(unused ->
Review Comment:
If I change the code to the following
```
return assignmentsUpdatedVv.update(causalityToken, (token, e) -> {
if (e != null) {
return failedFuture(e);
}
return tablesByIdVv.get(causalityToken)
.thenComposeAsync(tablesById -> inBusyLock(busyLock,
updateAssignmentsClosure), ioExecutor);
});
```
, I get an assertion:
```
Caused by: java.lang.AssertionError
at
org.apache.ignite.internal.table.distributed.TableManager.getPartitionStorages(TableManager.java:2502)
at
org.apache.ignite.internal.table.distributed.TableManager.lambda$createTablePartitionsLocally$13(TableManager.java:763)
at
org.apache.ignite.internal.util.IgniteUtils.inBusyLock(IgniteUtils.java:893)
at
org.apache.ignite.internal.table.distributed.TableManager.lambda$createTablePartitionsLocally$17(TableManager.java:930)
at
java.base/java.util.concurrent.CompletableFuture$UniCompose.tryFire(CompletableFuture.java:1072)
... 4 more
```
which means that `getOrCreatePartitionStorages()` is not finished yet.
Also, `tablesByIdVv.get(causalityToken)` is not done when
`assignmentsUpdatedVv`-related callback is called, so the assertion cannot be
added.
--
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]
