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.
--
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.
To unsubscribe, e-mail: github-unsubscr...@beam.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
