Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use data size for directory listing cache #23176

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

NikhilCollooru
Copy link
Contributor

@NikhilCollooru NikhilCollooru commented Jul 10, 2024

Description

Use data size for directory listing cache

Motivation and Context

We currently only have a size control on the number of values but not on the size in bytes. Given this is in memory cache, having a max size in bytes for the cache will be helpful

Impact

hive.file-status-cache-size is deprecated and hive.file-status-cache.max-retained-size now represents the max size in bytes for the directory listing cache.

Test Plan

existing tests

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

Hive Connector Changes
* Add support for setting the max size in bytes for directory listing cache. This can be set via the new `hive.file-status-cache.max-retained-size` configuration property. `hive.file-status-cache-size` is now deprecated :pr:`23176`

@NikhilCollooru NikhilCollooru requested a review from a team as a code owner July 10, 2024 22:29
@NikhilCollooru NikhilCollooru force-pushed the listCacheSize branch 2 times, most recently from de750ac to 67e3cc7 Compare July 11, 2024 13:43
swapsmagic
swapsmagic previously approved these changes Jul 11, 2024
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deprecated configuration property `hive.file-status-cache-size does not appear to be documented, so no doc change is needed for it.

Suggest add documentation for hive.file-status-cache.max-retained-size configuration property - maybe to https://github.com/prestodb/presto/blob/master/presto-docs/src/main/sphinx/connector/hive.rst#hive-configuration-properties

elharo
elharo previously approved these changes Jul 12, 2024
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! (docs)

Pull branch, new local doc build, looks good. Thanks!

@NikhilCollooru NikhilCollooru merged commit 0521740 into prestodb:master Jul 18, 2024
57 checks passed
@NikhilCollooru NikhilCollooru deleted the listCacheSize branch July 18, 2024 17:12
@tdcmeehan tdcmeehan mentioned this pull request Aug 23, 2024
34 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants