You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@beam.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2022/04/05 07:55:00 UTC

[jira] [Work logged] (BEAM-14244) Processing time timers should use outputTimestamp rather than input watermark for their timestamp

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

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

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


##########
runners/core-java/src/main/java/org/apache/beam/runners/core/SimpleDoFnRunner.java:
##########
@@ -209,7 +209,12 @@ public void processElement(WindowedValue<InputT> compressedElem) {
         break;
       case PROCESSING_TIME:
       case SYNCHRONIZED_PROCESSING_TIME:
-        effectiveTimestamp = stepContext.timerInternals().currentInputWatermarkTime();
+        Instant outputWatermark = stepContext.timerInternals().currentOutputWatermarkTime();
+        Instant inputWatermark = stepContext.timerInternals().currentInputWatermarkTime();
+        effectiveTimestamp =
+            outputTimestamp != null
+                ? outputTimestamp
+                : outputWatermark != null ? outputWatermark : inputWatermark;

Review Comment:
   I'm confused. There is a comment in the mentioned `setAndVerifyOutputTimestamp`, that states:
   ```java
        * <ul>
        *   Ensures that:
        *   <li>Users can't set {@code outputTimestamp} for processing time timers.
        *   <li>Event time timers' {@code outputTimestamp} is set before window expiration.
        * </ul>
   ```
   
   The method does not check the first part apparently. If this is correct we might want to fix the comment.





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

            Worklog Id:     (was: 752691)
    Remaining Estimate: 0h
            Time Spent: 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: 10m
>  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)