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

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

                Author: ASF GitHub Bot
            Created on: 05/Apr/22 12:46
            Start Date: 05/Apr/22 12:46
    Worklog Time Spent: 10m 
      Work Description: steveniemitz commented on code in PR #17262:
URL: https://github.com/apache/beam/pull/17262#discussion_r842741457


##########
runners/flink/src/test/java/org/apache/beam/runners/flink/translation/wrappers/streaming/DoFnOperatorTest.java:
##########
@@ -305,7 +305,10 @@ public void processElement(
             eventTimerWithOutputTimestamp
                 .withOutputTimestamp(timerOutputTimestamp)
                 .set(timerTimestamp);
-            
processingTimer.offset(Duration.millis(timerTimestamp.getMillis())).setRelative();
+            processingTimer

Review Comment:
   > this seems inconsistent with how event-time timers work, which set output 
timestamp based on the fire timestamp (by default). Do we have a reason for 
such a difference?
   
   My guess would be that "firing timestamp" is in a different time domain 
here, so element input timestamp is the only thing that can be used that's in 
the event time domain to set the hold on.
   
   > And what about looping processing-time timers, where there is actually no 
element?
   
   The output timestamp of the timer (the timer executing the OnTimer callback) 
is 
[used](https://github.com/apache/beam/blob/master/runners/core-java/src/main/java/org/apache/beam/runners/core/SimpleDoFnRunner.java#L674).
   
   I actually do agree that the default being the output watermark if the 
output timestamp on the timer is unset makes more sense.  This would also 
remove the need for a watermark hold at all, and remove a source of confusion 
around processing time timers.  It seems like it'd be a larger change though.





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

    Worklog Id:     (was: 752853)
    Time Spent: 1h 10m  (was: 1h)

> Processing time timers should use outputTimestamp rather than input watermark 
> for their timestamp
> -------------------------------------------------------------------------------------------------
>
>                 Key: BEAM-14244
>                 URL: https://issues.apache.org/jira/browse/BEAM-14244
>             Project: Beam
>          Issue Type: Bug
>          Components: runner-core, sdk-java-core
>            Reporter: Steve Niemitz
>            Assignee: Steve Niemitz
>            Priority: P1
>          Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> Currently processing time timers ignore the outputTimestamp and instead use 
> the input watermark at the time they fire.  This is wrong because the input 
> watermark can have advanced arbitrarily far past the actual output timestamp 
> when it fires.
> The correct behavior should be to instead use the outputTimestamp the timer 
> was configured to fire with.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to