search

Eventual-Inc / Daft

Distributed data engine for Python/SQL designed for the cloud, powered by Rust
https://getdaft.io
Apache License 2.0
2.39k stars 170 forks source link

[BUG] Fix ray wait in RayPartitionSet #3251

Closed jaychia closed 1 week ago

jaychia commented 2 weeks ago

Closes: #3249

Ray's ray.wait is supposed to:

  1. Defaults to fetch_local=True which will supposedly fetch data to wherever the wait is called before returning
  2. Defaults to num_returns=1 which will wait until only the first item is ready before returning

This seems to not be the intended behavior here, where RayPartitionSet is trying to wait on ALL the partitions to be ready, and does not want to pull any data down to the calling site.

codspeed-hq[bot] commented 2 weeks ago

CodSpeed Performance Report

Merging #3251 will degrade performances by 26.78%

Comparing jay/wait-no-local (a3ede70) with main (f566125)

Summary

⚡ 1 improvements ❌ 2 regressions ✅ 14 untouched benchmarks

:warning: Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main jay/wait-no-local Change
❌ test_count[1 Small File] 3.2 ms 4.1 ms -22.24%
⚡ test_iter_rows_first_row[100 Small Files] 358.8 ms 283.5 ms +26.57%
❌ test_show[100 Small Files] 24 ms 32.8 ms -26.78%
codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 77.86%. Comparing base (f566125) to head (a3ede70). Report is 41 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/Eventual-Inc/Daft/pull/3251/graphs/tree.svg?width=650&height=150&src=pr&token=J430QVFE89&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Eventual-Inc)](https://app.codecov.io/gh/Eventual-Inc/Daft/pull/3251?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Eventual-Inc) ```diff @@ Coverage Diff @@ ## main #3251 +/- ## ======================================= Coverage 77.86% 77.86% ======================================= Files 643 643 Lines 79558 79558 ======================================= Hits 61947 61947 Misses 17611 17611 ``` | [Files with missing lines](https://app.codecov.io/gh/Eventual-Inc/Daft/pull/3251?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Eventual-Inc) | Coverage Δ | | |---|---|---| | [daft/runners/ray\_runner.py](https://app.codecov.io/gh/Eventual-Inc/Daft/pull/3251?src=pr&el=tree&filepath=daft%2Frunners%2Fray_runner.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Eventual-Inc#diff-ZGFmdC9ydW5uZXJzL3JheV9ydW5uZXIucHk=) | `81.01% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/Eventual-Inc/Daft/pull/3251/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Eventual-Inc)