Refactor resource restriction handling in WorkerState
#6672
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 15 files ±0 15 suites ±0 6h 30m 22s ⏱️ - 2m 1s For more details on these failures, see this check. Results for commit b1f2557. ± Comparison against base commit 1a28011. ♻️ This comment has been updated with latest results. |
hendrikmakait
force-pushed
the
resources-in-worker-state
branch
from
90b7823 to
01d1dc8
Compare
hendrikmakait
changed the title
total_resources to WorkerStateWorkerState
hendrikmakait
changed the title
WorkerStateWorkerState
distributed/worker_state_machine.py
Outdated
| else: | ||
| self.available_resources[r] = quantity | ||
| self.total_resources[r] = quantity | ||
|
|
There was a problem hiding this comment.
There are many problems with the code of Worker.set_resources. It would deserve a much more thorough overhaul.
For the purpose of this PR, I'd rather revert this and just tamper with the state directly from Worker.set_resources.
There was a problem hiding this comment.
One issue I see with this is that this method would be quite useful for tests that are currently setting attributes directly which results in a technically invalid worker state (https://github.com/dask/distributed/blob/main/distributed/utils_test.py#L2452). Given that you have already set up a ticket for the more thorough overhaul (#6677), I'd prefer leaving this as is, in particular since a follow-up PR will add more validations of worker state that check for consistency between total_resources, available_resources and any currently acquired resources.
There was a problem hiding this comment.
it would need to be a SetResourcesEvent anyway. I'd rather not have a throwaway refactoring.
There was a problem hiding this comment.
Done, updated ws_with_running_task as well.
Co-authored-by: crusaderky <[email protected]>
|
Could you please also add a line to |
|
Thank you! |
total_resourcesintoWorkerState.WorkerState.validate_stateas these would currently fail. This will be done in a follow-up PR.Sets foundation for and therefore blocks fixes for #6663.