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

Persist: Enable reference-cache by default #9760

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

Conversation

snazy
Copy link
Member

@snazy snazy commented Oct 15, 2024

Default cache TTL is 5 minutes.

@snazy snazy added the pr-integrations NesQuEIT (Iceberg, Spark, Flink, Presto) label Oct 16, 2024
@snazy snazy force-pushed the ref-cache-enable-default branch 2 times, most recently from 4e93232 to f66a447 Compare November 4, 2024 17:05
@snazy snazy force-pushed the ref-cache-enable-default branch from f66a447 to 7a48740 Compare November 11, 2024 13:36
Default cache TTL is 5 minutes.
@snazy snazy force-pushed the ref-cache-enable-default branch from 7a48740 to b8bedf0 Compare December 4, 2024 07:59
@snazy snazy marked this pull request as ready for review December 4, 2024 15:07
Copy link
Member

@dimas-b dimas-b left a comment

Choose a reason for hiding this comment

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

Code changes LGTM, but I'm not sure about enabling this cache by default.

Comment on lines +59 to +63
- If you are not using k8s and you are running Nessie with multiple nodes, you must either
* configure `nessie.version.store.persist.cache-invalidations.service-names`, see
[docs reference](https://projectnessie.org/nessie-latest/configuration/#version-store-advanced-settings),
or
* disable the reference cache by setting `nessie.version.store.persist.reference-cache-ttl` to `PT0S`.
Copy link
Member

Choose a reason for hiding this comment

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

What's the benefit of enabling reference cache by default if users have to do additional config changes to run Nessie now?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's only if you run multiple nodes without k8s

Copy link
Member

Choose a reason for hiding this comment

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

But is reference cache critical for single-node deployments?

"Must" is a strong word. It looks like the new default is going to cause undue failures for naive users when they scale Nessie.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-integrations NesQuEIT (Iceberg, Spark, Flink, Presto)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants