JunRuiLee commented on code in PR #25798:
URL: https://github.com/apache/flink/pull/25798#discussion_r1893728656


##########
flink-runtime/src/main/java/org/apache/flink/streaming/api/graph/StreamGraphContext.java:
##########
@@ -62,4 +62,13 @@ public interface StreamGraphContext {
      * @return true if all modifications were successful and applied 
atomically, false otherwise.
      */
     boolean modifyStreamEdge(List<StreamEdgeUpdateRequestInfo> requestInfos);
+
+    interface StreamGraphUpdateListener {
+        /**
+         * This method is called whenever the StreamGraph is updated.
+         *
+         * @param streamGraph the updated StreamGraph
+         */
+        void onStreamGraphUpdated(StreamGraph streamGraph);

Review Comment:
   The context of the stream graph is generated from the AdaptiveGraphManager. 
Therefore, it is natural for the context to hold the same stream graph as the 
one held by the AdaptiveGraphManager. On the other hand, it is strange that the 
listener does not know which stream graph it is listening to. Therefore, I 
think there is no need to pass a stream graph.
   
   As for the JobStatus you mentioned, I believe it is fine as it is written 
based on event listening. They wouldn't pass a job graph object either. If this 
interface were modified to notify a specific event, such as which stream graph 
edges were updated, I think that would also work; however, I prefer to keep it 
simple and see no need to make it that complicated.



-- 
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]

Reply via email to