kristoffSC commented on code in PR #21022:
URL: https://github.com/apache/flink/pull/21022#discussion_r994542297
##########
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:
> Afaict in your suggestion, we also have to check whether the checkpointId
is set and fail eventually.
Yes but we could add null Check in constructor and fail fast, before
creating a instance.
My point (and I agree its a small thing) is that with default constructor we
"allow" user/developer to create instance that will not be fully functional ->
will throw an exception on deserialzie even though it was able to serialize the
object. What I'm proposing is that if instance will be created it will be fully
initialize/functional. Sure user can pass "invalid" arguments but we can check
them in constructor.
Plus we could use primitive long instead Long right? In that case we would
not need null check at all.
As an alternative maybe a javadoc on the default constructor describing the
drawback of using this constructor version would be ok.
--
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]
