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

Support graceful and quick farmer shutdown in the middle of plotting process #2100

Merged
merged 3 commits into from
Oct 14, 2023

Conversation

nazar-pc
Copy link
Member

@nazar-pc nazar-pc commented Oct 12, 2023

There were a few reports about this and it annoyed me as well.

Essentially what was happening here is a blocking operation of plotting that outside select couldn't interrupt until sector plotting finishes, which in case of network issues can take a very long time.

Here we do two things:

  • since plot_sector is async by itself, we race it with shutdown notification
  • inside of plot_sector we additionally yield to the async executor after each piece processed such that plotting can be interrupted in the middle of that compute-intensive process as well, not just during async piece downloading

Node that there are no yields when sector is ready and is being written to disk because this is both wasteful and shouldn't take a lot of time anyway.

Review with whitespace changes ignored, the diff is not that large.

Code contributor checklist:

rahulksnv
rahulksnv previously approved these changes Oct 14, 2023
crates/subspace-farmer-components/src/plotting.rs Outdated Show resolved Hide resolved
@nazar-pc nazar-pc added this pull request to the merge queue Oct 14, 2023
@nazar-pc nazar-pc removed this pull request from the merge queue due to a manual request Oct 14, 2023
@nazar-pc nazar-pc disabled auto-merge October 14, 2023 07:10
@nazar-pc nazar-pc merged commit 12ff066 into main Oct 14, 2023
10 checks passed
@nazar-pc nazar-pc deleted the quick-plotting-shutdown branch October 14, 2023 07:10
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.

2 participants