You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "turboFei (via GitHub)" <gi...@apache.org> on 2025/10/02 04:18:51 UTC
[PR] Proactive cleanup kubernetes terminated application info [kyuubi]
turboFei opened a new pull request, #7220:
URL: https://github.com/apache/kyuubi/pull/7220
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/master/contributing/code/index.html
2. If the PR is related to an issue in https://github.com/apache/kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug, and what versions are affected.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
### Was this patch authored or co-authored using generative AI tooling?
<!--
If a generative AI tooling has been used in the process of authoring this patch, please include
phrase 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Proactive cleanup kubernetes terminated application info [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #7220:
URL: https://github.com/apache/kyuubi/pull/7220#issuecomment-3359296315
the change seems to violate the semantic of `kyuubi.kubernetes.terminatedApplicationRetainPeriod`
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Proactive cleanup kubernetes terminated application info [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #7220:
URL: https://github.com/apache/kyuubi/pull/7220#issuecomment-3359326404
FROM GPT:
cache.cleanUp() forces the cache to process pending evictions (expired entries, over-capacity entries, etc.) right away instead of waiting for normal access patterns to trigger cleanup.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Proactive cleanup kubernetes terminated application info [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei closed pull request #7220: Proactive cleanup kubernetes terminated application info
URL: https://github.com/apache/kyuubi/pull/7220
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Proactive cleanup kubernetes terminated application info [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #7220:
URL: https://github.com/apache/kyuubi/pull/7220#issuecomment-3359323799
In **Guava’s `Cache` API**, the method `cache.cleanUp()` is used to **synchronously perform maintenance operations** on the cache.
Here’s what it does in detail:
---
### 🔹 Purpose of `cleanUp()`
* Guava caches perform most maintenance operations (like evicting expired entries or removing entries exceeding maximum size) **lazily** — that is, during normal cache operations such as `get`, `put`, or `getIfPresent`.
* The `cleanUp()` method forces the cache to immediately run these pending maintenance tasks.
---
### 🔹 When to Use It
* Normally, you **don’t need to call** `cleanUp()` yourself; Guava handles it during cache operations.
* But in some cases — like when you want to:
* **Benchmark** cache performance without pending work affecting results.
* **Manually trigger eviction** (e.g., after setting `expireAfterWrite` or `maximumSize`).
* **Ensure consistency** before shutting down an application or running a test — you might explicitly call `cleanUp()`.
---
### 🔹 Important Notes
* `cleanUp()` does **not** block future writes or reads. It just runs pending maintenance at the time of the call.
* If your cache uses expiration or size-based eviction, `cleanUp()` ensures those rules are applied **immediately**, rather than waiting for the next normal operation.
* It’s typically lightweight but should not be called in a tight loop — Guava already optimizes cleanup frequency internally.
---
✅ **In short**:
`cache.cleanUp()` forces the cache to process pending evictions (expired entries, over-capacity entries, etc.) right away instead of waiting for normal access patterns to trigger cleanup.
---
Would you like me to also show you a **small code example** demonstrating how entries are only removed after `cleanUp()` is called?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Proactive cleanup kubernetes terminated application info [kyuubi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7220:
URL: https://github.com/apache/kyuubi/pull/7220#issuecomment-3359192379
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/7220?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 0.00%. Comparing base ([`b5d7f58`](https://app.codecov.io/gh/apache/kyuubi/commit/b5d7f5800d1baf9b7117edd224e97e5716235144?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`127a907`](https://app.codecov.io/gh/apache/kyuubi/commit/127a907e0aa8277f7d81640bf80cabe920344402?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/kyuubi/pull/7220?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...kyuubi/engine/KubernetesApplicationOperation.scala](https://app.codecov.io/gh/apache/kyuubi/pull/7220?src=pr&el=tree&filepath=kyuubi-server%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fengine%2FKubernetesApplicationOperation.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvS3ViZXJuZXRlc0FwcGxpY2F0aW9uT3BlcmF0aW9uLnNjYWxh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/7220?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #7220 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 696 696
Lines 43546 43530 -16
Branches 5892 5883 -9
======================================
+ Misses 43546 43530 -16
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/kyuubi/pull/7220?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Proactive cleanup kubernetes terminated application info [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #7220:
URL: https://github.com/apache/kyuubi/pull/7220#issuecomment-3359499359
thanks, merged to master(1.11.0)
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Proactive cleanup kubernetes terminated application info [kyuubi]
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #7220:
URL: https://github.com/apache/kyuubi/pull/7220#issuecomment-3359329360
https://github.com/apache/kyuubi/blob/e613d7f6e4e0b4143dde8bb6ae23d1252698e639/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala#L179-L181
Here we respect the `retainPeriod` for `expireAfterWrite`.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] Proactive cleanup kubernetes terminated application info [kyuubi]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7220:
URL: https://github.com/apache/kyuubi/pull/7220#discussion_r2396716515
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/KubernetesApplicationOperation.scala:
##########
@@ -199,13 +199,7 @@ class KubernetesApplicationOperation extends ApplicationOperation with Logging {
expireCleanUpTriggerCacheExecutor,
() => {
try {
- Option(cleanupTerminatedAppInfoTrigger).foreach { trigger =>
- trigger.asMap().asScala.foreach {
- case (key, _) =>
- // do get to trigger cache eviction
- trigger.getIfPresent(key)
- }
- }
+ Option(cleanupTerminatedAppInfoTrigger).foreach(_.cleanUp())
Review Comment:
The code improvement simplifies the cache cleanup logic, but consider verifying that `cleanUp()` provides the same cleanup behavior as the previous manual iteration. The manual approach specifically triggered eviction through `getIfPresent()`, while `cleanUp()` may use different internal logic for expired entry removal.
```suggestion
Option(cleanupTerminatedAppInfoTrigger).foreach { cache =>
cache.asMap().keySet().asScala.foreach(cache.getIfPresent)
}
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org