mjsax commented on code in PR #19450:
URL: https://github.com/apache/kafka/pull/19450#discussion_r2048191835


##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StandbyTask.java:
##########
@@ -179,7 +179,11 @@ public void resume() {
      *                          or flushing state store get IO errors; such 
error should cause the thread to die
      */
     @Override
-    public Map<TopicPartition, OffsetAndMetadata> prepareCommit() {
+    public Map<TopicPartition, OffsetAndMetadata> prepareCommit(final boolean 
clean) {
+        if (!clean) {
+            log.warn("Skipped preparing {} standby task with id {} for commit 
since the task is getting closed dirty.", state(), id);

Review Comment:
   Why do we need to log this at WARN level?



##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamTask.java:
##########
@@ -444,6 +443,10 @@ public Map<TopicPartition, OffsetAndMetadata> 
prepareCommit() {
                     //
                     // TODO: this should be removed after we decouple caching 
with emitting
                     flush();
+                    if (!clean) {
+                        log.warn("Skipped preparing {} task with id {} for 
commit since the task is getting closed dirty.", state(), id);

Review Comment:
   as above -- should be be DEBUG ?



##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StandbyTask.java:
##########
@@ -179,7 +179,11 @@ public void resume() {
      *                          or flushing state store get IO errors; such 
error should cause the thread to die
      */
     @Override
-    public Map<TopicPartition, OffsetAndMetadata> prepareCommit() {
+    public Map<TopicPartition, OffsetAndMetadata> prepareCommit(final boolean 
clean) {
+        if (!clean) {

Review Comment:
   Should this to to the end of the method? So we preserve all logging?
   
   Or maybe even don't check `clean` flag at all, because StandbyTasks should 
also never return offsets, and we could just change `return 
Collections.emptyMap();` to `return null;` directly?



-- 
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: jira-unsubscr...@kafka.apache.org

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

Reply via email to