kristoffSC commented on code in PR #21022:
URL: https://github.com/apache/flink/pull/21022#discussion_r994322968


##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/operators/sink/committables/CommittableCollectorSerializer.java:
##########
@@ -161,6 +162,22 @@ public CheckpointCommittableManagerImpl<CommT> 
deserialize(int version, byte[] s
     private class SubtaskSimpleVersionedSerializer
             implements 
SimpleVersionedSerializer<SubtaskCommittableManager<CommT>> {
 
+        @Nullable private final Long checkpointId;
+
+        /**
+         * This ctor must be used to create a deserializer where the 
checkpointId is used to set the
+         * checkpointId of the deserialized SubtaskCommittableManager.
+         *
+         * @param checkpointId used to recover the SubtaskCommittableManager
+         */
+        public SubtaskSimpleVersionedSerializer(long checkpointId) {
+            this.checkpointId = checkpointId;
+        }
+
+        public SubtaskSimpleVersionedSerializer() {
+            this.checkpointId = null;

Review Comment:
   nit: I'm wondering if we need this constructor. Maybe it would be slightly 
better if it would be removed.
   It is used only in `CheckpointSimpleVersionedSerializer::serialize`.
   
   I get that we don't want to serialzie checkpointId however,
   if someone will call `new SubtaskSimpleVersionedSerializer()` and call ` 
public SubtaskCommittableManager<CommT> deserialize(int version, byte[] 
serialized)` on this instance, it will fail on null check for checkpointId.
   
   Currently usage pattern in Flink is not having this path, but Im thinking 
about general use case for this object.



-- 
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]

Reply via email to