RocMarshal commented on code in PR #865:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/865#discussion_r1716377459
##########
flink-autoscaler-standalone/src/main/java/org/apache/flink/autoscaler/standalone/StandaloneAutoscalerExecutor.java:
##########
@@ -113,6 +113,8 @@ public void start() {
public void close() {
scheduledExecutorService.shutdownNow();
scalingThreadPool.shutdownNow();
+ autoScaler.close();
+ eventHandler.close();
Review Comment:
> IIUC, eventHandler.close() is called twice.
Yes.
But, There're no semantic errors, it's just an extra call.
> We don't need to introduce the close for autoScaler, because we could
close eventHandler directly.
Yes, In the initial edition, I also did it this way.
The reason why I done it as the current way is:
In the current attributes of `StandaloneAutoscalerExecutor.java`,
introducing a close method for `eventHandler` alone would meet the requirements
and ensure good semantics. However, since `autoScaler` also references
`eventHandler`, would it be better to close the objects that reference
`eventHandler` before closing `eventHandler ` itself ?
Of course, The I'd like to update it based on the comments from you.
--
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
