fapaul commented on a change in pull request #18616:
URL: https://github.com/apache/flink/pull/18616#discussion_r798531176
##########
File path:
flink-core/src/test/java/org/apache/flink/api/common/typeutils/TypeSerializerUpgradeTestBase.java
##########
@@ -421,7 +421,7 @@ public void upgradedSerializerIsValidWhenCompatibleAsIs()
throws Exception {
DataInputView serializedData =
readAndThenWriteData(dataInput, serializer, serializer,
testDataMatcher);
TypeSerializerSnapshot<T> snapshot =
writeAndThenReadSerializerSnapshot(serializer);
- TypeSerializer<T> restoreSerializer = snapshot.restoreSerializer();
+ TypeSerializer<T> restoreSerializer =
snapshot.restoreSerializer().duplicate();
Review comment:
I think it would be better to check both cases only
`restoreSerializer()` and with `duplicate()`.
##########
File path:
flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/PojoSerializer.java
##########
@@ -163,25 +164,42 @@ public boolean isImmutableType() {
@Override
public PojoSerializer<T> duplicate() {
+ TypeSerializer<Object>[] duplicateFieldSerializers =
duplicateSerializers(fieldSerializers);
+ TypeSerializer<Object>[] duplicateRegisteredSerializers =
+ duplicateSerializers(registeredSerializers);
+
+ // we must create a new instance, otherwise the
subclassSerializerCache can create
+ // concurrency problems
+ return new PojoSerializer<>(
Review comment:
I am still having a hard time understanding the fix. Conceptually it
makes sense but how does the other ctor interact with the snapshot? To me, it
looks like the same fields are passed to the POJO serializer.
--
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
