apache / kyuubi

Apache Kyuubi is a distributed and multi-tenant gateway to provide serverless SQL on data warehouses and lakehouses.
https://kyuubi.apache.org/
Apache License 2.0
2.11k stars 915 forks source link

[KYUUBI #6704] Disable periodic gc if set interval to 0 #6725

Closed taylor12805 closed 1 month ago

taylor12805 commented 1 month ago

:mag: Description

Issue References ๐Ÿ”—

This pull request fixes https://github.com/apache/kyuubi/issues/6704

Describe Your Solution ๐Ÿ”ง

if periodic gc is set to 0, there is no need to perform an explicit gc.

Types of changes :bookmark:

Test Plan ๐Ÿงช

Behavior Without This Pull Request :coffin:

Behavior With This Pull Request :tada:

Related Unit Tests


Checklist ๐Ÿ“

Be nice. Be informative.

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (2d64255) to head (a52ddda). Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
...a/org/apache/kyuubi/server/PeriodicGCService.scala 0.00% 8 Missing :warning:
...in/scala/org/apache/kyuubi/config/KyuubiConf.scala 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #6725 +/- ## ====================================== Coverage 0.00% 0.00% ====================================== Files 684 684 Lines 42282 42283 +1 Branches 5767 5768 +1 ====================================== - Misses 42282 42283 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pan3793 commented 1 month ago

behavior change looks good to me, would you mind updating the doc of kyuubi.periodicGC.interval to mention that?

bowenliang123 commented 1 month ago

@taylor12805 Please update the document for kyuubi.server.periodicGC.interval in KyuubiConf class, and then run dev/gen/gen_all_config_docs.sh script to update the markdown docs. Thanks for your contribution.

taylor12805 commented 1 month ago

behavior change looks good to me, would you mind updating the doc of kyuubi.periodicGC.interval to mention that?

@taylor12805 Please update the document for kyuubi.server.periodicGC.interval in KyuubiConf class, and then run dev/gen/gen_all_config_docs.sh script to update the markdown docs. Thanks for your contribution.

sure, i have updated the document

pan3793 commented 1 month ago

@taylor12805 you need to modify the code and regenerate the md https://github.com/apache/kyuubi/blob/f8431da7acc4530929ffbddf9fdaa8e6cec9f15c/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala#L3112

taylor12805 commented 1 month ago

@taylor12805 you need to modify the code and regenerate the md

https://github.com/apache/kyuubi/blob/f8431da7acc4530929ffbddf9fdaa8e6cec9f15c/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala#L3112

yeah, sure, i have figured that out

pan3793 commented 1 month ago

Thanks, merged to master for 1.10