robertwb commented on code in PR #31092:
URL: https://github.com/apache/beam/pull/31092#discussion_r1585535722
##########
model/fn-execution/src/main/proto/org/apache/beam/model/fn_execution/v1/beam_fn_api.proto:
##########
@@ -1063,6 +1087,10 @@ message StateAppendRequest {
// Represents a part of a logical byte stream. Elements within
// the logical byte stream are encoded in the nested context and
// multiple append requests are concatenated together.
+ // For OrderedListState, elements of TimeStampedValue<T> should be encoded
+ // with TimestampedValueCoder.of(LengthPrefixCoder.of(Coder<T>)), so that
+ // the request handler knows how to decode timestamps from the data without
Review Comment:
TimestampedValueCoder is not a well defined portable/cross-language Beam
coder. We can continue to use TimestampedValue in the Java code if it makes
life easier, but IMHO we should be using a better defined coder. `KV<VarInt64,
LP<value>>` seems like the most natural option here. (Another bonus: we don't
have to dither about endianness or millis vs micros, even if it "usually" holds
a timestamp.)
--
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]
