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
[ENH] Add log cache #2021
base: main
Are you sure you want to change the base?
[ENH] Add log cache #2021
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
04ddca0
to
54c5844
Compare
8fab2b1
to
49712f9
Compare
@MrCroxx Thank you for making Foyer compatible with Rust stable. This PR uses Foyer's memory cache to cache the log records reading from databases in batches. It would be great to have you take a look and offer some insights on the PR. |
Hi, @Ishiihara . Thanks to invite me to review this PR. I'll finish it in 2-3 days. I noticed you introduced both foyer and foyer-memory. IMO, with the latest version (0.8.6), foyer is enouth for most usecases. Is there something that cannot be achieved? Please tell me and I'll export it if necessary. |
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.
Hi @Ishiihara , thanks again for trying foyer, I'll refactor it based on your needs ASAP. Wish you have a good experience with it,
@@ -0,0 +1,13 @@ | |||
use serde::Deserialize; | |||
|
|||
#[derive(Deserialize)] |
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.
Let me support serde for EvictionConfig
in foyer, then you can omit it.
} | ||
|
||
pub(crate) struct LogCache { | ||
cache: Cache<LogCacheKey, Arc<[LogRecord]>, LogCacheEventListener>, |
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.
Please use foyer::Cache
is only in-memory is needed, otherwise, use foyer::HybridCache
instead.
In short, only foyer
needs to be imported.
cache: Cache<LogCacheKey, Arc<[LogRecord]>, LogCacheEventListener>, | ||
} | ||
|
||
impl Debug for LogCache { |
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.
I'll impl Debug
for Cache
and HybridCache
, then you can use derive.
} | ||
} | ||
|
||
pub(crate) struct EntryDataHandle { |
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.
Remove it if you only read from foyer?
Hi @Ishiihara , you can run |
Description of changes
Summarize the changes made by this PR.
Test plan
How are these changes tested?
pytest
for python,yarn test
for js,cargo test
for rustDocumentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs repository?