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

cache: also commit current region for internal IO #308

Merged
merged 1 commit into from
Feb 25, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions storage/src/cache/filecache/cache_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ impl FileCacheEntry {
// There are some assumption applied to the `bios` passed to `read_iter()`.
// - The blob address of chunks in `bios` are continuous.
// - There is at most one user io request in the `bios`.
// - The user io request may not aligned on chunk boundary.
// - The user io request may not be aligned on chunk boundary.
// - The user io request may partially consume data from the first and last chunk of user io
// request.
// - Optionally there may be some prefetch/read amplify requests following the user io request.
Expand Down Expand Up @@ -546,7 +546,9 @@ impl FileCacheEntry {
// - the data in the file cache is uncompressed.
// - data validation is disabled
if is_ready && !self.is_compressed && !self.need_validate {
// Silently drop prefetch requests.
// Internal IO should not be committed to local cache region, just
bergwolf marked this conversation as resolved.
Show resolved Hide resolved
// commit this region without pushing any chunk to avoid discontinuous
// chunks in a region.
if req.tags[i].is_user_io() {
state.push(
RegionType::CacheFast,
Expand All @@ -555,6 +557,8 @@ impl FileCacheEntry {
req.tags[i].clone(),
None,
)?;
} else {
state.commit()
bergwolf marked this conversation as resolved.
Show resolved Hide resolved
}
} else if self.is_stargz || !self.is_direct_chunkmap || is_ready {
// Case to try loading data from cache
Expand All @@ -581,7 +585,7 @@ impl FileCacheEntry {
} else {
BlobIoTag::Internal(chunk.compress_offset())
};
// NOTE: Only this request region can steak more chunks from backend with user io.
// NOTE: Only this request region can steal more chunks from backend with user io.
state.push(
RegionType::Backend,
chunk.compress_offset(),
Expand Down Expand Up @@ -961,7 +965,7 @@ impl RegionType {
}
}

/// A continuous region in cache file or backend storage/blob, it may contains several chunks.
/// A continuous region in cache file or backend storage/blob, it may contain several chunks.
struct Region {
r#type: RegionType,
status: RegionStatus,
Expand Down Expand Up @@ -1041,12 +1045,16 @@ impl Region {

struct FileIoMergeState {
regions: Vec<Region>,
// Whether last region can take in more io chunks. If not, a new region has to be
// created for following chunks.
last_region_joinable: bool,
}

impl FileIoMergeState {
fn new() -> Self {
FileIoMergeState {
regions: Vec::with_capacity(8),
last_region_joinable: true,
}
}

Expand All @@ -1060,6 +1068,7 @@ impl FileIoMergeState {
) -> Result<()> {
if self.regions.is_empty() || !self.joinable(region_type) {
self.regions.push(Region::new(region_type));
self.last_region_joinable = true;
}

let idx = self.regions.len() - 1;
Expand All @@ -1068,6 +1077,13 @@ impl FileIoMergeState {
.map_err(|e| einval!(e))
}

// Committing current region ensures a new region will be created when more
// chunks has to be added since `push` checks if newly pushed chunk is continuous
// After committing, following `push` will create a new region.
fn commit(&mut self) {
self.last_region_joinable = false;
}

fn reset(&mut self) {
self.regions.truncate(0);
}
Expand All @@ -1077,7 +1093,7 @@ impl FileIoMergeState {
debug_assert!(!self.regions.is_empty());
let idx = self.regions.len() - 1;

self.regions[idx].r#type.joinable(region_type)
self.regions[idx].r#type.joinable(region_type) && self.last_region_joinable
}
}

Expand Down