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

[API Proposal]: [QUIC] Support managing available streams on QuicConnection #101534

Closed
ManickaP opened this issue Apr 25, 2024 · 6 comments · Fixed by #101531
Closed

[API Proposal]: [QUIC] Support managing available streams on QuicConnection #101534

ManickaP opened this issue Apr 25, 2024 · 6 comments · Fixed by #101531
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Quic
Milestone

Comments

@ManickaP
Copy link
Member

ManickaP commented Apr 25, 2024

Background and motivation

Similarly to HTTP/2, we want to introduce support for multiple HTTP/3 connections (#101535). In order to properly manage the available streams count, we need the following API additions.

QUIC protocol has baked in stream multiplexing into the transport, meaning it handles stream creation and enforcement of stream limits. This is a difference from HTTP/2 where it's the application protocol that handles that.
QUIC stream limits are exposed as an ever increasing stream id and not as a stream count. For example, QUIC server will send that it accepts streams up to id 20. The client cannot open additional streams even after closing all of the streams, it has to wait until the server sends a new, incremented max stream id (see QUIC MAX_STREAMS frame). Which leads to the shape of the API change based on the protocol definition: callback to listen to newly released stream limit so that we can use the connection for new requests.

Original request for multiple HTTP/3 connections: #51775.
Draft PR with real usage: #101531.

API Proposal

namespace System.Net.Quic;

// Existing class.
public abstract partial class QuicConnectionOptions
{
    // New callback, raised when MAX_STREAMS frame arrives, reports new capacity for stream limits.
    // First invocation with the initial values might happen during QuicConnection.ConnectAsync or QuicListener.AcceptConnectionAsync, before the QuicConnection object is handed out.
    public QuicConnectionStreamCapacityAvailableCallback? StreamCapacityAvailableCallback { get; set; }
}
// Callback definition:
// The arguments represent additional counts of released stream limits.
// The initial values will be reported by the first invocation of the callback.
public delegate void QuicConnectionStreamCapacityAvailableCallback(QuicConnection connection, QuicConnectionStreamCapacityAvailableArgs args);
// Callback arguments:
public readonly struct QuicConnectionStreamCapacityAvailableArgs
{
    public readonly int BidirectionalStreamsCountIncrement { get; init; }
    public readonly int UnidirectionalStreamsCountIncrement { get; init; }
}

API Usage

Simplified logic from Http3Connection:

// Method that creates Http3Connection. 
// Shows the slight disadvantage of passing callback in the options class together with not having public QuicConnection ctor.
// As a result, H/3 connection cannot take QuicConnection in a constructor argument and must be instantiated upfront, and only after ConnectAsync finishes, tied with QuicConnection.

Http3Connection h3Connection = new Http3Connection();
// The first invocation of h3Connection.StreamCapacityAvailableCallback will likely happen during QuicConnection.ConnectAsync before it returns the QuicConnection instance.
QuicConnection quicConnection = await QuicConnection.ConnectAsync(new QuicClientConnectionOptions()
{
    // Other options ...
    StreamCapacityAvailableCallback = h3Connection.StreamCapacityAvailableCallback
}, cancellationToken).ConfigureAwait(false);
h3Connection.InitQuicConnection(quicConnection);


internal class Http3Connection
{
    private int _availableRequestStreamsCount;

    // Using the available stream count to see whether we can process the request on the current HTTP/3 connection.
    public bool TryReserveStream()
    {
        lock (SyncObj)
        {
            if (_availableRequestStreamsCount == 0)
            {
                return false;
            }
            --_availableRequestStreamsCount;
            return true;
        }
    }

    // In case the TryReserveStream returns false, we remove the HTTP/3 connection from the pool of usable connections and wait for StreamCapacityAvailableCallback event to put it back.
    public Task WaitForAvailableStreamsAsync()
    {
        lock (SyncObj)
        {
            if (_availableRequestStreamsCount > 0)
            {
                return Task.FromResult(true);
            }

            _availableStreamsWaiter = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
            return _availableStreamsWaiter.Task;
        }
    }

    // Callback from the QUIC will increment the available stream count and unblock a waiter if there's one.
    public void StreamCapacityAvailableCallback(QuicConnection sender, QuicConnectionStreamCapacityAvailableArgs args)
    {
        lock (SyncObj)
        {
            _availableRequestStreamsCount += args.BidirectionalStreamsCountIncrement;
            _availableStreamsWaiter?.SetResult();
            _availableStreamsWaiter = null;
        }
    }
}

Alternative Designs

Using cumulative counts (not increments) in the available(uni|bi)directionalStreamsCount values

We could use the cumulative value for availableBidirectionalStreamsCount / availableUnidirectionalStreamsCount (corresponding to MAX_STREAMS). And the user would keep the cumulative value of opened + reserved streams. Also the user would need to make sure to ignore any lower value as the callbacks might swap order.

Callback arguments as individual parameters:

public delegate void QuicConnectionStreamCapacityAvailableCallback(QuicConnection connection,  int bidirectionalStreamsCountIncrement,  int unidirectionalStreamsCountIncrement);

Most of our callback do not wrap args into structs, it's easier to consume, but it isn't in any way extendable.

Using Event / Callback property on QuicConnection

The events are not very widespread tool used by .NET libraries. There are few in networking, but not much and nothing recent has been designed with them.

// Existing class.
public abstract partial class QuicConnectionOptions
{
    // New event, raised when MAX_STREAMS frame arrives, reports new capacity for stream limits.
    public event System.Net.Quic.QuicConnectionStreamCapacityAvailableEventHandler? StreamCapacityAvailable { add { } remove { } }
}
// Arguments for StreamCapacityAvailable event.
public partial class QuicConnectionStreamCapacityAvailableEventArgs : System.EventArgs
{
    internal QuicConnectionStreamCapacityAvailableEventArgs() { }
    // The additional count of released stream limit. Corresponds to new_value(AvailableBidirectionalStreamsCount) - old_value(AvailableBidirectionalStreamsCount).
    public int BidirectionalStreamsCountIncrement { get { throw null; } }
    // The additional count of released stream limit. Corresponds to new_value(AvailableUnidirectionalStreamsCount) - old_value(AvailableUnidirectionalStreamsCount).
    public int UnidirectionalStreamsCountIncrement { get { throw null; } }
}
// Event delegate definition.
public delegate void QuicConnectionStreamCapacityAvailableEventHandler(object? sender, System.Net.Quic.QuicConnectionStreamCapacityAvailableEventArgs e);

Using Tasks (alternative to events / callbacks)

It's possible to replace StreamCapacityAvailable with Tasks, one for each stream type:

public sealed partial class QuicConnection : System.IAsyncDisposable
{
    public Task<int> BidirectionalStreamsCapacityAvailable { get; }
    public Task<int> UnidirectionalStreamsCapacityAvailable { get; }
}

Disadvantage is that they have to be recreated and re-registered after each completion.

Risks

This is an advanced API that's being tailored mainly for H/3 multiple connection support (we will document this as such). The potential users must take care when designing logic around this and correctly keep the number of opened streams (decrementing the capacity).

@ManickaP ManickaP added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Apr 25, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Apr 25, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@ManickaP ManickaP removed the untriaged New issue has not been triaged by the area owner label Apr 25, 2024
@ManickaP ManickaP added this to the 9.0.0 milestone Apr 25, 2024
@ManickaP ManickaP self-assigned this Apr 25, 2024
@MihaZupan
Copy link
Member

MihaZupan commented Apr 25, 2024

The event handler doesn't need that exact shape, so you can have one that doesn't allocate, e.g.

public event Action<MyStruct> MyEvent;
public event Action<QuicConnection, QuicStreamType, int> MyEvent;

This also lets you drop the object? sender argument, or at least give it a type.


I find it confusing to have both the Available*StreamsCount properties and a notification API with increments, it's not obvious how to accurately consume that from multiple threads.

Using increments/diffs in the QuicConnectionStreamsAvailableEventArgs values

If this means dropping the properties and only exposing something like event Action<QuicStreamType, int>, that sounds reasonable to me.
It's a lot more clear to me how to implement something like your API Usage example on top of that.


Is the fact that opening a stream is always async "just" a limitation of the MsQuic API, or a more fundamental limitation?

@ManickaP
Copy link
Member Author

The event handler doesn't need that exact shape, so you can have one that doesn't allocate, e.g.

public event Action<MyStruct> MyEvent;
public event Action<QuicConnection, QuicStreamType, int> MyEvent;

This also lets you drop the object? sender argument, or at least give it a type.

None of our public APIs use this, we'd be first. I can put it up as an alternative design and let the API review committee decide.

I find it confusing to have both the Available*StreamsCount properties and a notification API with increments, it's not obvious how to accurately consume that from multiple threads.

It's not thread-safe. Available*StreamsCount will be accurate if you don't have any OpenStreamAsync in flight.

Using increments/diffs in the QuicConnectionStreamsAvailableEventArgs values

If this means dropping the properties and only exposing something like event Action<QuicStreamType, int>, that sounds reasonable to me.

What would be the value for the int? I'm not following.

It's a lot more clear to me how to implement something like your API Usage example on top of that.

Do you mean like your example you shared?

Is the fact that opening a stream is always async "just" a limitation of the MsQuic API, or a more fundamental limitation?

MsQuic limitation. MsQuic does all operations for one connection in a single thread from which notifies us about stream being opened.


More details about opening streams:
MsQuic has 2 ways to open the stream. Open the stream and if there's no capacity (no stream limit):

  • wait until there is (we get notified after that)
  • fail (we still get this info asynchronously)

We use the waiting method.

More info: I experimented with using the failing one for the H3 Multiple Connections. But even with checking the available stream count and trying not to get to a point when OpenStream would fail, the occasional fail was detrimental to the perf.

@ManickaP ManickaP added api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jun 5, 2024
@bartonjs
Copy link
Member

bartonjs commented Jun 13, 2024

Video

  • We changed a custom callback to use Action
  • We renamed... everything :)
