feat: sched: Improve worker assigning logic #8447
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b623f28 to
062f297
Compare
Codecov Report
@@ Coverage Diff @@
## master #8447 +/- ##
==========================================
+ Coverage 40.56% 40.63% +0.06%
==========================================
Files 686 686
Lines 75426 75461 +35
==========================================
+ Hits 30597 30664 +67
+ Misses 39544 39484 -60
- Partials 5285 5313 +28
|
062f297 to
80133aa
Compare
arajasek
approved these changes
Apr 7, 2022
There was a problem hiding this comment.
This makes sense and is correct assuming that the initial sort produces the same order for every task (type?). That is, it never ranks 2 > 3 > 1 for sqi[0], but 3 > 2 > 1 for sqi[1].
I assume that's the case (and it's not super important that it's perfectly true, just generally true).
|
I was about to try this on a big production setup, but I'm seeing a lot of merge conflicts when trying to merge this feature branch into v1.15.1 - can you maybe pull 1.15.1 in here or is it because its on the master branch here? |
|
You should be able to just cherry-pick the two commits instead of merging: |
|
I've yet to run this unit at full speed, but so far, the results look very promising! No more 70 GET's on a single C2 / PC2 machine, but neatly distributed over all machines. Awesome work @magik6k , thanks a ton. |
This was referenced Apr 18, 2022
|
It seems to be better, but its not perfect yet. Here I have 20 storage machines, each with a worker to fetch data, but 1 machine gets assigned all the GET's. The fetch throttling seems to keeps it down to just 5 transfers, but it would be much more efficient if the other machines would pull this data in, too... The "assigned" bit in the scheduler is a bit hasty IMHO. Would it be possible to "recalculate" the "assigned" jobs every once in a while? That would probably fix the issue (as I think the selection here was made because of available diskspace). This happened after restarting the miner by the way |
|
They also all end up in the same storage pool (there's 5 per storage machine) so there's probably some selection happening for the entire loop instead of per sector |
|
Also wondering; How is this too slow? I'm fine with the scheduler taking 200ms more to schedule properly - or does it need to refresh all the workers' stats or something? |
|
Still not fixed properly in 1.15.2. Sheduling becomes much more accurate but not completely fixed. Still facing situations when some workers idle while another ones with queues with prepared/assigned (not as often as before but still). |
|
Will post here too because its still not working. I have PC2 machines available, but the scheduler throws a PC2 job in "prepared" if they happen at the same time. I run 2 pledges at once, PC1 finishes pretty much at the same time, and then; Makes no sense, because there's about 20 other PC2 workers available which are doing nothing. |
18 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issues
Should fix things like #7607
Proposed Changes
Currently when the scheduler is assigning tasks to workers, it creates a list of
task->[]worker. This list is sorted by worker utilization at the start of scheduling cycle, but it isn't re-sorted when assigning many tasks to many workers in one cycle.We could simply run the scheduling algorithm for each task in the queue separately, but this is unfortunately way too slow, so we have to do scheduling in batches, at least if we want to avoid rewriting it in a bigger way.
This PR aims to fix this issue by tracking change in worker utilization while processing the
acceptableWindowsarray, which should result in much better scheduling decisions with only small increase of compute cost in scheduling.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 signaturesPR 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