[GitHub] [kafka] mjsax commented on a change in pull request #10407: KAFKA-12523: handle TaskCorruption for revoked tasks & remove commit in handleCorruption

[GitBox <gi...@apache.org>]

mjsax commented on a change in pull request #10407:
URL: https://github.com/apache/kafka/pull/10407#discussion_r602581215



##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/TaskManager.java
##########
@@ -167,18 +167,7 @@ void handleCorruption(final Set<TaskId> corruptedTasks) {
             }
         }
 
-        // Make sure to clean up any corrupted standby tasks in their entirety before committing
-        // since TaskMigrated can be thrown and the resulting handleLostAll will only clean up active tasks
         closeAndRevive(corruptedStandbyTasks);
-
-        commit(tasks()

Review comment:
       I am fine with refactoring. On the other hand, does it add even more "corner" case and make the code harder to reason about?
   
   I "simpler" solution might be to just risk that the commit fails (I think be risk is low), and if it indeed fails, we throw a `TaskMigratedException` that is already handled in the code.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org