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

[docs] Add documentation for cache configuration properties #24623

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

minhancao
Copy link
Contributor

@minhancao minhancao commented Feb 24, 2025

Description

Add documentation for cache configuration properties

All of these cache configuration properties are defined here in the codebase:
https://github.com/prestodb/presto/blob/master/presto-native-execution/presto_cpp/main/common/Configs.h#L328

Relates to facebookincubator/velox#11429

Motivation and Context

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • 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 ==

Documentation
* Add documentation for cache configuration properties

@minhancao minhancao self-assigned this Feb 24, 2025
@minhancao minhancao requested review from steveburnett, elharo and a team as code owners February 24, 2025 23:41
@minhancao minhancao requested a review from presto-oss February 24, 2025 23:41
@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Feb 24, 2025
@prestodb-ci prestodb-ci requested review from a team and anandamideShakyan and removed request for a team February 24, 2025 23:41
@github-actions github-actions bot added the docs label Feb 24, 2025
@minhancao minhancao force-pushed the cache_configs_doc_prestissimo branch from 2427fdf to 3d22475 Compare February 24, 2025 23:44
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.

Thanks for the doc! Some formatting issues and some nits of phrasing suggestions. Looks good overall!

are supported: ns, us, ms, s, m, h, d.

``cache.velox.ttl-check-interval``
^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
^^^^^^^^^^^^^^^^^^^^^^^^^^^
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

* **Type:** ``string``
* **Default value:** ``0s``

The interval for persisting in-memory cache to SSD. Setting this config
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The interval for persisting in-memory cache to SSD. Setting this config
The interval for persisting in-memory cache to SSD. Set this

* **Default value:** ``0s``

The interval for persisting in-memory cache to SSD. Setting this config
to a non-zero value will activate periodic cache persistence.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
to a non-zero value will activate periodic cache persistence.
to a non-zero value to activate periodic cache persistence.

* **Type:** ``integer``
* **Default value:** ``0``

The size of the SSD.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest mentioning the units for this: bytes, Mb, Gb?

* **Type:** ``string``
* **Default value:** ``/mnt/flash/async_cache.``

The directory that is mounted onto SSD.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The directory that is mounted onto SSD.
The path of the directory that is mounted in the SSD.

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* **Type:** ``bool``
* **Default value:** ``false``
When enabled, a CRC-based checksum is calculated for each cache entry written to SSD.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
When enabled, a CRC-based checksum is calculated for each cache entry written to SSD.
When enabled, a CRC-based checksum is calculated for each cache entry written to SSD.

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* **Type:** ``bool``
* **Default value:** ``false``
When enabled, the checksum is recalculated and verified against the stored value when
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
When enabled, the checksum is recalculated and verified against the stored value when
When enabled, the checksum is recalculated and verified against the stored value when

^^^^^^^^^^^^^^^^^^^^^^^^^^^
* **Type:** ``bool``
* **Default value:** ``false``
Enable TTL for AsyncDataCache and SSD cache.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Enable TTL for AsyncDataCache and SSD cache.
Enable TTL for AsyncDataCache and SSD cache.

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* **Type:** ``string``
* **Default value:** ``2d``
TTL duration for AsyncDataCache and SSD cache entries. The following time units
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
TTL duration for AsyncDataCache and SSD cache entries. The following time units
TTL duration for AsyncDataCache and SSD cache entries. The following time units

^^^^^^^^^^^^^^^^^^^^^^^^^^^
* **Type:** ``string``
* **Default value:** ``1h``
The periodic duration to apply cache TTL and evict AsyncDataCache and SSD cache entries.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The periodic duration to apply cache TTL and evict AsyncDataCache and SSD cache entries.
The periodic duration to apply cache TTL and evict AsyncDataCache and SSD cache entries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs from:IBM PR from IBM
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

3 participants