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] (ACTORS-1) Add DAFT_ENABLE_ACTOR_POOL_PROJECTS=1 feature flag and specifying concurrency #2668

Merged
merged 4 commits into from
Aug 16, 2024

Conversation

jaychia
Copy link
Contributor

@jaychia jaychia commented Aug 15, 2024

  1. Creates a new planning-time feature flag DAFT_ENABLE_ACTOR_POOL_PROJECTS=1 which controls whether or not we're using our actor-based stateful projections
  2. Adds a new .with_concurrency(...) API for stateful UDFs that will allow users to configure the concurrency of each UDF before calling it. Note that if DAFT_ENABLE_ACTOR_POOL_PROJECTS=1 is enabled then the users will have to call this, otherwise the UDF will fail to run with a ValueError.
  3. Modifies logic across our ActorPoolProject to derive its concurrency factor (previously called num_actors) from its projection instead.

@github-actions github-actions bot added the enhancement New feature or request label Aug 15, 2024
@jaychia jaychia force-pushed the jay/allow-setting-actor-pool-size branch from 5be0eb1 to 46ef51c Compare August 15, 2024 18:48
Copy link

codecov bot commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 58 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@291bd96). Learn more about missing BASE report.
Report is 9 commits behind head on main.

Files Patch % Lines
...c/daft-plan/src/physical_ops/actor_pool_project.rs 0.00% 22 Missing ⚠️
src/daft-dsl/src/functions/python/mod.rs 9.09% 20 Missing ⚠️
daft/udf.py 55.55% 4 Missing ⚠️
...rc/daft-plan/src/logical_ops/actor_pool_project.rs 20.00% 4 Missing ⚠️
src/common/daft-config/src/lib.rs 72.72% 3 Missing ⚠️
src/daft-plan/src/logical_plan.rs 0.00% 1 Missing ⚠️
...sical_optimization/rules/reorder_partition_keys.rs 0.00% 1 Missing ⚠️
src/daft-plan/src/physical_plan.rs 0.00% 1 Missing ⚠️
src/daft-plan/src/physical_planner/translate.rs 0.00% 1 Missing ⚠️
src/daft-scheduler/src/scheduler.rs 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2668   +/-   ##
=======================================
  Coverage        ?   63.73%           
=======================================
  Files           ?      979           
  Lines           ?   111588           
  Branches        ?        0           
=======================================
  Hits            ?    71121           
  Misses          ?    40467           
  Partials        ?        0           
Files Coverage Δ
daft/context.py 76.59% <100.00%> (ø)
daft/expressions/expressions.py 93.71% <ø> (ø)
src/common/daft-config/src/python.rs 67.32% <100.00%> (ø)
src/daft-dsl/src/python.rs 94.10% <100.00%> (ø)
...logical_optimization/rules/push_down_projection.rs 84.02% <100.00%> (ø)
src/daft-plan/src/logical_plan.rs 72.83% <0.00%> (ø)
...sical_optimization/rules/reorder_partition_keys.rs 80.21% <0.00%> (ø)
src/daft-plan/src/physical_plan.rs 54.24% <0.00%> (ø)
src/daft-plan/src/physical_planner/translate.rs 91.67% <0.00%> (ø)
src/daft-scheduler/src/scheduler.rs 89.10% <0.00%> (ø)
... and 5 more

@jaychia jaychia changed the title [FEAT] Add DAFT_ENABLE_ACTOR_POOL_PROJECTS=1 feature flag and specifying concurrency [FEAT] (ACTORS-1) Add DAFT_ENABLE_ACTOR_POOL_PROJECTS=1 feature flag and specifying concurrency Aug 16, 2024
Copy link
Contributor

@desmondcheongzx desmondcheongzx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

daft/udf.py Outdated Show resolved Hide resolved
Co-authored-by: Desmond Cheong <desmondcheongzx@gmail.com>
@jaychia jaychia enabled auto-merge (squash) August 16, 2024 20:52
@jaychia jaychia merged commit d5f7a26 into main Aug 16, 2024
44 checks passed
@jaychia jaychia deleted the jay/allow-setting-actor-pool-size branch August 16, 2024 21:10
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.

2 participants