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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
