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

fix(storage): ignore errors on shutdown. #497

Merged
merged 2 commits into from
Feb 18, 2022

Conversation

arkbriar
Copy link
Contributor

@arkbriar arkbriar commented Feb 18, 2022

Signed-off-by: arkbriar arkbriar@gmail.com

Shutdown of secondary storage might panic when the whole runtime's shutting down.

It's ok to ignore the errors here.

---- disk::_blob_slt_ stdout ----
thread 'tokio-runtime-worker' panicked at 'empty rowset', src/storage/secondary/rowset/rowset_builder.rs:104:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'disk::_blob_slt_' panicked at 'called `Result::unwrap()` on an `Err` value: ()', .../risinglight/src/storage/secondary/mod.rs:142:44

Signed-off-by: arkbriar <arkbriar@gmail.com>
Copy link
Member

@skyzh skyzh left a comment

Choose a reason for hiding this comment

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

LGTM. But the error in the PR body empty rowset should be fixed...

@skyzh skyzh enabled auto-merge (squash) February 18, 2022 07:29
@skyzh skyzh merged commit bfff925 into risinglightdb:main Feb 18, 2022
@arkbriar
Copy link
Contributor Author

arkbriar commented Feb 18, 2022

LGTM. But the error in the PR body empty rowset should be fixed...

Oops, forget about the PR. What I intended to fix never happens when they use the same runtime. I'm totally wrong about the reason. The root cause is still the problem we've talk about in #493 .

Could u please revert the commit here? I think we need to propagate the panic in compactor to the main thread to make it quit instead of wrapping errors.

@skyzh
Copy link
Member

skyzh commented Feb 18, 2022

Sure, let's get this reverted.

skyzh added a commit that referenced this pull request Feb 18, 2022
skyzh added a commit that referenced this pull request Feb 18, 2022
This reverts commit bfff925.

Signed-off-by: Alex Chi <iskyzh@gmail.com>
skyzh added a commit that referenced this pull request Feb 18, 2022
This reverts commit bfff925.

Signed-off-by: Alex Chi <iskyzh@gmail.com>
skyzh added a commit that referenced this pull request Feb 18, 2022
This reverts commit bfff925.

Signed-off-by: Alex Chi <iskyzh@gmail.com>
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

2 participants