-
Notifications
You must be signed in to change notification settings - Fork 204
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 generating separate blob for prefetched files #1629
base: master
Are you sure you want to change the base?
Conversation
builder/src/chunkdict_generator.rs
Outdated
let child = tree.get_node(&node.lock().unwrap().path()).unwrap(); | ||
let mut child = child.node.lock().unwrap(); | ||
child.layer_idx = prefetch_blob_index as u16; | ||
for chunk in &mut child.chunks { |
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 , nice job!
how about just construct the new blob based on the compressed chunks from the original blob here, without needing to revert_files or modify the Blob::dump
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1629 +/- ##
==========================================
- Coverage 60.51% 60.04% -0.48%
==========================================
Files 146 147 +1
Lines 49178 49567 +389
Branches 46659 47048 +389
==========================================
+ Hits 29761 29762 +1
- Misses 17636 18023 +387
- Partials 1781 1782 +1
|
blobs_dir_path.to_path_buf(), | ||
prefetch_nodes, | ||
) | ||
.unwrap(); |
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.
Error should be handled instead of calling unwrap
.
Signed-off-by: daiyongxuan <[email protected]>
smoke/tests/native_layer_test.go
Outdated
@@ -81,6 +81,7 @@ func (n *NativeLayerTestSuite) TestMakeLayers() test.Generator { | |||
ctx.Runtime.RafsMode = scenario.GetString(paramRafsMode) | |||
ctx.Runtime.EnablePrefetch = scenario.GetBool(paramEnablePrefetch) | |||
ctx.Runtime.AmplifyIO = scenario.GetUInt64(paramAmplifyIO) | |||
ctx.Runtime.ChunkDedupDb = scenario.GetString(paramChunkDedupDb) |
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.
This seems like an unnecessary merge commit.
} | ||
} | ||
|
||
impl Read for BlobNodeReader { |
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.
It seems that File
already implements Read
, why do we still need to wrap it in a BlobNodeReader
?
let mut blob_ctx = BlobContext::from(ctx, &blob_info, ChunkSource::Build)?; | ||
blob_ctx.chunk_count = 0; | ||
blob_ctx.blob_meta_info_enabled = true; | ||
let blob_writer = ArtifactWriter::new(crate::ArtifactStorage::SingleFile( |
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.
Should use ArtifactStorage::FileDir
here.
builder/src/chunkdict_generator.rs
Outdated
tree: &mut Tree, | ||
ctx: &mut BuildContext, | ||
bootstrap_mgr: &mut BootstrapManager, | ||
blobtable: &mut RafsV6BlobTable, |
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.
blob_table
, and need also to be compatible with RAFS v5.
Signed-off-by: daiyongxuan <[email protected]>
Relevant Issue #1589
The objective is to create an independent image layer to prioritize the fetching of necessary blobs during startup, enhancing performance.
Details
This introduces the "optimize" subcommand, which generates a new blob containing prefetch files and creates a new RAFS format bootstrap. This ensures compatibility with the nydus-image check and optimizes the startup process.