pnowojski commented on a change in pull request #16744:
URL: https://github.com/apache/flink/pull/16744#discussion_r685200674
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/ProcessingTimeServiceUtil.java
##########
@@ -34,9 +34,15 @@
*/
public static long getProcessingTimeDelay(long processingTimestamp, long
currentTimestamp) {
- // delay the firing of the timer by 1 ms to align the semantics with
watermark. A watermark
- // T says we won't see elements in the future with a timestamp smaller
or equal to T.
- // With processing time, we therefore need to delay firing the timer
by one ms.
- return Math.max(processingTimestamp - currentTimestamp, 0) + 1;
+ // Two cases of timers here:
+ // (1) future/now timers(processingTimestamp >= currentTimestamp):
delay the firing of the
+ // timer by 1 ms to align the semantics with watermark. A watermark
T says we won't see
+ // elements in the future with a timestamp smaller or equal to T.
With processing time, we
+ // therefore need to delay firing the timer by one ms.
+ // (2) past timers(processingTimestamp < currentTimestamp): do not
need to delay the firing
+ // because currentTimestamp is larger than processingTimestamp
pluses the 1ms offset.
+ // TODO. The processing timers' performance can be further improved.
+ // see FLINK-23690 and https://github.com/apache/flink/pull/16744
+ return Math.max(processingTimestamp - currentTimestamp, -1) + 1;
Review comment:
Can we expand this to an if check as you are describing in the comment?
```
if (processingTimestamp >= currentTimestamp) {
return processingTimestamp - currentTimestamp + 1;
}
else {
return 0;
}
```
I think it would be easier to read.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
