pan3793 commented on code in PR #5806:
URL: https://github.com/apache/kyuubi/pull/5806#discussion_r1417023887


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala:
##########
@@ -147,6 +150,26 @@ class KubernetesApplicationOperation extends 
ApplicationOperation with Logging {
         }
       })
       .build()
+    if 
(conf.get(KyuubiConf.KUBERNETES_SPARK_CLEANUP_TERMINATED_DRIVE_POD_IMMEDIATELY))
 {
+      expireCleanUpTriggerCacheExecutor = 
Executors.newSingleThreadScheduledExecutor(

Review Comment:
   Use ThreadUtils to create the executor



##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala:
##########
@@ -147,6 +150,26 @@ class KubernetesApplicationOperation extends 
ApplicationOperation with Logging {
         }
       })
       .build()
+    if 
(conf.get(KyuubiConf.KUBERNETES_SPARK_CLEANUP_TERMINATED_DRIVE_POD_IMMEDIATELY))
 {
+      expireCleanUpTriggerCacheExecutor = 
Executors.newSingleThreadScheduledExecutor(
+        new ThreadFactoryBuilder().setDaemon(true).setNameFormat(
+          s"pod-clean-up-trigger-thread").build())
+      expireCleanUpTriggerCacheExecutor.scheduleAtFixedRate(

Review Comment:
   prefer to use `scheduleAtFixedDelay` if possible
   
   I have encountered a performance issue when the task execution time is 
longer than the scheduling interval, which causes high CPU usage.



##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -1241,6 +1241,15 @@ object KyuubiConf {
       .stringConf
       .createWithDefault(KubernetesCleanupDriverPodStrategy.NONE.toString)
 
+  val KUBERNETES_SPARK_CLEANUP_TERMINATED_DRIVE_POD_IMMEDIATELY: 
ConfigEntry[Boolean] =
+    buildConf("kyuubi.kubernetes.spark.cleanupTerminatedDriverPodImmediately")

Review Comment:
   sorry for making you back and forth, I think the configuration name is 
confusing now. how about?
   ```
   kyuubi.kubernetes.spark.cleanupTerminatedDriverPod.kind=[NONE|ALL|COMPLETED]
   kyuubi.kubernetes.spark.cleanupTerminatedDriverPod.checkInterval=[PT1M]
   ```



##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala:
##########
@@ -147,6 +150,26 @@ class KubernetesApplicationOperation extends 
ApplicationOperation with Logging {
         }
       })
       .build()
+    if 
(conf.get(KyuubiConf.KUBERNETES_SPARK_CLEANUP_TERMINATED_DRIVE_POD_IMMEDIATELY))
 {
+      expireCleanUpTriggerCacheExecutor = 
Executors.newSingleThreadScheduledExecutor(
+        new ThreadFactoryBuilder().setDaemon(true).setNameFormat(
+          s"pod-clean-up-trigger-thread").build())
+      expireCleanUpTriggerCacheExecutor.scheduleAtFixedRate(
+        () => {
+          try {
+            val keys = cleanupTerminatedAppInfoTrigger.asMap().keySet().asScala
+            for (key <- keys) {
+              // do get to trigger cache eviction
+              cleanupTerminatedAppInfoTrigger.getIfPresent(key)
+            }

Review Comment:
   ```suggestion
               val keys = 
cleanupTerminatedAppInfoTrigger.asMap().asScala.foreach { case (key, _) =>
                 
                 // do get to trigger cache eviction
                 cleanupTerminatedAppInfoTrigger.getIfPresent(key)
               }
   ```



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to