[GitHub] [flink] igalshilman commented on a change in pull request #17402: [FLINK-24017] Setup Kryo to be usable without flink-scala

[GitBox <gi...@apache.org>]

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



##########
File path: flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/KryoUtils.java
##########
@@ -90,28 +90,32 @@
      * Apply a list of {@link KryoRegistration} to a Kryo instance. The list of registrations is
      * assumed to already be a final resolution of all possible registration overwrites.
      *
-     * <p>The registrations are applied in the given order and always specify the registration id as
-     * the next available id in the Kryo instance (providing the id just extra ensures nothing is
-     * overwritten, and isn't strictly required);
+     * <p>The registrations are applied in the given order and always specify the registration id,
+     * using the given {@code firstRegistrationId} and incrementing it for each registration.
      *
      * @param kryo the Kryo instance to apply the registrations
      * @param resolvedRegistrations the registrations, which should already be resolved of all
      *     possible registration overwrites
+     * @param firstRegistrationId the first registration id to use
      */
     public static void applyRegistrations(
-            Kryo kryo, Collection<KryoRegistration> resolvedRegistrations) {
+            Kryo kryo,
+            Collection<KryoRegistration> resolvedRegistrations,
+            int firstRegistrationId) {
 
+        int currentRegistrationId = firstRegistrationId;
         Serializer<?> serializer;
         for (KryoRegistration registration : resolvedRegistrations) {
             serializer = registration.getSerializer(kryo);
 
             if (serializer != null) {
-                kryo.register(
-                        registration.getRegisteredClass(),
-                        serializer,
-                        kryo.getNextRegistrationId());
+                kryo.register(registration.getRegisteredClass(), serializer, currentRegistrationId);
             } else {
-                kryo.register(registration.getRegisteredClass(), kryo.getNextRegistrationId());
+                kryo.register(registration.getRegisteredClass(), currentRegistrationId);
+            }
+            // if Kryo already had a serializer for that type then it ignores the registration
+            if (kryo.getRegistration(currentRegistrationId) != null) {

Review comment:
       @zentol this `if` is positioned in a suspicious place. could it ever be `!= null` ?




-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org