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

storage: fix io error may let the chunk map bit always pending #601

Merged
merged 1 commit into from
Jul 19, 2022

Conversation

kevinXYin
Copy link
Contributor

For now , when we get an io error in do_fetch_chunks, we just return
without clearing the chunk map pending bit. This may cause the following
io always wait for the pending bits.

Signed-off-by: Xin Yin [email protected]

@yqleng1987
Copy link
Contributor

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

@yqleng1987
Copy link
Contributor

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

Test CaseTest Result
merge-target-branch✅SUCCESS
build-docker-image✅SUCCESS
compile-nydus✅SUCCESS
compile-ctr-remote✅SUCCESS
compile-nydus-snapshotter✅SUCCESS
start-nydus-snapshotter-config-containerd✅SUCCESS
run-container-with-nydus-image✅SUCCESS

Congratulations, your test job passed!

Copy link
Contributor

@hsiangkao hsiangkao left a comment

Choose a reason for hiding this comment

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

LGTM, it'd better for others to take another look.

.set_ready_and_clear_pending(chunk.as_base())
.unwrap_or_else(|e| error!("set stargz chunk ready failed, {}", e));
match Self::persist_chunk(&self.file, chunk.uncompress_offset(), &buf) {
Ok(_) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems there's more fundamental issue here:
the chunkmap bits have not been marked as pending for stargz case:(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

emm,  yeah , I think we need further check and test for the stargz case. But I do not have environment to test it. How about reverting the change for the stargz part, and maybe others can help for this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please help to open an issue for stargz and revert related change? So we could move on with non-stargz part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes , this should be better

For now , when we get an io error in do_fetch_chunks, we just return
without clearing the chunk map pending bit. This may cause the following
io always wait for the pending bits.

Signed-off-by: Xin Yin <[email protected]>
@yqleng1987
Copy link
Contributor

@kevinXYin , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/14180

@yqleng1987
Copy link
Contributor

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

Test CaseTest Result
merge-target-branch✅ SUCCESS
build-docker-image✅ SUCCESS
compile-nydus✅ SUCCESS
compile-ctr-remote✅ SUCCESS
compile-nydus-snapshotter❌ FAIL

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

@hsiangkao
Copy link
Contributor

/retest

@yqleng1987
Copy link
Contributor

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

@yqleng1987
Copy link
Contributor

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

Test CaseTest Result
merge-target-branch✅ SUCCESS
build-docker-image✅ SUCCESS
compile-nydus✅ SUCCESS
compile-ctr-remote✅ SUCCESS
compile-nydus-snapshotter✅ SUCCESS
start-nydus-snapshotter-config-containerd✅ SUCCESS
run-container-with-nydus-image✅ SUCCESS

Congratulations, your test job passed!

@jiangliu jiangliu merged commit 401c930 into dragonflyoss:master Jul 19, 2022
@kevinXYin kevinXYin deleted the do_fetch_chunks-fix branch July 20, 2022 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants