zentol commented on a change in pull request #7061: [FLINK-10827][tests] Add
test for duplicate() to SerializerTestBase
URL: https://github.com/apache/flink/pull/7061#discussion_r232240467
##########
File path:
flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/PojoSerializer.java
##########
@@ -181,11 +181,13 @@ public boolean isImmutableType() {
}
}
- if (stateful) {
- return new PojoSerializer<T>(clazz,
duplicateFieldSerializers, fields, executionConfig);
- } else {
- return this;
+ if (!stateful) {
+ // as a small memory optimization, we can share the
same object between instances
+ duplicateFieldSerializers = fieldSerializers;
}
+
+ // we must create a new instance, otherwise the
subclassSerializerCache can create concurrency problems
Review comment:
Nope, I don't understand this change :/
The stacktrace shows that the KryoSerializer is used by multiple threads
concurrently. `KryoSerializer#duplicate` always returns a new instance, so this
method should've already returned a new instance since `stateful` is true.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
