nydusify: introduce chunkdict generate subcommand #1401

newthifans · 2023-08-15T06:31:49Z

Brief introduction

Add functionlity to generate chunk dictionary by database file and algorithm "exponential_smoothing"
Implement the command nydus-image chunkdict generate --database command

Basic Usage

nydus-image chunkdict generate --database ./metadata.db

Details

Write command “nydus-image chunkdict generate” entries
Call the database interface and get the chunk information in the database
Implement the exponential smoothing algorithm and obtain chunk dictionary
To be continued, dump chunk dictionary and save, and then compact chunk

adamqqqplay

Please fix the failing unit tests, thanks!

codecov · 2023-09-19T05:50:21Z

Codecov Report

Attention: Patch coverage is 54.43466% with 673 lines in your changes are missing coverage. Please review.

Project coverage is 61.24%. Comparing base (a0ec880) to head (6093309).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1401      +/-   ##
==========================================
- Coverage   61.34%   61.24%   -0.11%     
==========================================
  Files         144      145       +1     
  Lines       47001    48383    +1382     
  Branches    44537    45906    +1369     
==========================================
+ Hits        28835    29630     +795     
- Misses      16685    17194     +509     
- Partials     1481     1559      +78

Files	Coverage Δ
builder/src/lib.rs	`64.66% <ø> (ø)`
rafs/src/metadata/cached_v5.rs	`80.79% <100.00%> (+0.02%)`	⬆️
rafs/src/metadata/layout/v6.rs	`85.23% <100.00%> (+0.04%)`	⬆️
storage/src/cache/filecache/mod.rs	`67.25% <50.00%> (-0.25%)`	⬇️
storage/src/meta/mod.rs	`72.51% <89.47%> (+0.12%)`	⬆️
rafs/src/metadata/layout/v5.rs	`85.03% <25.00%> (-0.21%)`	⬇️
storage/src/device.rs	`68.98% <62.50%> (-0.06%)`	⬇️
utils/src/digest.rs	`88.32% <0.00%> (-3.74%)`	⬇️
contrib/nydusify/cmd/nydusify.go	`14.79% <0.00%> (-0.31%)`	⬇️
builder/src/core/context.rs	`65.94% <25.92%> (-1.09%)`	⬇️
... and 3 more

... and 2 files with indirect coverage changes

Algorithms Introduction.md

src/bin/nydus-image/main.rs

Algorithms Introduction.md

src/bin/nydus-image/main.rs

src/bin/nydus-image/deduplicate.rs

imeoer · 2023-12-25T01:43:41Z

builder/src/chunkdict_generator.rs

