Closed FlorianUekermann closed 1 year ago
Can you provide a concrete example of a use case the current API does not support? You can already set the limit to 0 to forbid new stream credit from being issued whenever you want.
It's pretty orthogonal. The current way limits stream concurrency. A precise max is useful if you want to allow concurrency, but still provide precise stream level backpressure due to another resource being limited.
A worker thread pool is shared across multiple client connections. Feeding the sending side on the client has a high startup cost, so it is desirable that the client can't open the stream before a worker slot is actually available. But concurrency is desired.
DB connections per server instance are limited, but clients can't cache the data for long. So streams that the client can open must actually be serviced. The backpressure is used on the client to decide to open additional connections to other servers.
This is super nice with quic out of the box, with no additional protocol layer needed to relay the quota. If you can only limit concurrency, stream completion will implicitly increase the quota.
The current way limits stream concurrency
In what way does the current API limit concurrency?
give a client a precise quota
Using stream ID limits this way would prevent setting a lesser bound on the number of concurrent streams, which I think is almost always a far more immediate concern. Stream ID limits are for flow control, to limit instantaneous transport level resource use; they're not well suited to encoding arbitrary application-layer limits, since e.g. they can never be lowered.
The current way limits stream concurrency
In what way does the current API limit concurrency?
It doesn't. I meant that set_max_concurrent_*_streams
(the current way) only allows limiting concurrency (as opposed to cumulative stream count). Sorry, my wording in the last comment isn't great in a few places. It's quite late over here.
give a client a precise quota
Using stream ID limits this way would prevent setting a lesser bound on the number of concurrent streams, which I think is almost always a far more immediate concern.
Limiting concurrency and the total number of streams aren't mutually exclusive. You just set a bound that expresses how many new streams you can deal with right now and adjust upwards later. If F
are finished, O
are currently open and I want to process at most N
in total for now, but limit concurrency to C
then I do min(C-O+F, N)
to calculate the current limit. What the limiting factors are is super specific to the application.
If you deal with embedded devices with specific resource constraints or workloads that cost hours of computation time and transfer X00GB of data per stream, precise back-pressure for resource allocation matters on much smaller scales than the typical concurrency limits for Quic. HTTP3 connections serving thousands of requests per second won't benefit from this.
Stream ID limits are for flow control, to limit instantaneous transport level resource use; they're not well suited to encoding arbitrary application-layer limits, since e.g. they can never be lowered.
I'm aware the limit can't be lowered, the solution is to not overcommit. I have a really hard time understanding the mismatch. If the application wants the remote to open at most 3 more streams until further notice then Quic makes that really easy to express, but afaik quinn-proto doesn't allow me to express that because as soon as a stream has been fully consumed another credit is automatically issued to the remote.
If the application wants the remote to open at most 3 more streams until further notice then Quic makes that really easy to express, but afaik quinn-proto doesn't allow me to express
This is expressed in quinn-proto by calling set_max_concurrent_streams(dir, 3)
(or n+3
) followed by set_max_concurrent_streams(dir, 0)
. We could probably document this behavior better, but it's the only way it can work, since flow control credit is irrevocable.
Great. Documenting that would be awesome. That should be completely equivalent if used correctly. This wasn't obvious and I do think it is super useful. Maybe just adding
pub fn set_max_uni_stream_id(&mut self, id: StreamId) {
let dir = Dir::Uni as usize;
let highest_idx = self.streams.max_remote[dir];
let current = self.streams.max_concurrent_remote_count[dir];
self.set_max_concurrent_uni_streams(id.index().saturating_sub(highest_idx));
self.set_max_concurrent_uni_streams(current);
}
or whatever the exact incantation is, helps users more than documenting subtleties.
I'm always a bit uncertain if I should rely on these slightly hacky ways to use quinn-proto. because if the contract isn't very explicitly documented, I don't feel like I should rely on the precise behavior being preserved until a breaking API change alerts me of a change. It's a similar story with accept
not affecting flow control. It seems totally reasonable for quinn-proto to make a small adjustment to flow control mechanisms without changing the public API. That's really hard to track without the compiler helping out.
Anyway, again, thank you for your help. I'm already feeling slightly more safe because it's been mentioned on the issue tracker once ;-). I have a lot of loose ends over here right now, but I should probably send a few PRs documenting quinn-proto details I rely on once things stabilize a bit.
I sympathize with the hesitance to rely on implementation details. Luckily, these particular details are not at risk of change, since they're more or less natural consequences of an efficient implementation. PRs to formalize/document them better would be very welcome, thanks!
Sorry, to bother you again, but I missed something yesterday.
Calculating X
for set_max_concurrent_streams(dir, X)
is not straightforward.
Would it be acceptable to expose a single one of these related infos per direction (in order of preference):
Implementations are all trivial one liners.
Being able to query the current stream concurrency and allowed concurrency for all 4 directions & initiators seems pretty useful for others as well.
If not here is the problem:
To use set_max_concurrent_streams(dir, X)
as you suggested, I have to know exactly how many streams in that direction have been closed over the lifetime of the connection (C
). Then X = L - C
, where L
is the desired lifetime stream count limit.
Or to be more precise, C
is how many streams have stopped counting towards the concurrent streams limit, which is probably streams that fulfill the following conditions (here I go assuming implementation details again :-/):
StreamEvent::Finished
having been emittedThat's really awkward, and are my conditions 1 and 2 even correct and sufficient?
Alternatively, this directly implements what I am after:
pub (crate) fn ensure_total_remote_streams(self: &mut StreamsState, dir: Dir, max: u64) {
let diff = max.saturating_sub(self.max_remote);
let old = self.max_concurrent_remote_count[dir];
self.max_concurrent_remote_count[dir] = self.allocated_remote_count.saturating_add(self.diff)
self.ensure_remote_streams(dir)
self.max_concurrent_remote_count[dir] = old;
}
I see no reason not to expose getters for the allowed stream concurrency, and even current actual stream concurrency (always >= allowed for incoming), if desired.
Awesome, thanks. I'll send a PR.
Quic gives explicit control over how many streams of each kind the remote can open via MAX_STREAMS frames. Afaik quinn-proto only exposes this as limit on the number of concurrent open streams, which is more permissive than the application may want to be. This allows for more precise and flexible back pressure control.
The config methods would probably look like this:
The condition could either be ORed or ANDed with the concurrency set by
set_max_concurrent_*_streams
.In practice OR would be used like this:
In practice AND would be used like this:
I have no preference between AND or OR semantics, both give the application sufficient control to implement any strategy it wants.
I'm happy to send a PR.