Skip to content

add timeout when waiting for chiapos to return #19510

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

emlowe
Copy link
Contributor

@emlowe emlowe commented Apr 16, 2025

This is likely only a small benefit, but it will log an error sooner and cause a FarmingInfo message to be sent to the farmer when the timeout fires, which is possibly helpful in diagnostics. It also upgrades from a warning to an error in cases where it takes such a long time for the call to return.

I believe the python thread that is "occupied" doing the actual disk lookups via chiapos will continue to be occupied after this timeout, so I don't think this frees the thread to be used by another lookup. That is the thread is still blocked waiting for a chiapos response.

This just allows us to log the error sooner and return a message to the farmer rather than waiting for however long it takes the actual blocking_lookup call to return.

My reading of the overall code is that once all the available threads (default 30) get blocked, your harvester won't be able to process any SPs and they will pile up here as they get scheduled by run_in_executor - possibly there should be some limit set as to how many SPs can get piled up here before we stop doing that. This would need to add in some tracking of the tasks outstanding and if too many, stop scheduling them.

@emlowe emlowe added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Apr 16, 2025
Copy link
Contributor

File Coverage Missing Lines
chia/harvester/harvester_api.py 66.7% lines 264, 275-276, 279-280
Total Missing Coverage
15 lines 5 lines 66%

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label May 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog coverage-diff merge_conflict Branch has conflicts that prevent merge to main
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant