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

[FEAT] Fix resource accounting in PyRunner #2567

Merged
merged 12 commits into from
Jul 30, 2024

Conversation

jaychia
Copy link
Contributor

@jaychia jaychia commented Jul 27, 2024

Together with #2566 , closes #2561

This PR changes the way the PyRunner performs resource accounting. Instead of updating the number of CPUs, GPUs and memory used only when futures are retrieved, we do this just before each task completes. These variables are protected with a lock to allow for concurrent access from across worker threads.

Additionally, this PR now tracks the inflight Futures across all executions globally in the PyRunner singleton. This is because there will be instances where a single execution might not be able to make forward progress (e.g. there are only 8 CPUs available, and there are 8 other currently-executing partitions). In this case, we need to wait for some execution globally to complete before attempting to make forward progress on the current execution.

@jaychia jaychia changed the title [FEAT] Separate CPU and memory resource accounting in PyRunner [FEAT] Fix resource accounting in PyRunner Jul 27, 2024
@jaychia jaychia force-pushed the jay/separate-resource-accounting-cpu-mem branch 2 times, most recently from e43a760 to 528a7bb Compare July 29, 2024 22:49
daft/runners/pyrunner.py Outdated Show resolved Hide resolved
daft/runners/pyrunner.py Outdated Show resolved Hide resolved
daft/runners/pyrunner.py Outdated Show resolved Hide resolved
Base automatically changed from jay/fix-concurrent-iters to main July 30, 2024 03:27
@github-actions github-actions bot added the enhancement New feature or request label Jul 30, 2024
@jaychia jaychia force-pushed the jay/separate-resource-accounting-cpu-mem branch from 67203b1 to 5051980 Compare July 30, 2024 18:07
@jaychia jaychia enabled auto-merge (squash) July 30, 2024 18:09
@jaychia jaychia merged commit b36b5ec into main Jul 30, 2024
44 checks passed
@jaychia jaychia deleted the jay/separate-resource-accounting-cpu-mem branch July 30, 2024 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Deadlock when iterating two dataframes concurrently
2 participants