laraschmidt commented on a change in pull request #15540:
URL: https://github.com/apache/beam/pull/15540#discussion_r714358223
##########
File path:
runners/core-java/src/main/java/org/apache/beam/runners/core/SimpleDoFnRunner.java
##########
@@ -1190,12 +1190,12 @@ public Timer withOutputTimestamp(Instant
outputTimestamp) {
* </ul>
*/
private void setAndVerifyOutputTimestamp() {
-
if (outputTimestamp != null) {
checkArgument(
- !outputTimestamp.isBefore(elementInputTimestamp),
Review comment:
So I ran the new validates runner tests against FnApiDoFnRunner. Turns
out it was not properly accounting for DoFN skew when setting timers. But it
wasn't doing any checking on timestamps at all for normal DoFn emit.
I added checks for the normal DoFn case and allowed for skew in the timer
case. It was a bit more extensive change then I expected so PTAL. If these
checks make sense I can add some unit testing for the FnApiRunner, though most
of the tests seem focused on splitting and not other functionality.
--
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]