namespace System.Net.Quic;

// Existing class.
public abstract partial class QuicConnectionOptions
{
    // New callback, raised when MAX_STREAMS frame arrives, reports new capacity for stream limits.
    // First invocation with the initial values might happen during QuicConnection.ConnectAsync or QuicListener.AcceptConnectionAsync, before the QuicConnection object is handed out.
    public Action<QuicConnection, QuicStreamCapacityChangedArgs>? StreamCapacityCallback { get; set; }
}

// Callback arguments:
public readonly struct QuicStreamCapacityChangedArgs
{
    public int BidirectionalIncrement { get; init; }
    public int UnidirectionalIncrement { get; init; }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jun 13, 2024
@JamesNK
Copy link
Member

JamesNK commented Jun 14, 2024

Should this be more generic for other types of information the server can send to the client? I see there is MAX_STREAMS, but there is also MAX_STREAMS_DATA and MAX_DATA. Do we care about those? Could we care about them in the future?

Nothing is stopping us having this callback, and then adding a different callback for each type of frame, but is that the best API?

Just want to think about the future.

@ManickaP
Copy link
Member Author

Nothing is stopping us having this callback, and then adding a different callback for each type of frame, but is that the best API?

We discussed exactly this in the team. The disadvantage of a shared callback is that it'd get called for all frames/events regardless whether you're interested in all of them or not. Yes, it can be designed in a way that the user would specify which ones are interesting to them, but then you get even more complicated thing to set up and use. So we decided to go with the specific callback here.

there is also MAX_STREAMS_DATA and MAX_DATA. Do we care about those? Could we care about them in the future?

At the moment we don't, we let MsQuic handle all the window updates and send buffering for us. Also the more interesting/actionable frame MAX_STREAM_DATA is per stream, so that'd require different callback anyway (unless the callback would mix stream and connection level events, which doesn't seem like a good idea).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Quic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants