AHeise commented on code in PR #25292:
URL: https://github.com/apache/flink/pull/25292#discussion_r1758856122
##########
flink-runtime/src/main/java/org/apache/flink/streaming/api/connector/sink2/CommittableMessageSerializer.java:
##########
@@ -91,13 +90,13 @@ public CommittableMessage<CommT> deserialize(int version,
byte[] serialized)
return new CommittableWithLineage<>(
SimpleVersionedSerialization.readVersionAndDeSerialize(
committableSerializer, in),
- readCheckpointId(in),
+ in.readLong(),
Review Comment:
Thanks for challenging that. It's one of the parts where most uncertainty
still resides. But let's look again on what is happening and has happened:
* On write: we replaced null with EOI, now we get EOI and write it; so it
should be the same byte sequence.
* On read: we have always used `readLong` which is not null-aware. We have
replaced EOI with null.
So for serialization nothing has changed. However, around serialization we
replace null with EOI in all instances of the Message.
That should be safe to change without migration, no?
For compatibility, I left getCheckpointId the same, so it will return an
OptionalLong.empty() on EOI where it previously returned it on null. I have not
found any usage of the method outside of Flink anyhow.
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
