[ 
https://issues.apache.org/jira/browse/BEAM-1589?focusedWorklogId=434315&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-434315
 ]

ASF GitHub Bot logged work on BEAM-1589:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 18/May/20 07:22
            Start Date: 18/May/20 07:22
    Worklog Time Spent: 10m 
      Work Description: reuvenlax commented on a change in pull request #11350:
URL: https://github.com/apache/beam/pull/11350#discussion_r426413338



##########
File path: 
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingModeExecutionContext.java
##########
@@ -591,7 +595,7 @@ public void flushState() {
               timerId,
               "",
               cleanupTime,
-              cleanupTime,
+              cleanupTime.minus(1L),

Review comment:
       I want to avoid the performance regression of setting a watermark hold 
if not needed.  Make the outputTimestamp be a parameter to this function, and 
pass it in only if the DoFn has an onWindowExpiration (SimpleParDoFn has the 
DoFnSignature so you should be able to tell there).

##########
File path: 
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingModeExecutionContext.java
##########
@@ -552,7 +552,11 @@ public void flushState() {
       if (!cachedFiredTimers.hasNext()) {
         return null;
       }
-      return cachedFiredTimers.next();
+      TimerData nextTimer = cachedFiredTimers.next();
+      // system timers ( GC timer) must be explicitly deleted when delivered, 
to release the implied
+      // hold.
+      systemTimerInternals.deleteTimer(nextTimer);

Review comment:
       I think we should only do this if there is a hold to delete.




----------------------------------------------------------------
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:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 434315)
    Time Spent: 11h 10m  (was: 11h)

> Add OnWindowExpiration method to Stateful DoFn
> ----------------------------------------------
>
>                 Key: BEAM-1589
>                 URL: https://issues.apache.org/jira/browse/BEAM-1589
>             Project: Beam
>          Issue Type: New Feature
>          Components: runner-core, sdk-java-core
>            Reporter: Jingsong Lee
>            Assignee: Rehman Murad Ali
>            Priority: P2
>          Time Spent: 11h 10m
>  Remaining Estimate: 0h
>
> See BEAM-1517
> This allows the user to do some work before the state's garbage collection.
> It seems kind of annoying, but on the other hand forgetting to set a final 
> timer to flush state is probably data loss most of the time.
> FlinkRunner does this work very simply, but other runners, such as 
> DirectRunner, need to traverse all the states to do this, and maybe it's a 
> little hard.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to