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

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

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


##########
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:
   It seems that this change was actually introduced later. Which is why the 
comment on `setAndVerifyOutputTimestamp` says that processing-time timers 
should not have output timestamp.
   
   > 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.
   
   The different time domain relates to *when* the timer should fire. The 
default output timestamp can be (and I think it should be) consistent in both 
cases - it is the position in *event-time* in both cases. In the case of 
event-time timer that is the firing timestamp, in the case of processing-time 
timer it is the output watermark.
   
   Agree that the change to make this consistent would be a larger work, but we 
should know what the correct behavior should be and if we actually have runners 
that violate the model.





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

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

> 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 20m
>  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