masteryhx commented on code in PR #21635:
URL: https://github.com/apache/flink/pull/21635#discussion_r1092823807
##########
flink-core/src/main/java/org/apache/flink/api/common/typeutils/TypeSerializerSnapshot.java:
##########
@@ -124,11 +124,40 @@ void readSnapshot(int readVersion, DataInputView in,
ClassLoader userCodeClassLo
* program's serializer re-serializes the data, thus converting the format
during the restore
* operation.
*
+ * @deprecated This method has been replaced by {@link
TypeSerializerSnapshot
+ * #resolveSchemaCompatibility(TypeSerializerSnapshot)}.
* @param newSerializer the new serializer to check.
* @return the serializer compatibility result.
*/
- TypeSerializerSchemaCompatibility<T> resolveSchemaCompatibility(
- TypeSerializer<T> newSerializer);
+ @Deprecated
+ default TypeSerializerSchemaCompatibility<T> resolveSchemaCompatibility(
+ TypeSerializer<T> newSerializer) {
+ return
newSerializer.snapshotConfiguration().resolveSchemaCompatibility(this);
+ }
+
+ /**
+ * Checks current serializer's compatibility to read data written by the
prior serializer.
+ *
+ * <p>When a checkpoint/savepoint is restored, this method checks whether
the serialization
+ * format of the data in the checkpoint/savepoint is compatible for the
format of the serializer
+ * used by the program that restores the checkpoint/savepoint. The outcome
can be that the
+ * serialization format is compatible, that the program's serializer needs
to reconfigure itself
+ * (meaning to incorporate some information from the
TypeSerializerSnapshot to be compatible),
+ * that the format is outright incompatible, or that a migration needed.
In the latter case, the
+ * TypeSerializerSnapshot produces a serializer to deserialize the data,
and the restoring
+ * program's serializer re-serializes the data, thus converting the format
during the restore
+ * operation.
+ *
+ * <p>This method must be implemented to clarify the compatibility. See
FLIP-263 for more
+ * details.
+ *
+ * @param oldSerializerSnapshot the old serializer snapshot to check.
+ * @return the serializer compatibility result.
+ */
+ default TypeSerializerSchemaCompatibility<T> resolveSchemaCompatibility(
+ TypeSerializerSnapshot<T> oldSerializerSnapshot) {
+ return TypeSerializerSchemaCompatibility.incompatible();
Review Comment:
Thanks for the detail case.
It seems it's the most critical problem so I think we could resolve this
firstly.
For resolving this, there are some solutions:
1. implement all internal serializers as before pr but not remove the old
deprecated method for composite serializers:
a. Benefit: From users' perspective, previous existing custon serializers
and new serializers could work no matter whether users migrate to the new
method:
ⅰ. For some previous existing custom serializers that don't migrate,
all old code paths will be accessed as before. Users' codes could run without
any modifying.
ⅱ. For some previous existing custom serializers that want to migrate,
they could just implement the new method.
1. If users don't remove the old method, it could also work. Of
course, we recomend users to remove it.
2. If users has used the internal serializers, the new method
could just be called.
ⅲ. For new defined custom serializers:
1. If users don't implement any methods and restore from the
snapshot. INCOMPATIBLE will be returned and the job will fail. Of course, we
highly recomend users to implement the new method.
2. If users implement the new method, it's same as [b].
b. Cost: there are many deprecated method remained.
2. Also implement the new method to call the old method:
a. Benefit: Then we could just migrate all internal serializers and remove
old methods.
b. Cost: It requires users to implement at least one method else it will
stuck in an infinite loop. Although Option 1 also have requirement for users,
this is a worse case obviously.
3. Just remove the old method in the interface:
a. Benefit: it will make code path clear and we could migrate all
serializers and make all caller use the new method
b. Cost: it will beak change so all users have to migrate their
serializers to the new method. Else their jobs could not compile.
I think Option 1 looks good to me. WDYT? Or any other suggestions ?
--
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
