Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to cancel AsyncIO gRPC stream requests #417

Merged
merged 5 commits into from
Oct 13, 2023
Merged

Conversation

kthui
Copy link
Contributor

@kthui kthui commented Oct 11, 2023

Related PR: triton-inference-server/server#6408

This PR adds the ability to cancel AsyncIO gRPC stream requests. There are interface changes on stream_infer(), but the "syntax" on the old and new interfaces are the same, so the amount of code breaking should be minimal if any.

@kthui kthui changed the title Add ability to cancel AsyncIO gRPC stream or non-stream requests Add ability to cancel AsyncIO gRPC stream requests Oct 11, 2023
@@ -636,7 +636,7 @@ async def stream_infer(

Parameters
----------
inputs_iterator : async_generator
inputs_iterator : asynchronous iterator
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is always "asynchronous iterator" not "asynchronous generator function", because inputs_iterator is used directly without inputs_iterator().

@@ -624,7 +624,7 @@ async def infer(
except grpc.RpcError as rpc_error:
raise_error_grpc(rpc_error)

async def stream_infer(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This used to be an "asynchronous generator function" which the user calls like:

async for response in stream_infer(...):
    ...

Now, it is a normal function that returns an "asynchronous iterator", which the user can call it in the exact same way above.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@kthui kthui requested a review from tanmayv25 October 12, 2023 21:53
README.md Outdated
```

See more details about these APIs in
[grpc/\aio/\__init__.py](src/python/library/tritonclient/grpc/aio/__init__.py).
Copy link
Contributor

Choose a reason for hiding this comment

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

this rendered weirdly, might not need backslashes, please double check it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Updated.

README.md Outdated

```python
infer_task = asyncio.create_task(aio_client.infer())
await something_else
Copy link
Contributor

Choose a reason for hiding this comment

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

is this await something_else necessary/useful to the example?

Copy link
Contributor Author

@kthui kthui Oct 13, 2023

Choose a reason for hiding this comment

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

It is not necessary to demonstrate cancellation, so removed for simplicity.

A side note: we are assuming our users should know AsyncIO is single threaded. Without await between creating the task and cancelling the task, there is no context switching, so the task is cancelled without given a chance to run. Thus, the task should not have been created in the first place, in the real world.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@rmccorm4 rmccorm4 left a comment

Choose a reason for hiding this comment

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

Do these changes break any of our existing aio client examples or usage?

@kthui
Copy link
Contributor Author

kthui commented Oct 13, 2023

Do these changes break any of our existing aio client examples or usage?

No, our client example is not changed. For example, https://github.com/triton-inference-server/client/blob/main/src/python/examples/simple_grpc_aio_sequence_stream_infer_client.py#L133-L145 is not impacted.

@kthui kthui requested review from rmccorm4 and Tabrizian October 13, 2023 00:59
@kthui kthui merged commit 05c4741 into main Oct 13, 2023
3 checks passed
@kthui kthui deleted the jacky-req-cancel-aio branch October 13, 2023 20:56
kthui added a commit that referenced this pull request Oct 13, 2023
* Add ability to cancel AsyncIO gRPC stream or non-stream requests

* Add docs on AsyncIO request cancellation

* Improve AsyncIO docs

* Improve example code styling

Co-authored-by: Ryan McCormick <[email protected]>

* Skip await on documentation

---------

Co-authored-by: Ryan McCormick <[email protected]>
mc-nv pushed a commit that referenced this pull request Oct 16, 2023
* Add ability to cancel AsyncIO gRPC stream or non-stream requests

* Add docs on AsyncIO request cancellation

* Improve AsyncIO docs

* Improve example code styling



* Skip await on documentation

---------

Co-authored-by: Ryan McCormick <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants