Skip to content

Commit

Permalink
[serve] Implement timeouts for streaming & enable all tests with expe…
Browse files Browse the repository at this point in the history
…rimental streaming flag turned on (ray-project#36261)

This is the penultimate PR for turning RAY_SERVE_EXPERIMENTAL_STREAMING on by default.

In this change, we are now running all Serve tests under three conditions:

- RAY_SERVE_EXPERIMENTAL_STREAMING=0 && RAY_SERVE_ENABLE_NEW_ROUTING=0
- RAY_SERVE_EXPERIMENTAL_STREAMING=0 && RAY_SERVE_ENABLE_NEW_ROUTING=1
- RAY_SERVE_EXPERIMENTAL_STREAMING=1 && RAY_SERVE_ENABLE_NEW_ROUTING=1

(Note that RAY_SERVE_EXPERIMENTAL_STREAMING=1 implies RAY_SERVE_ENABLE_NEW_ROUTING so the latter is not explicitly set).

This required making two functionality fixes:

- Never using the streaming path for Java even if the flag is on.
- Implementing timeouts in the streaming path.

I could split these into separate PRs, but they can't really be fully tested independently, so opting to combine them here.

Signed-off-by: harborn <[email protected]>
  • Loading branch information
edoakes authored and harborn committed Aug 17, 2023
1 parent 97f1222 commit 865d97e
Show file tree
Hide file tree
Showing 24 changed files with 600 additions and 282 deletions.
37 changes: 35 additions & 2 deletions .buildkite/pipeline.build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
commands:
- ./java/test.sh

- label: ":java: Java (RAY_SERVE_ENABLE_NEW_ROUTING=1)"
- label: ":java: Java (RAY_SERVE_ENABLE_EXPERIMENTAL_STREAMING=1)"
conditions: ["RAY_CI_JAVA_AFFECTED"]
instance_size: medium
commands:
- export RAY_SERVE_ENABLE_NEW_ROUTING=1 && ./java/test.sh
- export RAY_SERVE_ENABLE_EXPERIMENTAL_STREAMING=1 && ./java/test.sh

- label: ":serverless: Dashboard Tests"
conditions:
Expand Down Expand Up @@ -110,6 +110,39 @@
--test_env=RAY_SERVE_ENABLE_NEW_ROUTING=1
$(cat test_shard.txt)

- label: ":serverless: Serve Tests (RAY_SERVE_ENABLE_EXPERIMENTAL_STREAMING=1)"
parallelism: 3
conditions:
[
"RAY_CI_SERVE_AFFECTED",
"RAY_CI_PYTHON_AFFECTED",
"RAY_CI_ML_AFFECTED",
]
instance_size: large
commands:
- cleanup() { if [ "${BUILDKITE_PULL_REQUEST}" = "false" ]; then ./ci/build/upload_build_info.sh; fi }; trap cleanup EXIT
- TORCH_VERSION=1.9.0 ./ci/env/install-dependencies.sh
- bash ./ci/ci.sh prepare_docker
- 'git clone https://github.com/wg/wrk.git /tmp/wrk && pushd /tmp/wrk && make -j && sudo cp wrk /usr/local/bin && popd'
- ./ci/env/env_info.sh
- >-
set -x;
python ./ci/run/bazel_sharding/bazel_sharding.py
--exclude_manual
--index "\${BUILDKITE_PARALLEL_JOB}" --count "\${BUILDKITE_PARALLEL_JOB_COUNT}"
--tag_filters=-post_wheel_build,-gpu
python/ray/serve/...
> test_shard.txt
- cat test_shard.txt
- bazel test --config=ci $(./ci/run/bazel_export_options)
--test_tag_filters=-post_wheel_build,-gpu
--test_env=DOCKER_HOST=tcp:https://docker:2376
--test_env=DOCKER_TLS_VERIFY=1
--test_env=DOCKER_CERT_PATH=/certs/client
--test_env=DOCKER_TLS_CERTDIR=/certs
--test_env=RAY_SERVE_ENABLE_EXPERIMENTAL_STREAMING=1
$(cat test_shard.txt)

- label: ":python: Minimal install Python {{matrix}}"
conditions: ["RAY_CI_PYTHON_AFFECTED"]
instance_size: medium
Expand Down
39 changes: 37 additions & 2 deletions .buildkite/pipeline.build_py37.yml
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@
$(cat test_shard.txt)


- label: ":cold_face: :python: :serverless: Python 3.7 Serve Tests (RAY_SERVE_USE_NEW_ROUTING=1)"
- label: ":cold_face: :python: :serverless: Python 3.7 Serve Tests (RAY_SERVE_ENABLE_NEW_ROUTING=1)"
parallelism: 3
conditions:
[
Expand Down Expand Up @@ -173,5 +173,40 @@
--test_env=DOCKER_TLS_VERIFY=1
--test_env=DOCKER_CERT_PATH=/certs/client
--test_env=DOCKER_TLS_CERTDIR=/certs
--test_env=RAY_SERVE_USE_NEW_ROUTING=1
--test_env=RAY_SERVE_ENABLE_NEW_ROUTING=1
$(cat test_shard.txt)


- label: ":cold_face: :python: :serverless: Python 3.7 Serve Tests (RAY_SERVE_ENABLE_EXPERIMENTAL_STREAMING=1)"
parallelism: 3
conditions:
[
"RAY_CI_SERVE_AFFECTED",
"RAY_CI_PYTHON_AFFECTED",
"RAY_CI_ML_AFFECTED",
]
instance_size: large
commands:
- cleanup() { if [ "${BUILDKITE_PULL_REQUEST}" = "false" ]; then ./ci/build/upload_build_info.sh; fi }; trap cleanup EXIT
- ./ci/env/install-minimal.sh 3.7
- PYTHON=3.7 TORCH_VERSION=1.9.0 ./ci/env/install-dependencies.sh
- bash ./ci/ci.sh prepare_docker
- 'git clone https://github.com/wg/wrk.git /tmp/wrk && pushd /tmp/wrk && make -j && sudo cp wrk /usr/local/bin && popd'
- ./ci/env/env_info.sh
- >-
set -x;
python ./ci/run/bazel_sharding/bazel_sharding.py
--exclude_manual
--index "\${BUILDKITE_PARALLEL_JOB}" --count "\${BUILDKITE_PARALLEL_JOB_COUNT}"
--tag_filters=-post_wheel_build,-gpu
python/ray/serve/...
> test_shard.txt
- cat test_shard.txt
- bazel test --config=ci $(./ci/run/bazel_export_options)
--test_tag_filters=-post_wheel_build,-gpu
--test_env=DOCKER_HOST=tcp:https://docker:2376
--test_env=DOCKER_TLS_VERIFY=1
--test_env=DOCKER_CERT_PATH=/certs/client
--test_env=DOCKER_TLS_CERTDIR=/certs
--test_env=RAY_SERVE_ENABLE_EXPERIMENTAL_STREAMING=1
$(cat test_shard.txt)
28 changes: 9 additions & 19 deletions python/ray/serve/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,14 @@ py_test(
deps = [":serve_lib"],
)

py_test(
name = "test_request_timeout",
size = "medium",
srcs = serve_tests_srcs,
tags = ["exclusive", "team:serve"],
deps = [":serve_lib"],
)

py_test(
name = "test_standalone",
size = "large",
Expand Down Expand Up @@ -494,29 +502,12 @@ py_test(
deps = [":serve_lib"],
)

# Runs a subset of the tests with experimental streaming turned on.
py_test(
name = "test_experimental_streaming",
size = "large",
srcs = glob(["tests/test_experimental_streaming.py",
"tests/test_api.py",
"tests/test_failure.py",
"tests/test_fastapi.py",
"tests/test_http_adapters.py",
"tests/test_http_headers.py",
"**/conftest.py"]),
tags = ["exclusive", "team:serve"],
deps = [":serve_lib"],
env = {"RAY_SERVE_ENABLE_EXPERIMENTAL_STREAMING": "1"},
)

py_test(
name = "test_streaming_response",
size = "large",
srcs = serve_tests_srcs,
tags = ["exclusive", "team:serve"],
deps = [":serve_lib"],
env = {"RAY_SERVE_ENABLE_EXPERIMENTAL_STREAMING": "1"},
)

py_test(
Expand All @@ -525,7 +516,6 @@ py_test(
srcs = serve_tests_srcs,
tags = ["exclusive", "team:serve"],
deps = [":serve_lib"],
env = {"RAY_SERVE_ENABLE_EXPERIMENTAL_STREAMING": "1"},
)

py_test(
Expand Down Expand Up @@ -613,7 +603,7 @@ py_test(

py_test(
name = "test_gradio",
size = "small",
size = "medium",
srcs = serve_tests_srcs,
tags = ["exclusive", "team:serve"],
deps = [":serve_lib"],
Expand Down
7 changes: 6 additions & 1 deletion python/ray/serve/_private/application_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,14 @@ def apply_deployment_info(
self._deployment_state_manager.deploy(deployment_name, deployment_info)

if deployment_info.route_prefix is not None:
config = deployment_info.deployment_config
self._endpoint_state.update_endpoint(
deployment_name,
EndpointInfo(route=deployment_info.route_prefix, app_name=self._name),
EndpointInfo(
route=deployment_info.route_prefix,
app_name=self._name,
app_is_cross_language=config.is_cross_language,
),
)
else:
self._endpoint_state.delete_endpoint(deployment_name)
Expand Down
9 changes: 8 additions & 1 deletion python/ray/serve/_private/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,14 @@ def deploy(
route_prefix=route_prefix,
)

updating = ray.get(self._controller.deploy.remote(**controller_deploy_args))
updating = ray.get(
self._controller.deploy.remote(
# TODO(edoakes): this is a hack because the deployment_language
# doesn't seem to get set properly from Java.
is_deployed_from_python=True,
**controller_deploy_args,
)
)

tag = self.log_deployment_update_status(name, version, updating)

Expand Down
1 change: 1 addition & 0 deletions python/ray/serve/_private/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
class EndpointInfo:
route: str
app_name: str
app_is_cross_language: bool = False


# Keep in sync with ServeReplicaState in dashboard/client/src/type/serve.ts
Expand Down
Loading

0 comments on commit 865d97e

Please sign in to comment.