zentol commented on a change in pull request #17402:
URL: https://github.com/apache/flink/pull/17402#discussion_r726172216



##########
File path: 
flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/kryo/KryoSerializer.java
##########
@@ -67,6 +69,35 @@
  * <p>This serializer is intended as a fallback serializer for the cases that 
are not covered by the
  * basic types, tuples, and POJOs.
  *
+ * <p>The set of serializers registered with Kryo via {@link Kryo#register}, 
with their respective
+ * IDs, depends on whether flink-java or flink-scala are on the classpath. 
This is for
+ * backwards-compatibility reasons.
+ *
+ * <p>If neither are available (which should only apply to tests in 
flink-core), then:
+ *
+ * <ul>
+ *   <li>0-9 are used for Java primitives
+ *   <li>10+ are used for user-defined registration
+ * </ul>
+ *
+ * <p>If flink-scala is available, then:
+ *
+ * <ul>
+ *   <li>0-9 are used for Java primitives
+ *   <li>10-72 are used for Scala classes
+ *   <li>73-84 are used for Java classes
+ *   <li>85+ are used for user-defined registration
+ * </ul>

Review comment:
       Existing tests cover this quite well already; if you go over the 
boundary you mess up the next serializer range and thus break deserialization 
of existing snapshots.
   
   I did think about enforcing these limits at runtime but it seemed pointless 
because at that point we've already messed up big time and this must be caught 
earlier.
   I thought about specific tests for the ranges, but it wouldn't catch 
anything that existing tests aren't already catching.
   
   




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