-
Notifications
You must be signed in to change notification settings - Fork 40
Disable negative cache for table metadata #3182
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
Disable negative cache for table metadata #3182
Conversation
Summary of ChangesHello @brfrn169, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical caching bug where the system would incorrectly cache the absence of table metadata. This led to scenarios where newly created tables were not discoverable until the cache expired. The solution involves migrating the underlying caching library from Guava to Caffeine, which, by design, does not store null values, thereby eliminating the negative caching behavior and improving the reliability of table metadata lookups. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully disables negative caching for table metadata by migrating from Guava Cache to Caffeine. The changes are logical and correctly leverage Caffeine's behavior of not caching null values. The accompanying tests are thorough and validate the new caching strategy, especially the absence of negative caching.
I've added a couple of suggestions to use modern Java features like method references and lambda expressions to replace anonymous inner classes. These changes will make the code more concise and readable.
core/src/main/java/com/scalar/db/common/TableMetadataManager.java
Outdated
Show resolved
Hide resolved
...src/main/java/com/scalar/db/transaction/consensuscommit/TransactionTableMetadataManager.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Torch3333
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
jnmt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
komamitsu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 👍
feeblefakie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
Description
This PR disables negative caching for table metadata by migrating from Guava Cache to Caffeine and changing the cache behavior to not cache null values.
Previously,
TableMetadataManagerandTransactionTableMetadataManagerused Guava's LoadingCache which cachedOptional<TableMetadata>values. When a table was not found (returningOptional.empty()), this negative result was cached. This caused problems when:Related issues and/or PRs
N/A
Changes made
com.google.common.cache.LoadingCachetocom.github.benmanes.caffeine.cache.LoadingCache.TableMetadataManagerandTransactionTableMetadataManagerto use Caffeine's cache API.Checklist
Additional notes (optional)
N/A
Release notes
N/A