scwhittle commented on code in PR #35050:
URL: https://github.com/apache/beam/pull/35050#discussion_r2108499847


##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnApiDoFnRunner.java:
##########
@@ -1533,6 +1533,8 @@ private <K> void processTimerDirect(
     currentTimer = timer;
     currentTimeDomain = timeDomain;
     doFnInvoker.invokeOnTimer(timerId, timerFamilyId, onTimerContext);
+    // Finalize state to ensure metrics and other state changes are committed.
+    this.stateAccessor.finalizeState();

Review Comment:
   this is going to result in finalizeState being called multiple times if 
there are multiple timers.
   
   But it also seems that this shouldn't be needed because finalizeState is 
already called in finishBundle.  Is that not being called for some reason?  The 
reported bug is old, have you verified that it is still present? If so adding a 
test that fails without a fix would be good.



-- 
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: github-unsubscr...@beam.apache.org

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

Reply via email to