+        inode.set_blocks(256);
+        let node_info = NodeInfo {
+            explicit_uidgid: true,
+            src_dev: 66305,


This value is meaningless, maybe set it to zero.

imeoer · 2023-12-25T01:43:58Z

builder/src/chunkdict_generator.rs

+        let node_info = NodeInfo {
+            explicit_uidgid: true,
+            src_dev: 66305,
+            src_ino: 24772610,


imeoer · 2023-12-25T01:44:45Z

builder/src/chunkdict_generator.rs

+        let node_info = NodeInfo {
+            explicit_uidgid: true,
+            src_dev: 66305,
+            src_ino: 24775126,


imeoer · 2023-12-25T01:46:13Z

builder/src/chunkdict_generator.rs

+    ) -> Result<()> {
+        for chunk_info in chunkdict.iter() {
+            let chunk_size: u32 = chunk_info.chunk_compressed_size;
+            let file_offset = 1 as u64 * chunk_size as u64;


Is this value valid?

imeoer · 2023-12-25T01:50:01Z

builder/src/chunkdict_generator.rs

+            // update blob context
+            let (blob_index, blob_ctx) =
+                blob_mgr.get_or_cerate_blob_for_chunkdict(ctx, &chunk_info.chunk_blob_id)?;
+            if blob_ctx.blob_id.is_empty() {


A chunk must belong to a blob, when will the blob_id be empty?

imeoer · 2023-12-25T01:50:20Z

builder/src/generate.rs

@@ -0,0 +1,257 @@
+// Copyright (C) 2022 Nydus Developers. All rights reserved.


The file should be removed?

contrib/nydusify/cmd/nydusify.go

imeoer · 2023-12-25T01:56:05Z

src/bin/nydus-image/main.rs

+                                .num_args(1..),
+                        )
+                        .arg(
+                            Arg::new("digester")


Why we need this option?

imeoer · 2023-12-25T01:56:23Z

src/bin/nydus-image/main.rs

+                        )
+                        .arg(
+                            Arg::new("features")
+                                .long("features")


Why we need this option?

imeoer · 2023-12-25T02:06:46Z

src/bin/nydus-image/main.rs

+            let path_name: Vec<&str> = path.split('/').collect();
+
+            // Extract the image name and version  name from the bootstrap directory
+            let bootstrap_dir = match path_name.get(path_name.len() - 2) {


Should use https://doc.rust-lang.org/std/path/struct.Path.html#method.parent ?

imeoer · 2023-12-25T02:20:57Z

src/bin/nydus-image/main.rs

+            let full_image_name: Vec<&str> = bootstrap_dir.split(':').collect();
+            let image_name = match full_image_name.get(full_image_name.len() - 2) {
+                Some(&second_last) => second_last.to_string(),
+                None => bail!("Invalid image name"),


Please also print the image name in the error message.

imeoer · 2023-12-25T02:21:31Z

src/bin/nydus-image/main.rs

+                Some(&second_last) => second_last.to_string(),
+                None => bail!("Invalid image name"),
+            };
+            let version_name = match full_image_name.last() {


It better be let image_tag.

imeoer · 2023-12-25T02:38:04Z

src/bin/nydus-image/main.rs

+            .set_blob_accessible(matches.get_one::<String>("config").is_some());
+        build_ctx.configuration = config;
+        build_ctx.blob_storage = Some(chunkdict_bootstrap_path);
+        build_ctx.blob_features = BlobFeatures::CAP_TAR_TOC;


blob_features should be from the base bootstrap.

imeoer · 2023-12-25T02:38:31Z

src/bin/nydus-image/main.rs

+        // Build_ctx.blob_features.insert(BlobFeatures::ENCRYPTED);
+        build_ctx.features = features;
+
+        let digester = matches


digester should also be from the base bootstrap.

docs/chunk-deduplication.md

contrib/nydusify/pkg/chunkdict/generator/generator.go

imeoer · 2024-03-12T01:41:30Z

docs/chunk-deduplication.md

+
+
+
+| image_name | version number | total_image_size(OCI) | total_image_size(nydus) | total_image_size (nydus after dedulicating) | chunkdict_image_size | dedulicating rate |


Should be number of versions?

imeoer · 2024-03-12T01:42:27Z

docs/chunk-deduplication.md

+
+| image_name | version number | total_image_size(OCI) | total_image_size(nydus) | total_image_size (nydus after dedulicating) | chunkdict_image_size | dedulicating rate |
+|------------|----------------|-----------------------|-------------------------|---------------------------------------------|----------------------|-------------------|
+| redis      | 10             | 341.78                | 419.37                  | 319.48                                      | 41.87                | 23.82%            |


All image size should be MB unit.

imeoer · 2024-03-12T01:46:28Z

docs/chunk-deduplication.md

+| image_name | version number | total_image_size(OCI) | total_image_size(nydus) | total_image_size (nydus after dedulicating) | chunkdict_image_size | dedulicating rate |
+|------------|----------------|-----------------------|-------------------------|---------------------------------------------|----------------------|-------------------|
+| redis      | 10             | 341.78                | 419.37                  | 319.48                                      | 41.87                | 23.82%            |
+| ubuntu     | 10             | 290.26                | 308.59                  | 140.28                                      | 30.8                 | 54.54%            |


Let's split the table into two, one for "image info" and one for "deduplication result"

imeoer · 2024-03-12T01:47:11Z

smoke/tests/image_test.go

+
+		// jsonData := `{
+		// 	"endpoint": "oss-cn-zhangjiakou.aliyuncs.com",
+		// 	"access_key_id": "LTAI5tKHuSQQXVjSE7PgKYhf",


The AK is leaked.

imeoer · 2024-03-12T01:49:05Z

smoke/tests/image_test.go

+			ctx.Binary.Nydusify, logLevel, image2, target2v6, ctx.Binary.Builder, ctx.Env.TempDir,
+		)
+		tool.RunWithoutOutput(i.T, convertCmd2)
+		convertCmd3 := fmt.Sprintf(


Should write a helper func for image conversion, instead of writing three times.

imeoer · 2024-03-12T01:49:21Z

smoke/tests/image_test.go

+		// 	"bucket_name": "testcompact1"
+		// }`
+
+		// formattedData, err := json.MarshalIndent(json.RawMessage(jsonData), "", "  ")


Please remove the unused codes.

imeoer · 2024-03-12T01:51:13Z

smoke/tests/image_test.go

+		)
+		tool.RunWithoutOutput(i.T, checkCmd)
+
+		// Generate v5 chunkdcit


Should write a helper func for each of the v5 and v6.

imeoer · 2024-03-12T01:51:43Z

smoke/tests/image_test.go

+		)
+		tool.RunWithoutOutput(i.T, checkCmd1)
+
+        // Test v5 chunkdict convert


It seems the codes is unformatted.

imeoer · 2024-03-12T01:52:20Z

docs/chunk-deduplication.md

+
+
+### Exponential smoothing algorithm test table
+Step 1: Download 10 OCI versions of an image and count the total size


Please use the list format for steps.

newthifans requested a review from a team as a code owner August 15, 2023 06:31

newthifans requested review from liubogithub, jiangliu and luodw and removed request for a team August 15, 2023 06:31

adamqqqplay reviewed Aug 23, 2023

View reviewed changes

newthifans changed the title ~~nydus-image: introduce chunkdict generate subcommand~~ Sep 8, 2023

newthifans force-pushed the yuanzhao08 branch from dce32c0 to f43d991 Compare September 19, 2023 05:46

newthifans force-pushed the yuanzhao08 branch from de21a76 to 3c427af Compare September 19, 2023 07:32

newthifans changed the title ~~[WIP] nydus-image: introduce chunkdict generate subcommand~~ Sep 22, 2023

newthifans force-pushed the yuanzhao08 branch 4 times, most recently from 9d17ca3 to b04d80e Compare September 26, 2023 09:43

imeoer reviewed Sep 27, 2023

View reviewed changes

Algorithms Introduction.md Outdated Show resolved Hide resolved

imeoer reviewed Sep 27, 2023

View reviewed changes

src/bin/nydus-image/main.rs Outdated Show resolved Hide resolved

imeoer reviewed Sep 27, 2023

View reviewed changes

src/bin/nydus-image/main.rs Outdated Show resolved Hide resolved

imeoer reviewed Sep 27, 2023

View reviewed changes

src/bin/nydus-image/main.rs Outdated Show resolved Hide resolved

imeoer reviewed Sep 27, 2023

View reviewed changes

src/bin/nydus-image/main.rs Outdated Show resolved Hide resolved

imeoer reviewed Sep 27, 2023

View reviewed changes

Algorithms Introduction.md Outdated Show resolved Hide resolved

newthifans force-pushed the yuanzhao08 branch from b04d80e to 26af7bf Compare October 1, 2023 08:07

newthifans force-pushed the yuanzhao08 branch 6 times, most recently from e4b7288 to c7654ba Compare October 26, 2023 18:20

ccx1024cc suggested changes Oct 30, 2023

View reviewed changes

src/bin/nydus-image/main.rs Show resolved Hide resolved

src/bin/nydus-image/deduplicate.rs Outdated Show resolved Hide resolved

newthifans force-pushed the yuanzhao08 branch from ba3c206 to 7fc7008 Compare November 9, 2023 17:56