feat: persist sst meta size #1440
Conversation
There was a problem hiding this comment.
After the review, I find it's weird to fix the object store cache problem by avoiding using get because the interface of the object store doesn't tell the caller get is worse than get_range.
Maybe the right way is to ensure get method to work as expected so that any caller won't be required to remember the file size to use get_range.
| meta_size = Some(size.parse::<usize>().with_context(|| InvalidSize { | ||
| size: size.to_string(), | ||
| })?); |
There was a problem hiding this comment.
I guess snafu will auto clone for the string reference:
| meta_size = Some(size.parse::<usize>().with_context(|| InvalidSize { | |
| size: size.to_string(), | |
| })?); | |
| meta_size = Some(size.parse::<usize>().context(InvalidSize { | |
| size, | |
| }?); |
|
@jiacai2050 Would you please choose the main branch as the base for merging? |
|
The best way to fix is indeed to add cache for
So here I choose to bypass |
| Some(size) => { | ||
| let all_range = 0..size; | ||
| self.store | ||
| .get_range(meta_path, all_range) | ||
| .await | ||
| .with_context(|| FetchFromStore { | ||
| file_path: meta_path.to_string(), | ||
| })? | ||
| } | ||
| None => self | ||
| .store | ||
| .get(meta_path) | ||
| .await | ||
| .with_context(|| FetchFromStore { | ||
| file_path: meta_path.to_string(), | ||
| })? | ||
| .bytes() | ||
| .await | ||
| .with_context(|| FetchAndDecodeSstMeta { | ||
| file_path: meta_path.to_string(), | ||
| })?, |
There was a problem hiding this comment.
Add a comment to explain why we are in favor of get_range?
Rationale
When read sst's metadata, it will use
getmethod to read whole file, but there is no cache inget, so perf may degrade.Detailed Changes
meta_sizein SST's custom meta.get_rangewhen we havesize.Test Plan
CI