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

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

                Author: ASF GitHub Bot
            Created on: 07/Apr/22 18:01
            Start Date: 07/Apr/22 18:01
    Worklog Time Spent: 10m 
      Work Description: lukecwik commented on code in PR #17262:
URL: https://github.com/apache/beam/pull/17262#discussion_r845426878


##########
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:
   +1 on their being two different timestamps, the firing timestamp and the 
output timestamp.
   
   The firing timestamp can be in the processing time domain or the event time 
domain.
   The output timestamp is always in the event time domain and is used to hold 
the watermark.
   
   For event time timers, the output timestamp being equivalent to the firing 
time was expected to be a natural default since users would want to produce 
data with the same output timestamp as the firing timestamp but users could 
still override it by setting the output timestamp separately from the firing 
timestamp.
   
   For processing time timers, the output timestamp can't rely on the firing 
time as it doesn't make sense as a default since it represents a different time 
domain. This is why the output timestamp defaults to the input element/timer 
timestamp and can be overridden to a different output timestamp.
   
   Also, any scheduled timers should have an input timestamp equal to the 
output timestamp that was set allowing for new timers that are scheduled from 
the timer callback to get reasonable defaults as well.





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

    Worklog Id:     (was: 754268)
    Time Spent: 9h 50m  (was: 9h 40m)

> 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
>             Fix For: 2.39.0
>
>          Time Spent: 9h 50m
>  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