Skip to content

[Tune] Add use_threads=False in pyarrow syncing - #32256

Merged
krfricke merged 3 commits into
ray-project:masterfrom
Yard1:pyarrow_use_threads_false
Feb 7, 2023
Merged

[Tune] Add use_threads=False in pyarrow syncing#32256
krfricke merged 3 commits into
ray-project:masterfrom
Yard1:pyarrow_use_threads_false

Conversation

@Yard1

@Yard1 Yard1 commented Feb 7, 2023

Copy link
Copy Markdown
Member

Signed-off-by: Antoni Baum antoni.baum@protonmail.com

Why are these changes needed?

Fixes a pyarrow issue where the syncing deadlocks when there are more files in a directory than available CPU cores.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@Yard1
[Tune] Add use_threads=False in pyarrow syncing
ba562e5 
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
@Yard1
Yard1 requested a review from krfricke February 7, 2023 01:16
@Yard1
Tweak
b7e9f4a 
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
@krfricke

krfricke commented Feb 7, 2023

Copy link
Copy Markdown
Contributor

For reference, this will workaround this issue: apache/arrow#32372

Kai Fricke
Only use no_threads for S3, enhance tests
d71d26e 
Signed-off-by: Kai Fricke <kai@anyscale.com>
@krfricke
krfricke marked this pull request as ready for review February 7, 2023 19:59
krfricke
krfricke approved these changes Feb 7, 2023
View reviewed changes

@krfricke krfricke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems to be passing fine, cc @Yard1 for self-review

@Yard1

Yard1 commented Feb 7, 2023

Copy link
Copy Markdown
Member Author

@krfricke looks good to me, let's merge

@krfricke
krfricke merged commit 773f7bf into ray-project:master Feb 7, 2023
@Yard1
Yard1 deleted the pyarrow_use_threads_false branch February 7, 2023 22:12
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
@Yard1 Kai Fricke @edoakes
[Tune] Add use_threads=False in pyarrow syncing (ray-project#32256)
bfe06f2 
Fixes a pyarrow issue where the syncing deadlocks when there are more files in a directory than available CPU cores.

Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
Co-authored-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@krfricke krfricke krfricke approved these changes

Assignees

@krfricke krfricke

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Yard1 @krfricke