Re: [PR] [ISSUES-3667] KubernetesClusterDescriptor not closed after use, cause the file handle resource leaks [incubator-streampark]

[via GitHub]

caicancai merged PR #3668:
URL: https://github.com/apache/incubator-streampark/pull/3668


-- 
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...@streampark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [ISSUES-3667] KubernetesClusterDescriptor not closed after use, cause the file handle resource leaks [incubator-streampark]

[via GitHub]

caicancai commented on code in PR #3668:
URL: 
https://github.com/apache/incubator-streampark/pull/3668#discussion_r1566599866


##
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/KubernetesRetriever.scala:
##
@@ -89,15 +89,19 @@ object KubernetesRetriever extends Logger {
 val clusterProvider: KubernetesClusterDescriptor =
   
clientFactory.createClusterDescriptor(flinkConfig).asInstanceOf[KubernetesClusterDescriptor]
 
-Try {
-  clusterProvider
-.retrieve(flinkConfig.getString(KubernetesConfigOptions.CLUSTER_ID))
-.getClusterClient
-} match {
-  case Success(v) => Some(v)
-  case Failure(e) =>
-logError(s"Get flinkClient error, the error is: $e")
-None
+try {

Review Comment:
   +1



-- 
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...@streampark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [ISSUES-3667] KubernetesClusterDescriptor not closed after use, cause the file handle resource leaks [incubator-streampark]

[via GitHub]

wolfboys commented on code in PR #3668:
URL: 
https://github.com/apache/incubator-streampark/pull/3668#discussion_r1565641719


##
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/KubernetesRetriever.scala:
##
@@ -89,15 +89,19 @@ object KubernetesRetriever extends Logger {
 val clusterProvider: KubernetesClusterDescriptor =
   
clientFactory.createClusterDescriptor(flinkConfig).asInstanceOf[KubernetesClusterDescriptor]
 
-Try {
-  clusterProvider
-.retrieve(flinkConfig.getString(KubernetesConfigOptions.CLUSTER_ID))
-.getClusterClient
-} match {
-  case Success(v) => Some(v)
-  case Failure(e) =>
-logError(s"Get flinkClient error, the error is: $e")
-None
+try {

Review Comment:
   Can you test the improved code below? If it can pass the test, it is 
recommended to use the following code
   
   ```scala
   // retrieve flink cluster client
 clusterClientServiceLoader
   .getClusterClientFactory(flinkConfig)
   .createClusterDescriptor(flinkConfig)
   .asInstanceOf[KubernetesClusterDescriptor]
   .autoClose {
 clusterProvider =>
   Try {
 clusterProvider
   
.retrieve(flinkConfig.getString(KubernetesConfigOptions.CLUSTER_ID))
   .getClusterClient
   } match {
 case Success(v) => Some(v)
 case Failure(e) =>
   logError(s"Get flinkClient error, the error is: $e")
   None
   }
   }
   
   ```



-- 
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...@streampark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[PR] [ISSUES-3667] KubernetesClusterDescriptor not closed after use, cause the file handle resource leaks [incubator-streampark]

[via GitHub]

jianjun159 opened a new pull request, #3668:
URL: https://github.com/apache/incubator-streampark/pull/3668

   
   
   ## What changes were proposed in this pull request
   
   Issue Number: close #3667 
   
   
   
   ## Brief change log
   
   
   
   ## Verifying this change
   
   
   
   This change is a trivial rework / code cleanup without any test coverage.
   
   *(or)*
   
   This change is already covered by existing tests, such as *(please describe 
tests)*.
   
   *(or)*
   
   This change added tests and can be verified as follows:
   
   
   
   ## Does this pull request potentially affect one of the following parts
- Dependencies (does it add or upgrade a dependency): (yes / no)
   


-- 
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...@streampark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org