vdmitrienko commented on code in PR #7064:
URL: https://github.com/apache/ignite-3/pull/7064#discussion_r2561000517


##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexBuildTask.java:
##########
@@ -221,9 +235,18 @@ private CompletableFuture<Void> handleNextBatch(@Nullable 
RowId highestRowId) {
 
         try {
             return createBatchToIndex(highestRowId)
-                    .thenCompose(batch -> {
-                        return replicaService.invoke(node, 
createBuildIndexReplicaRequest(batch, initialOperationTimestamp));
-                    })
+                    .thenCompose(batch ->
+                            replicaService.invoke(node, 
createBuildIndexReplicaRequest(batch, initialOperationTimestamp))
+                                    .whenComplete((unused, throwable) -> {
+                                        if (throwable == null) {
+                                            
taskListener.onRaftCallSuccess(taskId);
+                                        } else {
+                                            
taskListener.onRaftCallFailure(taskId);
+                                        }
+                                    })
+                                    .thenApply(unused -> batch)
+                    )
+                    .thenAccept(batch -> taskListener.onBatchProcessed(taskId, 
batch.rowIds.size()))

Review Comment:
   I chose this approach because it allows to differentiate between failures in 
`createBatchToIndex(...)` and `replicaService.invoke(...)` and that allows to 
calculate the correct number of successful and unsuccessful RAFT calls.
   
   In this case, if failure occurs in `createBatchToIndex()`, it's hard to 
calculate correct statistics.
   ```
   createBatchToIndex()
   .thenCompose(...) // RAFT call
   .whenComplete(...) // Need to understand what failed: createBatchToIndex() 
or RAFT call.
   .thenAccept
   ``` 



##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexBuildTask.java:
##########
@@ -221,9 +235,18 @@ private CompletableFuture<Void> handleNextBatch(@Nullable 
RowId highestRowId) {
 
         try {
             return createBatchToIndex(highestRowId)
-                    .thenCompose(batch -> {
-                        return replicaService.invoke(node, 
createBuildIndexReplicaRequest(batch, initialOperationTimestamp));
-                    })
+                    .thenCompose(batch ->
+                            replicaService.invoke(node, 
createBuildIndexReplicaRequest(batch, initialOperationTimestamp))
+                                    .whenComplete((unused, throwable) -> {
+                                        if (throwable == null) {
+                                            
taskListener.onRaftCallSuccess(taskId);
+                                        } else {
+                                            
taskListener.onRaftCallFailure(taskId);
+                                        }
+                                    })
+                                    .thenApply(unused -> batch)
+                    )
+                    .thenAccept(batch -> taskListener.onBatchProcessed(taskId, 
batch.rowIds.size()))

Review Comment:
   I chose this approach because it allows to differentiate between failures in 
`createBatchToIndex(...)` and `replicaService.invoke(...)`, and that allows to 
calculate the correct number of successful and unsuccessful RAFT calls.
   
   In this case, if failure occurs in `createBatchToIndex()`, it's hard to 
calculate correct statistics.
   ```
   createBatchToIndex()
   .thenCompose(...) // RAFT call
   .whenComplete(...) // Need to understand what failed: createBatchToIndex() 
or RAFT call.
   .thenAccept
   ``` 



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

Reply via email to