[Core] Ray-based concurrent.futures.Executor #30826

jeicher · 2022-12-01T11:34:19Z

Why are these changes needed?

This PR provides RayExecutor, a drop-in replacement for ProcessPoolExecutor and ThreadPoolExecutor from concurrent.futures, which executes tasks on a Ray cluster.

Related issue number

Closes #29456

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 :(

Co-authored-by: Mohamed Nidabdella <mohamed.nidabdella@tweag.io> Signed-off-by: Johann Eicher <johann.eicher@tweag.io>

Signed-off-by: Johann Eicher <johann.eicher@tweag.io>

Signed-off-by: mohamed <mohamed.nidabdella@tweag.io>

jeicher · 2022-12-01T14:05:25Z

The three failing tests don't seem to have anything to do with our contribution. The Docker Bootstrap test, at the very least, failed because Ubuntu failed to update.

Perhaps someone at Ray could shed some light on the failures?

rkooo567 · 2022-12-13T15:49:05Z

sorry for the delay. I will review this tomorrow!

rkooo567

Hmm actually before deep diving into the review, is it currently only support submitting 1 task? It looks like current subclasses of Executor (ProcessExecutor and ThreadPoolExecutor) both has a concept of pool (max_workers). Should we have the same thing?

python/ray/util/ray_executor.py

rkooo567 · 2022-12-16T15:31:26Z

python/ray/util/ray_executor.py

+            return fn(*args, **kwargs)
+
+        self.__remote_fn = remote_fn
+        self.context = ray.init(ignore_reinit_error=True, **kwargs)


Should we expose context as @Property method?

The initial intent was that RayExecutor will be used in context (the with python context). So once we exit the context, ray is shutdown. I don't think we need to expose context as @property method, at least for now

python/ray/util/ray_executor.py

nidabdella · 2023-01-02T13:04:22Z

@rkooo567 can you take another look at this ?

… fixed set of Actors

jeicher · 2023-01-04T17:49:38Z

Hmm actually before deep diving into the review, is it currently only support submitting 1 task? It looks like current subclasses of Executor (ProcessExecutor and ThreadPoolExecutor) both has a concept of pool (max_workers). Should we have the same thing?

Thanks @rkooo567! I've refactored quite a bit to make multiple task submission much clearer. The executor now has a max_workers argument which will execute tasks over an actor pool. If that kwarg is not specified, multiple tasks can still be submitted to the same RayExecutor instance, but scheduling will be managed entirely by ray (i.e. no fixed set of actor/workers).

rkooo567 · 2023-01-12T14:35:47Z

thanks for the update and sorry for the delay! I am planning to review this tomorrow (have been off around new year!)

rkooo567 · 2023-01-12T14:36:01Z

btw there are lots of test failures. Do you mind merging the latest master?

nidabdella · 2023-04-26T09:11:30Z

@rkooo567 any update on this?

jjyao · 2023-04-26T16:18:33Z

Hi @nidabdella,

Could you reply to the comments I posted above? We can also schedule a meeting to go through the design and make sure we are aligned.

jeicher · 2023-05-03T15:02:24Z

Hi @nidabdella,

Could you reply to the comments I posted above? We can also schedule a meeting to go through the design and make sure we are aligned.

Thanks @jjyao, I've reverted to an Actor-based model. In the meantime, I've pinged you on Slack. Let's try and organise a chat to make sure we're on the same page.

rkooo567 · 2023-05-10T01:53:26Z

I am a bit occupied lately, so I will rely on @jjyao to finish the review!

stale · 2023-06-10T04:07:28Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

jjyao · 2023-06-14T15:37:43Z

Hi @jeicher, do you have any updates on this?

jeicher · 2023-06-14T19:59:57Z

Hi @jeicher, do you have any updates on this?

Hi @jjyao, thanks for checking. I am on leave at the moment so it will be a while before I get to this again.

stale · 2023-07-15T02:41:09Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

jeicher · 2023-07-17T13:42:25Z

Hey guys, I'll be back from leave soon. Hoping to get to this in the near future!

stale · 2023-08-11T21:20:54Z

Hi again! The issue will be closed because there has been no more activity in the 14 days since the last message.

Please feel free to reopen or open a new issue if you'd still like it to be addressed.

Again, you can always ask for help on our discussion forum or Ray's public slack channel.

Thanks again for opening the issue!

fkaleo · 2023-11-14T13:28:12Z

Just chiming in to say I love the idea :)

jeicher · 2023-11-14T14:39:42Z

@fkaleo I'll have a finished solution for this very soon 👍🏻

jeicher and others added 9 commits December 1, 2022 13:31

Added RayExecutor

dcd360b

Co-authored-by: Mohamed Nidabdella <mohamed.nidabdella@tweag.io> Signed-off-by: Johann Eicher <johann.eicher@tweag.io>

Added tests

3965a5c

Co-authored-by: Mohamed Nidabdella <mohamed.nidabdella@tweag.io> Signed-off-by: Johann Eicher <johann.eicher@tweag.io>

