-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: worker: Close all storage paths on worker shutdown #9153
fix: worker: Close all storage paths on worker shutdown #9153
Conversation
18ce082
to
10dba44
Compare
|
||
// Detach any storage associated with this worker | ||
err = api.StorageDetachAll(ctx) | ||
if err != nil { |
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.
I'm of the opinion that we should continue execution since shutdown should be best effort and should not be stopped if we can't close some paths
But the linter doesn't allow me ignore the return. Is there a better way to do this here? What are your thoughts?
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.
replace err
with _
if you want to explicitly ignore a return value. Leave a helpful comment with why you're doing so :)
In this case we can probably just log the error instead of returning if we want to continue on.
Tested this today, and it closed the storage path correctly here. Not seeing any errors when running
|
Related Issues
#5227
#7570
Proposed Changes
Add an API to close all paths for a worker which is then used during worker shutdown to properly clear state
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>
fix: mempool: Introduce a cache for valid signatures
PR type
: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, testarea
: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps