icerpc / icerpc-csharp

A C# RPC framework built for QUIC, with bidirectional streaming, first-class async/await, and Protobuf support.
https://docs.icerpc.dev
Apache License 2.0
94 stars 13 forks source link

Detecting when an outgoing request is sent #4001

Open bernardnormier opened 1 month ago

bernardnormier commented 1 month ago

When we send a one-way request, the invocation completes when the outgoing request is sent. For example:

await logger.LogAsync(message); // log operation is oneway
// the message was sent. Don't know if it was received

Then, when we send a two-way request, there is no easy way to find out when the request is sent.

Task<string> task = greeter.GreetAsync(name);
string greeting = await task; // we know it was sent because we received the response

Knowing when a (two-way) request is sent is useful for flow-control. If the peer stops reading (applies back pressure), it's desirable to find out, with one-way request and also with two-way requests.

Currently, we can figure this out by decorating the OutgoingRequest's payload PipeReader. When IceRPC calls Complete on this payload, it means the request was sent or the sending failed (let's ignore payload continuations for now)

icerpc protocol: https://github.com/icerpc/icerpc-csharp/blob/4946120c56c50285e6499c6f458e6f5131ef0656/src/IceRpc/Internal/IceRpcProtocolConnection.cs#L452

ice protocol: https://github.com/icerpc/icerpc-csharp/blob/4946120c56c50285e6499c6f458e6f5131ef0656/src/IceRpc/Internal/IceProtocolConnection.cs#L381

This PipeReader decoration is doable but not convenient. And Ice provides a much simpler API to figure out when a request is sent.

Proposal

Add a new feature, IRequestSentFeature, that allows the caller to figure out when the request is sent / the sending fails:

public interface IRequestSentFeature
{
    // Called by the protocol connection implementation when the request is sent / failed to send.
    void Sent(); 
    void SendFailed(Exception exception);
    void SendCanceled();
}

public class RequestSentFeature : IRequestSentFeature
{
    public RequestSentFeature(TaskCompletionSource tcs) { ... }

    // trivial implementation using tcs.
}

"request sent successfully" includes the sending of the payload continuation when there is one.

Usage:

var tcs = new TaskCompletionSource();
var features = new FeatureConnection();
features.Set<IRequestSentFeature>(new RequestSentFeature(tcs));

Task<string> task = greeter.GreetAsync(name, features);
await tcs.Task;
// request was sent successfully
string greeting = await task;
bernardnormier commented 1 month ago

One difficulty is the interaction with the Retry interceptor. With the Retry interceptor, it's possible for a request to be sent multiple times.

pepone commented 1 month ago

Maybe we can model this around IProgress<T> with a progress feature that reports the state changes.

Sending -> Sent -> Retrying -> Sent -> Completed

bernardnormier commented 1 month ago

We could keep the simple IRequestSentFeature/RequestSentFeature as I proposed it. If your request is sent multiple times by the retry interceptor, you'll see only the first send.

This is simple and useful for flow control. Retries shouldn't be very common. You want to know if the (first) sends take a long time because the peer is not reading.

Then, we should add a separate feature that gives you more insights into retries, an IRetryFeature. I would put it in the core and it would be (should be) called by any retry interceptor, including the built-in retry interceptor. Something like:

public interceptor IRetryFeature
{
    void NewAttempt(int attempt, int maxAttempts, string? reason); // can be 1, 1, null for a non-retryable invocation
    void Sent(int attempt);
    void SendFailed(int attempt, Exception exception);
    void SendCanceled(int attempt);
}

However, I don't see an obvious default implementation for IRetryFeature. We could defer this retry feature until we understand better how it would be used.