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

Support direct use blob cache to run image #1436

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zyfjeff
Copy link
Contributor

@zyfjeff zyfjeff commented Oct 10, 2023

Relevant Issue (if applicable)

#1415
#1414

Details

In localfs mode, we can directly generate blob cache through -blob-cache-dir, in this case, Nydusd can directly use blob cache to start, but it manages the state of chunk through ChunkMap by default, and by default all chunks are not ready, and will go to babckend to download, for which I introduced disable_chunk_map config, to turn off the ability of ChunkMap, by default make all chunks are ready, although Nydusd will still visit the backend to verify the validity of the blob when initializing the backend. it requires that the original nydus blob must exist, I introduced use_cache_only config to disable Nydusd access backend

  1. add disable_chunk_map to disable ChunkMap func
  2. add use_cache_only to disable nydusd access backend

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation Update (if none of the other choices apply)

Checklist

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

@zyfjeff zyfjeff requested a review from a team as a code owner October 10, 2023 11:51
@zyfjeff zyfjeff requested review from liubogithub, imeoer and adamqqqplay and removed request for a team October 10, 2023 11:51
@zyfjeff
Copy link
Contributor Author

zyfjeff commented Oct 10, 2023

Ping @jiangliu

@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Merging #1436 (a484d43) into master (002b2f2) will decrease coverage by 0.12%.
The diff coverage is 7.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1436      +/-   ##
==========================================
- Coverage   54.85%   54.74%   -0.12%     
==========================================
  Files         123      124       +1     
  Lines       40477    40584     +107     
  Branches    40477    40584     +107     
==========================================
+ Hits        22204    22217      +13     
- Misses      17034    17130      +96     
+ Partials     1239     1237       -2     
Files Coverage Δ
storage/src/backend/mod.rs 56.07% <ø> (ø)
api/src/config.rs 72.23% <20.00%> (+0.25%) ⬆️
storage/src/cache/filecache/mod.rs 66.66% <33.33%> (-0.60%) ⬇️
storage/src/factory.rs 36.30% <40.00%> (+0.53%) ⬆️
storage/src/cache/state/noop_chunk_map.rs 5.88% <4.44%> (-27.46%) ⬇️
storage/src/backend/noop.rs 0.00% <0.00%> (ø)

... and 3 files with indirect coverage changes

@zyfjeff
Copy link
Contributor Author

zyfjeff commented Oct 11, 2023

thanks, It has all been fixed, PTAL @jiangliu

@@ -755,6 +756,9 @@ pub struct FileCacheConfig {
/// Key for data encryption, a heximal representation of [u8; 32].
#[serde(default)]
pub encryption_key: String,
/// disbale chunk map, it is assumed that all data is ready
#[serde(default)]
pub disable_chunk_map: bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this option similar to disable_indexed_map ? And should we export the option to the user?

@@ -842,6 +846,9 @@ pub struct RafsConfigV2 {
/// Filesystem prefetching configuration.
#[serde(default)]
pub prefetch: PrefetchConfigV2,
// Only use cache, don't access the backend
#[serde(default)]
pub use_cache_only: bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

The option should be put into the device.cache ?

@zyfjeff zyfjeff force-pushed the feature-cache-only branch 2 times, most recently from 49261a6 to ae5e587 Compare October 25, 2023 14:35
In localfs mode, we can directly generate blob cache through -blob-cache-dir,
in this case, Nydusd can directly use blob cache to start, but it manages the state of
chunk through ChunkMap by default, and by default all chunks are not ready,
and will go to babckend to download, for which I introduced disable_chunk_map config,
to turn off the ability of ChunkMap, by default make all chunks are ready,
although Nydusd will still visit the backend to verify the validity of the blob
when initializing the backend. it requires that the original nydus blob must exist,
I introduced use_cache_only config to disable Nydusd access backend

1. add disable_chunk_map to disable ChunkMap func
2. add use_cache_only to disable nydusd access backend

Signed-off-by: zyfjeff <[email protected]>
@@ -1260,6 +1267,9 @@ impl TryFrom<&BackendConfig> for BackendConfigV2 {
"registry" => {
config.registry = Some(serde_json::from_value(value.backend_config.clone())?);
}
"noop" => {
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need noop instead of using the localfs backend? You can amend localfs to work w/o the chunkmap. There is no need to introduce a new backend type for this IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants