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

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

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


##########
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 to me that the test with regard to the watermark contract makes 
little sense for processing time timers. If we want to keep the test, can we 
change the validation to verify that the output timestamp is equal to output 
watermark instead of explicitly setting the output timestamp?
   
   I must say I'm not sure how to interpret user defined `withOutputTimestamp` 
for processing time timers (and even less for relative timers as this one). 
There is no relation between the two time domains and processing time timers do 
not hold output watermark, that seems to be very error prone. cc @reuvenlax 





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

    Worklog Id:     (was: 752693)
    Time Spent: 20m  (was: 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: 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