Added RayExecutor to API documentation

96ed566

Signed-off-by: Johann Eicher <johann.eicher@tweag.io>

formatting

8e733a5

Signed-off-by: Johann Eicher <johann.eicher@tweag.io>

added map tests with process/threadpool executors

a42f6c6

Signed-off-by: Johann Eicher <johann.eicher@tweag.io>

test formatting

23bf9c9

Signed-off-by: Johann Eicher <johann.eicher@tweag.io>

added test to check that detached named actors are cleaned up

e617dfa

Signed-off-by: Johann Eicher <johann.eicher@tweag.io>

added test for same actor instance with all tasks

46c5331

Signed-off-by: Johann Eicher <johann.eicher@tweag.io>

remove unnecessary slash

fb6f093

Signed-off-by: mohamed <mohamed.nidabdella@tweag.io>

jeicher marked this pull request as ready for review December 1, 2022 14:05

jeicher requested a review from a team as a code owner December 1, 2022 14:05

jeicher changed the title ~~Feature/executor2~~ [Core] Ray-based concurrent.futures.Executor Dec 1, 2022

richardliaw assigned rkooo567, scv119 and jjyao Dec 2, 2022

richardliaw added the core Issues that should be addressed in Ray Core label Dec 2, 2022

rkooo567 reviewed Dec 16, 2022

View reviewed changes

[UPDATE] address rkooo567 suggestions

6b4244b

jeicher added 5 commits January 4, 2023 13:16

added test to make multiple task submissions explicit

9392f4b

max_workers will spawn actor pool; submit working

2d4f546

removed actor functions; added max_workers kwarg which executes via a…

30710d0

… fixed set of Actors

linting

7e134d9

more linting

25d846f

Merge branch 'master' into feature/executor2

dff7042

jeicher added 2 commits May 3, 2023 11:46

existing instances ignore max_workers with warning

769e6d0

typing and linting update

d46bee6

jeicher closed this May 3, 2023

jeicher reopened this May 3, 2023

jeicher added 3 commits May 3, 2023 13:42

simple actor mapping

2f1cd3f

tests passing

d6e4adb

typing, linting

7f03952

jeicher added 5 commits May 3, 2023 17:38

testing actor results available after shutdown

51ce6c1

linting

72a6cbc

simplification refactor

a282756

results accessible after shutdown test

211abd8

linting

39c638e

rkooo567 removed their assignment May 10, 2023

stale bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Jun 10, 2023

jjyao removed the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Jun 14, 2023

stale bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Jul 15, 2023

stale bot closed this Aug 11, 2023

jeicher mentioned this pull request Nov 14, 2023

add RayExecutor #41126

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core] Ray-based concurrent.futures.Executor #30826

[Core] Ray-based concurrent.futures.Executor #30826

jeicher commented Dec 1, 2022

jeicher commented Dec 1, 2022

rkooo567 commented Dec 13, 2022

rkooo567 left a comment

rkooo567 Dec 16, 2022

nidabdella Dec 19, 2022

nidabdella commented Jan 2, 2023 •

edited

jeicher commented Jan 4, 2023

rkooo567 commented Jan 12, 2023

rkooo567 commented Jan 12, 2023

nidabdella commented Apr 26, 2023

jjyao commented Apr 26, 2023

jeicher commented May 3, 2023

rkooo567 commented May 10, 2023

stale bot commented Jun 10, 2023

jjyao commented Jun 14, 2023

jeicher commented Jun 14, 2023

stale bot commented Jul 15, 2023

jeicher commented Jul 17, 2023

stale bot commented Aug 11, 2023

fkaleo commented Nov 14, 2023

jeicher commented Nov 14, 2023

[Core] Ray-based concurrent.futures.Executor #30826

[Core] Ray-based concurrent.futures.Executor #30826

Conversation

jeicher commented Dec 1, 2022

Why are these changes needed?

Related issue number

Checks

jeicher commented Dec 1, 2022

rkooo567 commented Dec 13, 2022

rkooo567 left a comment

Choose a reason for hiding this comment

rkooo567 Dec 16, 2022

Choose a reason for hiding this comment

nidabdella Dec 19, 2022

Choose a reason for hiding this comment

nidabdella commented Jan 2, 2023 • edited

jeicher commented Jan 4, 2023

rkooo567 commented Jan 12, 2023

rkooo567 commented Jan 12, 2023

nidabdella commented Apr 26, 2023

jjyao commented Apr 26, 2023

jeicher commented May 3, 2023

rkooo567 commented May 10, 2023

stale bot commented Jun 10, 2023

jjyao commented Jun 14, 2023

jeicher commented Jun 14, 2023

stale bot commented Jul 15, 2023

jeicher commented Jul 17, 2023

stale bot commented Aug 11, 2023

fkaleo commented Nov 14, 2023

jeicher commented Nov 14, 2023

nidabdella commented Jan 2, 2023 •

edited