dengpanyin commented on a change in pull request #13294:
URL: https://github.com/apache/beam/pull/13294#discussion_r521523240
##########
File path:
runners/samza/src/main/java/org/apache/beam/runners/samza/runtime/KeyedTimerData.java
##########
@@ -197,6 +197,8 @@ public void encode(KeyedTimerData<K> value, OutputStream
outStream)
}
final String timerFamilyId = inStream.available() > 0 ?
STRING_CODER.decode(inStream) : "";
+ final Instant outputTimestamp =
Review comment:
Maybe add a comment saying this condition is used for version upgrade,
can be cleaned up later.
##########
File path:
runners/samza/src/main/java/org/apache/beam/runners/samza/runtime/KeyedTimerData.java
##########
@@ -170,13 +170,13 @@ public void encode(KeyedTimerData<K> value, OutputStream
outStream)
}
STRING_CODER.encode(timer.getTimerFamilyId(), outStream);
+ INSTANT_CODER.encode(timer.getOutputTimestamp(), outStream);
Review comment:
Ideally, this field should be put immediately after "timestamp" field so
that when perform a range query, the earliest entries will be processed first.
The assumption for this fix is that the outputTimestamp will not contribute to
the order of range query.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
