storage: fix batch chunk runtime error #1355

hangvane · 2023-07-07T08:05:38Z

Relevant Issue (if applicable)

If there are Issues related to this PullRequest, please list it.

Details

Fixed batch chunk runtime error that will cause core dump when running containers built with --batch-size enabled.
Improve runtime performance of batch chunk.

Types of changes

What types of changes does your PullRequest introduce? Put an x in all the boxes that apply:

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

Go over all the following points, and put an x in all the boxes that apply.

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

anolis-bot · 2023-07-07T08:05:47Z

@hangvane , a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/83345

codecov · 2023-07-07T08:10:11Z

Codecov Report

Attention: 257 lines in your changes are missing coverage. Please review.

Comparison is base (596492b) 61.29% compared to head (c91f9fc) 61.39%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1355      +/-   ##
==========================================
+ Coverage   61.29%   61.39%   +0.09%     
==========================================
  Files         144      144              
  Lines       46597    46962     +365     
  Branches    44133    44498     +365     
==========================================
+ Hits        28562    28832     +270     
- Misses      16589    16646      +57     
- Partials     1446     1484      +38

Files	Coverage Δ
storage/src/meta/batch.rs	`97.60% <100.00%> (-0.11%)`	⬇️
storage/src/test.rs	`51.92% <100.00%> (+2.94%)`	⬆️
src/bin/nydus-image/main.rs	`0.71% <0.00%> (ø)`
builder/src/core/blob.rs	`41.33% <0.00%> (ø)`
builder/src/compact.rs	`80.32% <0.00%> (-0.25%)`	⬇️
rafs/src/metadata/cached_v5.rs	`80.76% <0.00%> (-0.30%)`	⬇️
rafs/src/metadata/direct_v5.rs	`56.31% <0.00%> (-0.31%)`	⬇️
rafs/src/metadata/layout/v5.rs	`85.23% <0.00%> (-0.21%)`	⬇️
rafs/src/metadata/md_v5.rs	`28.94% <0.00%> (-0.47%)`	⬇️
rafs/src/mock/mock_chunk.rs	`90.66% <0.00%> (-3.78%)`	⬇️
... and 10 more

... and 3 files with indirect coverage changes

anolis-bot · 2023-07-07T09:00:03Z

@hangvane , The CI test is completed, please check result:

Test Case		Test Result
nydus_ci		❌ FAIL

Sorry, your test job failed. Please get the details in the link.

Desiki-high · 2023-07-07T11:02:44Z

/retest

anolis-bot · 2023-07-07T11:04:42Z

@Desiki-high , the test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/83380

anolis-bot · 2023-07-07T11:20:42Z

@Desiki-high , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		❌ FAIL

Sorry, your test job failed. Please get the details in the link.

hangvane · 2023-07-08T02:09:46Z

/retest

anolis-bot · 2023-07-08T02:09:56Z

@hangvane , the test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/83444

anolis-bot · 2023-07-08T02:16:42Z

@hangvane , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		❌ FAIL

Sorry, your test job failed. Please get the details in the link.

hangvane · 2023-07-08T03:16:10Z

/retest

anolis-bot · 2023-07-08T03:16:21Z

@hangvane , the test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/83446

imeoer · 2023-10-25T01:55:22Z

@jiangliu Please take a look this again, thanks!

hangvane · 2023-12-19T02:06:08Z

Great Job! Please add e2e test for build/run with batch, which reproduces bugfix in master, but resolved in your pr.

E2E tests have been added to master,

nydus/smoke/tests/native_layer_test.go

Lines 43 to 45 in a4ec9b8

    
           Dimension(paramEnablePrefetch, []interface{}{false, true}). 
        
           Dimension(paramBatch, []interface{}{"0", "0x100000"}). 
        
           Dimension(paramEncrypt, []interface{}{false, true}).

and may randomly fail E2E tests. But the logs are expired and cannot be traced.
https://github.com/dragonflyoss/nydus/actions/runs/5452176504

Desiki-high · 2023-12-21T08:29:36Z

@ccx1024cc Please take a look at this again, thanks!

…fo for batch chunks. 1. `compressed_offset` is used for build-time and runtime sorting for chunk info. So we move `compressed_offset` from `BatchInflateContext` to chunk info for batch chunks. 2. the `compressed_size` for the blobs in batch mode is not correctly set. We thus fix it by setting the value of `dumped_size`. Signed-off-by: Wenhao Ren <[email protected]>

Signed-off-by: Wenhao Ren <[email protected]>

Currently, many error are output as `os error 22` lossing customized log info. So we change the Error type for correctly output and log the error info as what we expected. Signed-off-by: Wenhao Ren <[email protected]>

Read amplification for batch chunk is not correctly implemented that may crash. The read amplification is rewrited to fix this bug. A unit test for read amplification is also added for covering this code. Signed-off-by: Wenhao Ren <[email protected]>

`BlobCompressionContextInfo` is need to read batch chunk info. `BlobCCI` is introduced for simplifying the code, and decrease the times of getting this context by lazy loading. Signed-off-by: Wenhao Ren <[email protected]>

1. Add the validation for batch chunks. 2. Add unit test for `BatchInflateContext`. Signed-off-by: Wenhao Ren <[email protected]>

Signed-off-by: Wenhao Ren <[email protected]>

By passing the chunk continuous check, and correctly sort batch chunks, the prefetch request will not be interrupted by batch chunks anymore. Signed-off-by: Wenhao Ren <[email protected]>

hangvane requested a review from a team as a code owner July 7, 2023 08:05

hangvane requested review from liubin, luodw and adamqqqplay and removed request for a team July 7, 2023 08:05

anolis-bot added the anolis_testing label Jul 7, 2023

anolis-bot added anolis_test_fail and removed anolis_testing labels Jul 7, 2023

anolis-bot added anolis_testing and removed anolis_test_fail labels Jul 7, 2023

anolis-bot added anolis_test_fail and removed anolis_testing labels Jul 7, 2023

anolis-bot added anolis_testing and removed anolis_test_fail labels Jul 8, 2023

anolis-bot added anolis_test_fail and removed anolis_testing labels Jul 8, 2023

anolis-bot added anolis_testing and removed anolis_test_fail labels Jul 8, 2023

anolis-bot removed the anolis_testing label Jul 8, 2023

hangvane force-pushed the nydusd branch 4 times, most recently from 0b8c1de to 62c345e Compare October 24, 2023 02:45

hangvane force-pushed the nydusd branch from 62c345e to 6be36b3 Compare October 25, 2023 02:56

hangvane force-pushed the nydusd branch from 6be36b3 to 4da92e7 Compare November 1, 2023 09:50

hangvane requested a review from a team as a code owner November 1, 2023 09:50

hangvane requested review from bergwolf and gaius-qi and removed request for a team November 1, 2023 09:50

hangvane force-pushed the nydusd branch from 4da92e7 to f9982d6 Compare November 2, 2023 02:54

hangvane force-pushed the nydusd branch from f9982d6 to 31bc53d Compare November 9, 2023 09:27

jiangliu force-pushed the nydusd branch from 31bc53d to 1717679 Compare November 29, 2023 03:42

hangvane force-pushed the nydusd branch from 1717679 to ed3412e Compare December 19, 2023 01:59

Desiki-high force-pushed the nydusd branch from ed3412e to e1aca7d Compare December 21, 2023 03:38

Desiki-high approved these changes Dec 21, 2023

View reviewed changes

ccx1024cc approved these changes Jan 15, 2024

View reviewed changes

hangvane added 8 commits January 15, 2024 17:24

rafs: add is_batch() for BlobChunkInfo

fd421a7

Signed-off-by: Wenhao Ren <[email protected]>

storage: fix the Error type to log the error correctly

148df0a

Currently, many error are output as `os error 22` lossing customized log info. So we change the Error type for correctly output and log the error info as what we expected. Signed-off-by: Wenhao Ren <[email protected]>

storage: introduce BlobCCI for reading batch chunk info

601b275

`BlobCompressionContextInfo` is need to read batch chunk info. `BlobCCI` is introduced for simplifying the code, and decrease the times of getting this context by lazy loading. Signed-off-by: Wenhao Ren <[email protected]>

storage: add validation and unit test for batch chunks

10d62bc

1. Add the validation for batch chunks. 2. Add unit test for `BatchInflateContext`. Signed-off-by: Wenhao Ren <[email protected]>

action: add convert test for batch chunk

72f4736

Signed-off-by: Wenhao Ren <[email protected]>

storage: fix the tiny prefetch request for batch chunks

c91f9fc

By passing the chunk continuous check, and correctly sort batch chunks, the prefetch request will not be interrupted by batch chunks anymore. Signed-off-by: Wenhao Ren <[email protected]>

Desiki-high force-pushed the nydusd branch from e1aca7d to c91f9fc Compare January 15, 2024 09:24

Desiki-high merged commit 9dae4ec into dragonflyoss:master Jan 15, 2024
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

storage: fix batch chunk runtime error #1355

storage: fix batch chunk runtime error #1355

hangvane commented Jul 7, 2023 •

edited

Loading

anolis-bot commented Jul 7, 2023

codecov bot commented Jul 7, 2023 •

edited

Loading

anolis-bot commented Jul 7, 2023

Desiki-high commented Jul 7, 2023

anolis-bot commented Jul 7, 2023

anolis-bot commented Jul 7, 2023

hangvane commented Jul 8, 2023

anolis-bot commented Jul 8, 2023

anolis-bot commented Jul 8, 2023

hangvane commented Jul 8, 2023

anolis-bot commented Jul 8, 2023

imeoer commented Oct 25, 2023

hangvane commented Dec 19, 2023

Desiki-high commented Dec 21, 2023

storage: fix batch chunk runtime error #1355

storage: fix batch chunk runtime error #1355

Conversation

hangvane commented Jul 7, 2023 • edited Loading

Relevant Issue (if applicable)

Details

Types of changes

Checklist

anolis-bot commented Jul 7, 2023

codecov bot commented Jul 7, 2023 • edited Loading

Codecov Report

anolis-bot commented Jul 7, 2023

Desiki-high commented Jul 7, 2023

anolis-bot commented Jul 7, 2023

anolis-bot commented Jul 7, 2023

hangvane commented Jul 8, 2023

anolis-bot commented Jul 8, 2023

anolis-bot commented Jul 8, 2023

hangvane commented Jul 8, 2023

anolis-bot commented Jul 8, 2023

imeoer commented Oct 25, 2023

hangvane commented Dec 19, 2023

Desiki-high commented Dec 21, 2023

hangvane commented Jul 7, 2023 •

edited

Loading

codecov bot commented Jul 7, 2023 •

edited

Loading