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

[QUIC] Decide how to handle QUIC stream limits #32079

Closed
scalablecory opened this issue Feb 11, 2020 · 14 comments · Fixed by #52704
Closed

[QUIC] Decide how to handle QUIC stream limits #32079

scalablecory opened this issue Feb 11, 2020 · 14 comments · Fixed by #52704
Assignees
Labels
area-System.Net.Quic enhancement Product code improvement that does NOT require public API changes/additions
Projects
Milestone

Comments

@scalablecory
Copy link
Contributor

scalablecory commented Feb 11, 2020

How should we be validating these limits?

/// <summary>
/// Limit on the number of bidirectional streams the peer connection can create
/// on an accepted connection.
/// Default is 100.
/// </summary>
// TODO consider constraining these limits to 0 to whatever the max of the QUIC library we are using.
public long MaxBidirectionalStreams { get; set; } = 100;
/// <summary>
/// Limit on the number of unidirectional streams the peer connection can create
/// on an accepted connection.
/// Default is 100.
/// </summary>
// TODO consider constraining these limits to 0 to whatever the max of the QUIC library we are using.
public long MaxUnidirectionalStreams { get; set; } = 100;

How do we get notified that a connection's stream limit has changed(MAX_STREAMS frame event)? This is required for HTTP/3

/// <summary>
/// Gets the maximum number of bidirectional streams that can be made to the peer.
/// </summary>
public long GetRemoteAvailableUnidirectionalStreamCount() => _provider.GetRemoteAvailableUnidirectionalStreamCount();
/// <summary>
/// Gets the maximum number of unidirectional streams that can be made to the peer.
/// </summary>
public long GetRemoteAvailableBidirectionalStreamCount() => _provider.GetRemoteAvailableBidirectionalStreamCount();

// TODO: how do we get this event?
private void OnMaximumStreamCountIncrease(long newMaximumStreamCount)
{
lock (SyncObj)
{
if (newMaximumStreamCount <= _maximumRequestStreams)
{
return;
}
_requestStreamsRemaining += (newMaximumStreamCount - _maximumRequestStreams);
_maximumRequestStreams = newMaximumStreamCount;
while (_requestStreamsRemaining != 0 && _waitingRequests.TryDequeue(out TaskCompletionSourceWithCancellation<bool> tcs))
{
if (tcs.TrySetResult(true))
{
--_requestStreamsRemaining;
}
}
}
}

@scalablecory scalablecory added this to the 5.0 milestone Feb 11, 2020
@scalablecory scalablecory added this to To Do (Low Priority) in HTTP/3 via automation Feb 11, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Feb 11, 2020
@scalablecory scalablecory moved this from To Do (Low Priority) to To Do (High Priority) in HTTP/3 Feb 11, 2020
@scalablecory scalablecory removed the untriaged New issue has not been triaged by the area owner label Feb 11, 2020
@karelz karelz added the enhancement Product code improvement that does NOT require public API changes/additions label Feb 20, 2020
@karelz karelz changed the title Decide how to handle QUIC stream limits [QUIC] Decide how to handle QUIC stream limits Mar 11, 2020
@karelz karelz modified the milestones: 5.0.0, 6.0.0 Aug 18, 2020
@geoffkizer
Copy link
Contributor

Why does HTTP3 need to be notified?

I would assume that QuicConnection itself enforces the limits. That is, if we try to create a stream that exceeds the current limit, the first operation on that QuicStream will pend until the limit is increased (or cancellation occurs, of course). Is that not how it works currently?

@geoffkizer
Copy link
Contributor

We also need to figure out the reverse issue -- meaning, how does a connection control and update its inbound stream limit.

@scalablecory
Copy link
Contributor Author

scalablecory commented Mar 4, 2021

There are three scenarios I'd like to enable here.

Just open me a stream

This is the most user-easy operation: just open a stream, and if none are available, wait for one to become available.

using QuicStream stream = await quicConnection.OpenBidirectionalStreamAsync(cancellationToken);
// await only completes once a stream is available and reserved.

Try to open a stream, without waiting

This is something useful for for both YARP and HttpClient. -- if a stream is available right now, let me use it. otherwise, fail immediately. This can be used to decide when to try a request against another server.

using QuicStream? stream = quicConnection.OpenBidirectionalStreamIfAvailable();
if (stream is not null)
{
   // stream is reserved for us and immediately available.
}
else
{
   // a null return indicates no streams available -- maybe try your HTTP/3 request on a different IP that DNS/config provided.
}

Allowing the caller to perform their own wait

This will be useful if the standard queuing behavior is not ideal. You could use it for diagnostics. A proxy might use this to start more aggressively load balancing to other hosts if the stream count is low.

There is an inherent race condition here in that, when we tell the user they have X streams available, they may, by the time they get that notification, have already used those streams elsewhere, or the peer may have given us even more streams to use. This is something ultimately I don't think we can 100% solve for the user -- they will need to do some synchronization -- but having this plus the IfAvailable overload will allow them to solve this themselves.

QuicConnection con = ...;
SemaphoreSlim sem = ...;

while(true)
{
   // note: this tells you how many new streams can be opened, not a new "maximum stream count" value.
   int newStreamCount = await con.WaitForAvailableBidirectionalStreamsAsync();
   sem.Release(newStreamCount);
}

async Task<HttpResponseMessage> SendAsync(...)
{
   await sem.WaitAsync(cancellationToken); // wait for a stream to become available.

   // Stream limits can never be decreased in QUIC, and we are managing our own queuing, so this call should always succeed.
   using QuicStream? stream = quicConnection.OpenBidirectionalStreamIfAvailable();
   Debug.Assert(stream is not null);

   // ... now send out the request.
}

@karelz
Copy link
Member

karelz commented Mar 4, 2021

@scalablecory I still don't get the value of the last scenario "Allowing the caller to perform their own wait":
Why would users want to do their own wait? Typically the increase will be 1 stream (when another stream finishes), unless server sends new higher stream count.
I think the Open and TryOpen should be enough -- either I want to wait, or I have some fallback option.

For more aggressive load balancing strategies, I think that current number of available streams is more useful data point -- how close am I to the limit, so that I can choose between multiple options without any waiting.

@ManickaP
Copy link
Member

ManickaP commented Mar 4, 2021

Meeting notes:

  • We may want to expose current available streams count.
  • We may want to implement reservation of streams (and not just rely on msquic info that may become out of date if highly concurrent scenario).
  • Async APIs might be better than just callbacks.

@scalablecory
Copy link
Contributor Author

@scalablecory I still don't get the value of the last scenario "Allowing the caller to perform their own wait". Why would users want to do their own wait?

I'm YARP and I have 2 servers I'm load balancing against.

I'm at my stream limit for both servers, so I need to wait. I don't want to pick a connection and wait -- I want to wait for the first connection to become available.

Maybe when streams become available, I don't want to wake up requests in a strictly FIFO order, but instead want to prioritize them.

Typically the increase will be 1 stream (when another stream finishes), unless server sends new higher stream count.

QUIC doesn't work this way --

In HTTP/2, you configure a maximum stream count and when one stream finishes you can open a new one.

In QUIC, a stream finishing doesn't mean you can open a new one. The remote side must send you a new chunk of streams to use. It is very similar to how HTTP/2 handles receive window updates.

@ManickaP ManickaP self-assigned this Apr 15, 2021
@ManickaP ManickaP moved this from To Do (High Priority) to In Progress in HTTP/3 Apr 15, 2021
@scalablecory
Copy link
Contributor Author

Triage: @ManickaP is doing research on this right now, will have opinions later.

@ManickaP
Copy link
Member

Notes:

Additional thoughts:

  • would we ever use QUIC_STREAM_START_FLAG_IMMEDIATE: Even if there is currently no data to send on the stream, MsQuic will inform the peer of the stream being opened. Without this flag, MsQuic will wait until data is queued on the stream.?
  • what is the level GetParam thread safety? what happens when we call get available stream count while msquic is in the middle of processing MAX_STREAMS?

@ManickaP
Copy link
Member

API suggestions:

  • OpenBidirectionalStreamIfAvailable --> bool TryOpenBidirectionalStream(out stream)
    However this API is sync and if I get it right we still should wait for START_COMPLETE event, which will happen asynchronously (I think, I have to test it first). So should we block on the event? Make the method async? Or ignore it since we know it will happen and it will happen soon?

  • I'm not 100% sure how would we properly implement WaitForAvailableBidirectionalStreamsAsync though. We can wait with reservation, or not wait at all if I get the msquic right.

@scalablecory
Copy link
Contributor Author

Triage:

The MVP here is something like scenario 3 outlined in #32079 (comment)

We should consider if we need the other APIs -- if we suspect scenario 3 will be rare, or too difficult for the average QUIC user, then we should think about adding easier APIs.

@geoffkizer
Copy link
Contributor

I'm not 100% sure how would we properly implement WaitForAvailableBidirectionalStreamsAsync though. We can wait with reservation, or not wait at all if I get the msquic right.

How does the msquic API work here?

I assume there is some way to get notified when the peer makes more streams available, right?

@ManickaP
Copy link
Member

I assume there is some way to get notified when the peer makes more streams available, right?

Yes, when the peer increases the MAX_STREAMS we get a notification. We can hook this up to this event. When I wrote it, I was thinking about what this method promises and whether it can deliver it, which depends on synchronizing your work on the connection.

@geoffkizer
Copy link
Contributor

Small thought:

Above, we have WaitForAvailableBidirectionalStreamsAsync returning the new available stream count.

I'm not sure this is useful. I suspect you usually just care that there is an available stream. And if you care about the exact available count, you can just get this yourself. It's immediately out of date anyway, so I expect the only thing people really care about is whether it's > 0 or not.

That said, I do think WaitForAvailableBidirectionalStreamsAsync needs to handle connection shutdown gracefully. So I think it should return a bool. True = streams are available. False = connection is shutting down and no more streams will ever be available. Thoughts?

@scalablecory
Copy link
Contributor Author

scalablecory commented May 5, 2021

Above, we have WaitForAvailableBidirectionalStreamsAsync returning the new available stream count.

Above proposal returns the number of new streams available, not total count available. it's so you can do a very simple semaphore.Release(x) without anything being out of date.

The confusion over what the count is here tells me it would need either a name change or do as you propose. We could move the semaphore inside of QuicConnection, I guess. We'll want to think if this would limit any designs that might have used a different mechanism for queuing things.

handle connection shutdown gracefully

Bool seems fine. if we keep it returning the new stream count, it could just return 0 to indicate end of connection.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 13, 2021
HTTP/3 automation moved this from In Progress to Done Jun 6, 2021
ManickaP added a commit that referenced this issue Jun 6, 2021
Implements the 3rd option Allowing the caller to perform their own wait from #32079 (comment)
Adds WaitForAvailable(Bidi|Uni)rectionalStreamsAsync:
- triggered by peer announcement about new streams (QUIC_CONNECTION_EVENT_TYPE.STREAMS_AVAILABLE)
- if the connection is closed/disposed, the method throws QuicConnectionAbortedException which fitted our H3 better than boolean (can be changed)
Changes stream limit type to int
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 6, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Quic enhancement Product code improvement that does NOT require public API changes/additions
Projects
No open projects
HTTP/3
  
Done
Development

Successfully merging a pull request may close this issue.

5 participants