[Serve] allow gRPC deployment to use grpc context #41667

GeneDer · 2023-12-06T18:55:51Z

Why are these changes needed?

This PR passes a grpc_context to deployments if the deployment uses it to get gRPC request related info can use to set code, details, trailing metadata, and compression. The original grpc._cython.cygrpc._ServicerContext type is not serializable, so we created a RayServegRPCContext to be able to pass to the deployment. Will follow up with doc change.

Related issue number

Closes #38616

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 added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
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 :(

Signed-off-by: Gene Su <e870252314@gmail.com>

shrekris-anyscale

Nice work!

python/ray/serve/_private/grpc_util.py

python/ray/serve/_private/proxy.py

python/ray/serve/_private/proxy_request_response.py

python/ray/serve/_private/router.py

python/ray/serve/tests/test_grpc.py

python/ray/serve/api.py

Signed-off-by: Gene Su <e870252314@gmail.com>

GeneDer · 2023-12-07T17:06:50Z

Docs build completed this time https://readthedocs.com/projects/anyscale-ray/builds/1885865/ not sure why it's showing failed here 😅

GeneDer · 2023-12-07T17:43:59Z

@edoakes would be nice if you have bandwidth to take a look at this one as well🙏

edoakes

@GeneDer question on the API here: from what I remember, when using vanilla gRPC the handler function will be passed the context directly. Why not follow that paradigm instead of adding the get_grpc_context API?

python/ray/serve/grpc_util.py

GeneDer · 2023-12-07T18:06:23Z

Bc that would be a breaking change given there are already users for gRPC proxy. Also I feel since there are already mechanisms for getting request metadata such as multiplex api, following that pattern is a better alternatives (than forcing users to take an gRPC context that they might not case about)

edoakes · 2023-12-07T18:14:03Z

Bc that would be a breaking change given there are already users for gRPC proxy. Also I feel since there are already mechanisms for getting request metadata such as multiplex api, following that pattern is a better alternatives (than forcing users to take an gRPC context that they might not case about)

You can make it an optional kwarg (I think that's what gRPC does to begin with). If user-provided functions take that kwarg, we pass it, else we don't.

Signed-off-by: Gene Su <e870252314@gmail.com>

GeneDer · 2023-12-08T17:41:39Z

@edoakes @shrekris-anyscale I addressed all the comments PTAL and hopefully we can get this merged and cherry picked today🙏

python/ray/serve/_private/proxy.py

python/ray/serve/_private/replica.py

python/ray/serve/tests/test_grpc.py

python/ray/serve/_private/grpc_util.py

python/ray/serve/_private/proxy_request_response.py

python/ray/serve/grpc_util.py

python/ray/serve/tests/test_grpc.py

python/ray/serve/tests/unit/test_grpc_util.py

Signed-off-by: Gene Su <e870252314@gmail.com>

This PR passes a grpc_context to deployments if the deployment uses it to get gRPC request related info can use to set code, details, trailing metadata, and compression. The original grpc._cython.cygrpc._ServicerContext type is not serializable, so we created a RayServegRPCContext to be able to pass to the deployment. Will follow up with doc change. --------- Signed-off-by: Gene Su <e870252314@gmail.com>

… (#41751) This PR passes a grpc_context to deployments if the deployment uses it to get gRPC request related info can use to set code, details, trailing metadata, and compression. The original grpc._cython.cygrpc._ServicerContext type is not serializable, so we created a RayServegRPCContext to be able to pass to the deployment. Will follow up with doc change. Why are these changes needed? Pick of #41667 --------- Signed-off-by: Gene Su <e870252314@gmail.com>

GeneDer added 7 commits December 6, 2023 10:54

[Serve] allow grpc deployment to use grpc context

791845b

Signed-off-by: Gene Su <e870252314@gmail.com>

import RayServegRPCContext

2f48030

Signed-off-by: Gene Su <e870252314@gmail.com>

add a test for it

85e4104

Signed-off-by: Gene Su <e870252314@gmail.com>

Merge branch 'master' into pass-grpc-context

7953dce

fix tests

b138607

Signed-off-by: Gene Su <e870252314@gmail.com>

add docstring

e4898cf

Signed-off-by: Gene Su <e870252314@gmail.com>

refactor test a bit

e095189

Signed-off-by: Gene Su <e870252314@gmail.com>

GeneDer requested a review from a team December 6, 2023 20:36

GeneDer added release-blocker P0 Issue that blocks the release ray 2.9 Issues targeting Ray 2.9 release (~Q4 CY2023) labels Dec 6, 2023

GeneDer marked this pull request as ready for review December 6, 2023 20:36

GeneDer added 2 commits December 6, 2023 13:19

drop duplicated method

77b56fd

Signed-off-by: Gene Su <e870252314@gmail.com>

Merge branch 'master' into pass-grpc-context

a08c185

shrekris-anyscale reviewed Dec 6, 2023

View reviewed changes

GeneDer added 3 commits December 6, 2023 16:02

addres comments

66abf05

Signed-off-by: Gene Su <e870252314@gmail.com>

add api annotation

37e3737

Signed-off-by: Gene Su <e870252314@gmail.com>

add more comments

308a2bd

Signed-off-by: Gene Su <e870252314@gmail.com>

GeneDer requested a review from edoakes December 7, 2023 17:44

edoakes reviewed Dec 7, 2023

View reviewed changes

python/ray/serve/grpc_util.py Outdated Show resolved Hide resolved

address comments

bab5dbc

Signed-off-by: Gene Su <e870252314@gmail.com>

edoakes reviewed Dec 8, 2023

View reviewed changes

python/ray/serve/_private/proxy.py Outdated Show resolved Hide resolved

python/ray/serve/_private/proxy.py Outdated Show resolved Hide resolved

python/ray/serve/_private/replica.py Outdated Show resolved Hide resolved

python/ray/serve/tests/test_grpc.py Show resolved Hide resolved

Merge branch 'master' into pass-grpc-context

1bc0109

shrekris-anyscale reviewed Dec 8, 2023

View reviewed changes

address comments

fad4467

Signed-off-by: Gene Su <e870252314@gmail.com>

edoakes approved these changes Dec 8, 2023

View reviewed changes

edoakes merged commit 2487553 into ray-project:master Dec 8, 2023
11 of 15 checks passed

GeneDer deleted the pass-grpc-context branch December 8, 2023 21:09

GeneDer mentioned this pull request Dec 8, 2023

[Cherry-pick][Serve] allow gRPC deployment to use grpc context (#41667) #41751

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serve] allow gRPC deployment to use grpc context #41667

[Serve] allow gRPC deployment to use grpc context #41667

GeneDer commented Dec 6, 2023 •

edited

Loading

shrekris-anyscale left a comment

GeneDer commented Dec 7, 2023

GeneDer commented Dec 7, 2023

edoakes left a comment

GeneDer commented Dec 7, 2023

edoakes commented Dec 7, 2023

GeneDer commented Dec 8, 2023

[Serve] allow gRPC deployment to use grpc context #41667

[Serve] allow gRPC deployment to use grpc context #41667

Conversation

GeneDer commented Dec 6, 2023 • edited Loading

Why are these changes needed?

Related issue number

Checks

shrekris-anyscale left a comment

Choose a reason for hiding this comment

GeneDer commented Dec 7, 2023

GeneDer commented Dec 7, 2023

edoakes left a comment

Choose a reason for hiding this comment

GeneDer commented Dec 7, 2023

edoakes commented Dec 7, 2023

GeneDer commented Dec 8, 2023

GeneDer commented Dec 6, 2023 •

edited

Loading