1996fanrui commented on code in PR #23550:
URL: https://github.com/apache/flink/pull/23550#discussion_r1371930593
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/graph/StreamConfig.java:
##########
@@ -368,7 +373,8 @@ public void setStreamOperator(StreamOperator<?> operator) {
public void setStreamOperatorFactory(StreamOperatorFactory<?> factory) {
if (factory != null) {
- toBeSerializedConfigObjects.put(SERIALIZEDUDF, factory);
+ toBeSerializedConfigObjects.put(SERIALIZED_UDF, factory);
+ config.setString(SERIALIZED_UDF_CLASS_NAME,
factory.getClass().getName());
Review Comment:
I have a question, is there the same classloader issue for solution1?
For example, the new sub class and SinkWriterOperatorFactory are loaded by
different classloader, and we cannot check whether the class is a subclass of
SinkWriterOperatorFactory.
Maybe my consideration is unnecessary, if the subclass cannot be check, too
many code cannot work.
Assume we can ignore the class loader issue. Whether the solution3 is more
clear? It allow developer add new subclass in the future, and code is easy to
read.
Also, the TaskExextor and SinkWriterOperatorFactory should be loaded by one
classloader, they are flink code, from the same dist jar. And we try to
deserialize the operator factory using the TaskExector.class.getClassLoader, so
the class loader should be same.
How about solution3?
--
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]