[Serve] Refactor ray.serve for compatibility #4759

simon-mo · 2019-05-10T04:18:29Z

Use flask instead of starlette for http framework for now
Move example actors to a file instead of a module.
Add serve to travis ci

AmplabJenkins · 2019-05-10T04:22:38Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-Perf-Integration-PRB/745/
Test PASSed.

AmplabJenkins · 2019-05-10T04:29:54Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-Perf-Integration-PRB/746/
Test PASSed.

AmplabJenkins · 2019-05-10T06:30:38Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/14163/
Test FAILed.

AmplabJenkins · 2019-05-10T07:53:12Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-Perf-Integration-PRB/747/
Test PASSed.

AmplabJenkins · 2019-05-10T08:30:16Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-Perf-Integration-PRB/749/
Test PASSed.

AmplabJenkins · 2019-05-10T09:04:48Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/14167/
Test FAILed.

AmplabJenkins · 2019-05-10T09:42:52Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/14170/
Test FAILed.

simon-mo · 2019-05-10T15:29:29Z

@robertnishihara This PR is ready for review. (I can't request a reviewer somehow)

robertnishihara · 2019-05-12T19:09:10Z

.travis.yml

@@ -173,6 +173,11 @@ script:
  - if [ $RAY_CI_RLLIB_AFFECTED == "1" ]; then ./ci/suppress_output python python/ray/rllib/tests/test_optimizers.py; fi
  - if [ $RAY_CI_RLLIB_AFFECTED == "1" ]; then ./ci/suppress_output python python/ray/rllib/tests/test_evaluators.py; fi

+  # ray serve tests
+  - if [ $RAY_CI_SERVE_AFFECTED == "1" ] && [ $RAY_CI_PY3 == "1" ]; then ./ci/suppress_output python python/ray/experimental/serve/tests/test_actors.py; fi


This doesn't actually run the test, right? You need to do

Suggested change

- if [ $RAY_CI_SERVE_AFFECTED == "1" ] && [ $RAY_CI_PY3 == "1" ]; then ./ci/suppress_output python python/ray/experimental/serve/tests/test_actors.py; fi

- if [ $RAY_CI_SERVE_AFFECTED == "1" ] && [ $RAY_CI_PY3 == "1" ]; then ./ci/suppress_output python -m pytest -v --durations=5 --timeout=300 python/ray/experimental/serve/tests/test_actors.py; fi

same with the others.

robertnishihara · 2019-05-12T19:10:14Z

@simon-mo sorry for the delay, I'm still looking over it.

The reason for switching to flask is to support Python 3.5? Did you notice any performance changes there?

robertnishihara · 2019-05-12T21:55:55Z

python/ray/experimental/serve/example_actors.py

+are used for testing as well as demoing purpose.
+"""
+
+from __future__ import absolute_import, division, print_function


Suggested change

from __future__ import absolute_import, division, print_function

from __future__ import absolute_import

from __future__ import division

from __future__ import print_function

robertnishihara · 2019-05-13T02:06:11Z

.travis.yml

+  - if [ $RAY_CI_SERVE_AFFECTED == "1" ] && [ $RAY_CI_PY3 == "1" ]; then ./ci/suppress_output python python/ray/experimental/serve/tests/test_actors.py; fi
+  - if [ $RAY_CI_SERVE_AFFECTED == "1" ] && [ $RAY_CI_PY3 == "1" ]; then ./ci/suppress_output python python/ray/experimental/serve/tests/test_deadline_router.py; fi
+  - if [ $RAY_CI_SERVE_AFFECTED == "1" ] && [ $RAY_CI_PY3 == "1" ]; then ./ci/suppress_output python python/ray/experimental/serve/tests/test_default_app.py; fi
+
  # ray tests
  # Python3.5+ only. Otherwise we will get `SyntaxError` regardless of how we set the tester.
  - if [ $RAY_CI_PYTHON_AFFECTED == "1" ]; then python -c 'import sys;exit(sys.version_info>=(3,5))' || python -m pytest -v --durations=5 --timeout=300 python/ray/experimental/test/async_test.py; fi


Let's use the RAY_CI_PY3 variable here.

robertnishihara · 2019-05-13T02:08:02Z

ci/travis/determine_tests_to_run.py

@@ -54,6 +69,10 @@ def list_changed_files(commit_range):
                RAY_CI_RLLIB_AFFECTED = 1
                RAY_CI_LINUX_WHEELS_AFFECTED = 1
                RAY_CI_MACOS_WHEELS_AFFECTED = 1
+            elif changed_file.startswith("python/ray/experimental/serve"):
+                RAY_CI_SERVE_AFFECTED = 1


We also need to add RAY_CI_SERVE_AFFECTED = 1 to the python block, the src bloc, and both else blocks.

robertnishihara · 2019-05-13T02:08:56Z

.travis.yml

+  # ray serve tests
+  - if [ $RAY_CI_SERVE_AFFECTED == "1" ] && [ $RAY_CI_PY3 == "1" ]; then ./ci/suppress_output python python/ray/experimental/serve/tests/test_actors.py; fi
+  - if [ $RAY_CI_SERVE_AFFECTED == "1" ] && [ $RAY_CI_PY3 == "1" ]; then ./ci/suppress_output python python/ray/experimental/serve/tests/test_deadline_router.py; fi
+  - if [ $RAY_CI_SERVE_AFFECTED == "1" ] && [ $RAY_CI_PY3 == "1" ]; then ./ci/suppress_output python python/ray/experimental/serve/tests/test_default_app.py; fi


I think it'd be really great to figure out why these tests are failing when we run them with one big pytest command. I'll look into it a little.

robertnishihara · 2019-05-17T02:23:25Z

@simon-mo, I just noticed that we're calling ray.experimental.get_actor a bunch of times in the router. This call isn't particularly lightweight, and so we should be sure to do it only once per actor.

EDIT: It's not that the actual call is slow or anything, but the raylet currently has to do some bookkeeping for each actor handle.

robertnishihara · 2019-05-17T02:38:21Z

After creating a router (and not doing any work), and letting it loop for 30 seconds, there are about 100K actor handles that the raylet is keeping track of and 100K local objects.

pcmoritz · 2019-05-17T02:38:39Z

Should we use aiohttp instead of flask to avoid the blocking ray.get here? In that case we can use Ray's non-blocking async get.

robertnishihara · 2019-05-17T02:42:27Z

@pcmoritz are you talking about for the HTTP frontend? One benefit of flask is that it will work with Python 2 (which I realize we don't support yet here, but we could..).

pcmoritz · 2019-05-17T02:42:37Z

Something like this:

from ray.experimental import async_api

async_api.init()

async def handle(request):
    object_id = # Call actor
    result = await async_api.as_future(object_id)
    # Return jsonified result

simon-mo · 2019-05-17T02:44:35Z

Yes. Aiohttp has a good async support for python 3.5. I’ll start with that.

I realized flask is probably not idea because it’s a threaded server.

pcmoritz · 2019-05-17T03:00:14Z

@robertnishihara Yes, we might want to consider only supporting python 3 until somebody asks for python 2 support (which will be EOL pretty soon).

robertnishihara · 2019-05-17T07:31:54Z

Yeah I agree about not worrying about python 2 at this stage for ray.serve.

…

On Thu, May 16, 2019 at 8:00 PM Philipp Moritz ***@***.***> wrote: @robertnishihara <https://github.com/robertnishihara> Yes, we might want to consider only supporting python 3 until somebody asks for python 2 support (which will be EOL pretty soon). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4759?email_source=notifications&email_token=AAB45LJ7UHDI7VHVON4OLK3PVYNT7A5CNFSM4HL7XNX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVTS4PY#issuecomment-493301311>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAB45LM7MIXBO5OJYCC45JTPVYNT7ANCNFSM4HL7XNXQ> .

robertnishihara · 2019-05-23T01:52:23Z

Ok, once #4838 is merged, we should be able to run all of the serve tests with a single pytest command.

EDIT: Ok, #4838 partially fixed the issue, but not completely.

EDIT: Ok, once #4844 is merged, the issue should be fixed.

AmplabJenkins · 2019-05-24T23:17:59Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-Perf-Integration-PRB/937/
Test FAILed.

AmplabJenkins · 2019-05-25T01:49:09Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/14403/
Test FAILed.

simon-mo · 2019-08-26T21:55:13Z

Closed by #5541

robertnishihara reviewed May 12, 2019

View reviewed changes

robertnishihara reviewed May 13, 2019

View reviewed changes

simon-mo added 10 commits May 24, 2019 15:49

Refactor ray.serve code base

e1ccca6

Format code

69b082e

Setup ci for ray serve

0fcec26

Make sure ci instlal flask

50b9c37

Format code

41b9d52

Skip ci files

2001245

Restore setup.py

024a7e1

Fix determine_tests_to_run script

46ccdab

Format code with right version of yapf, also fix determine tests

413ccc5

Fix travis yml

64a9ebd

Combine tests into one.

eeef9d5

robertnishihara force-pushed the serve/refactor-v1 branch from 7fe2aa7 to eeef9d5 Compare May 24, 2019 22:51

simon-mo closed this Aug 26, 2019

simon-mo mentioned this pull request Aug 26, 2019

Remove previous version of ray.serve #5541

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serve] Refactor ray.serve for compatibility #4759

[Serve] Refactor ray.serve for compatibility #4759

simon-mo commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

simon-mo commented May 10, 2019

robertnishihara May 12, 2019

robertnishihara commented May 12, 2019

robertnishihara May 12, 2019

robertnishihara May 13, 2019

robertnishihara May 13, 2019

robertnishihara May 13, 2019

robertnishihara commented May 17, 2019 •

edited

Loading

robertnishihara commented May 17, 2019

pcmoritz commented May 17, 2019

robertnishihara commented May 17, 2019

pcmoritz commented May 17, 2019 •

edited

Loading

simon-mo commented May 17, 2019

pcmoritz commented May 17, 2019

robertnishihara commented May 17, 2019 via email

robertnishihara commented May 23, 2019 •

edited

Loading

AmplabJenkins commented May 24, 2019

AmplabJenkins commented May 25, 2019

simon-mo commented Aug 26, 2019

	- if [ $RAY_CI_SERVE_AFFECTED == "1" ] && [ $RAY_CI_PY3 == "1" ]; then ./ci/suppress_output python python/ray/experimental/serve/tests/test_actors.py; fi
	- if [ $RAY_CI_SERVE_AFFECTED == "1" ] && [ $RAY_CI_PY3 == "1" ]; then ./ci/suppress_output python -m pytest -v --durations=5 --timeout=300 python/ray/experimental/serve/tests/test_actors.py; fi

[Serve] Refactor ray.serve for compatibility #4759

[Serve] Refactor ray.serve for compatibility #4759

Conversation

simon-mo commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

AmplabJenkins commented May 10, 2019

simon-mo commented May 10, 2019

robertnishihara May 12, 2019

Choose a reason for hiding this comment

robertnishihara commented May 12, 2019

robertnishihara May 12, 2019

Choose a reason for hiding this comment

robertnishihara May 13, 2019

Choose a reason for hiding this comment

robertnishihara May 13, 2019

Choose a reason for hiding this comment

robertnishihara May 13, 2019

Choose a reason for hiding this comment

robertnishihara commented May 17, 2019 • edited Loading

robertnishihara commented May 17, 2019

pcmoritz commented May 17, 2019

robertnishihara commented May 17, 2019

pcmoritz commented May 17, 2019 • edited Loading

simon-mo commented May 17, 2019

pcmoritz commented May 17, 2019

robertnishihara commented May 17, 2019 via email

robertnishihara commented May 23, 2019 • edited Loading

AmplabJenkins commented May 24, 2019

AmplabJenkins commented May 25, 2019

simon-mo commented Aug 26, 2019

robertnishihara commented May 17, 2019 •

edited

Loading

pcmoritz commented May 17, 2019 •

edited

Loading

robertnishihara commented May 23, 2019 •

edited

Loading