Shutdown the executor service in KubernetesApplicationOperation and prevent NPE #6785
Conversation
turboFei
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6785 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 688 688
Lines 42598 42605 +7
Branches 5807 5809 +2
======================================
- Misses 42598 42605 +7 ☔ View full report in Codecov by Sentry. |
| @@ -273,6 +274,16 @@ class KubernetesApplicationOperation extends ApplicationOperation with Logging { | |||
| Utils.tryLogNonFatalError(client.close()) | |||
| } | |||
| kubernetesClients.clear() | |||
|
|
|||
| if (expireCleanUpTriggerCacheExecutor != null) { | |||
There was a problem hiding this comment.
This may need to be placed before assigning cleanupTerminatedAppInfoTrigger to null.
There was a problem hiding this comment.
No, we need to invalidate the cache before shutdown the executor.
There was a problem hiding this comment.
So that, the remaining terminated application can be cleaned before server stop.
There was a problem hiding this comment.
The non-null check here is not atomic, so NPE may still occur.
There was a problem hiding this comment.
How about?
Option(cleanupTerminatedAppInfoTrigger).foreach { trigger =>
trigger.asMap().asScala.foreach {
case (key, _) =>
// do get to trigger cache eviction
trigger.getIfPresent(key)
}
}
|
Thanks for the PR! This PR is being closed due to inactivity. This isn't a judgement on the merit of the PR in any way. If this is still an issue with the latest version of Kyuubi, please reopen it and ask a committer to remove the Stale tag! Thank you for using Kyuubi! |
🔍 Description
Issue References 🔗
As title.
Fix NPE, because the cleanupTerminatedAppInfoTrigger will be set to
null.kyuubi/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala
Line 269 in d3520dd
Also shutdown the ExecutorService when KubernetesApplicationOperation stoped.
Describe Your Solution 🔧
Shutdown the thread executor service and check the null.
Types of changes 🔖
Test Plan 🧪
Behavior Without This Pull Request ⚰️
Behavior With This Pull Request 🎉
Related Unit Tests
Checklist 📝
Be nice. Be informative.