Fixes on the gRPC frontend to handle AsyncNotifyWhenDone() API

[tanmayv25]

There are few items that have been identified and fixed:

1. It is unsafe to call ctx_->IsCancelled() if the tag from AsyncNotifyWhenDone(). I am using a dummy State object that points to the first state object of ongoing context tracking the bidi grpc streaming RPC.
2. Introduced a new state step called PARTIAL_COMPLETE. We would need to handle the lifetimes of the state objects ourselves so that State is released only when all the RPC is complete. This means that callbacks have been processed, all the standard loops from the state->steps_ are complete and Done notification has been processed. The sad thing is there is no order guarantee on the standard CQ delivery and Done notification delivery.
3. There is a possibility that server when shutting down might not receive the Done() event. Exploring more on this,

I am adding some tests on the gRPC state cleanup logic for the following permutations and combinations.
[non-streaming, streaming non-decoupled, streaming decoupled][normal infer, cancelled, timeout, kill server, restricted API error].
I think if these tests pass. We should be confident on our end.

There are some open gaps that got identified in the process of this work:

The server shutdown and handler are asynchronously invoked which might lead to certain race condition. Specifically, when calling Finish() on a listening RPC within handler. There are several points where it can occur. See here. I will fix them in a separate PR as it is going out-of-context with this PR and the issue is already seem to be present in the current implementation. Opened a ticket here to track this: DLIS-5563

[rmccorm4]

General flow LGTM. Just looking for some minor comments and descriptions so this code is easier to understand when coming back to it in the future.

[rmccorm4]

From talking offline, is there an edge case here where the state has already been processed and gets stuck in PARTIAL_COMPLETION?

[tanmayv25]

I have to think about this more. As per my reading and experimentation, a race condition exists in our current implementation as well.
Specifically, we don't know at this point here: https://github.com/triton-inference-server/server/blob/main/src/grpc/stream_infer_handler.cc#L183-L195
Whether the stream was closed from the client side or the server is in shutdown. By introducing custom sleep, I was able to cause a gRPC assert error because it tries to call Finish() on a compeletion queue that is in shutdown state.

So, it is not getting stuck, but it might cause grpc assert when closing the server while running heavy streaming inference workload.

[rmccorm4]

By introducing custom sleep, I was able to cause a gRPC assert error because it tries to call Finish() on a compeletion queue that is in shutdown state.

So, it is not getting stuck, but it might cause grpc assert when closing the server while running heavy streaming inference workload.

Is this a graceful failure? or do we need to catch this somehow?

[tanmayv25]

assert_error is not a graceful error. I think a better way would be to prime the handlers for cancellation before shutting down the completion queue. I will address this issue in a separate PR.

[rmccorm4]

Ok, separate PR is fine. Add a ticket if needed for tracking.