added Improvement in documentation for parallel streaming invocation…#37194
added Improvement in documentation for parallel streaming invocation…#37194classyk12 wants to merge 1 commit into
Conversation
|
Hello @classyk12 ... That's not a correct issue number for this repo. We do need an issue for a contribution such as this. Do you have the correct issue number? If not, can you open an issue for this? 👇 Use THIS LINK to open an issue for that article. After the issue is opened, update your OP☝️with the correct issue for this repo. |
| throw new HubException($"The connection is limited to {_maxConcurrentStreams} concurrent streaming invocations."); | ||
| } | ||
|
|
||
| try |
There was a problem hiding this comment.
I had to look at this a few times, but I think the limit would never be honored.
The count is decremented the moment the stream starts delivering, not when the stream actually finishes.
Every stream increments the counter, then decrements it almost instantly as soon as the stream object is returned. So by the time a second stream request comes in, the first one has already decremented. The counter is back to zero even though the first stream is still actively running.
So it seems for example, we could start 100 concurrent streams even though there is a limit of 1, because the counter never stays elevated long enough to block as intended, so no actual concurrency control.
@BrennanConroy, what would be ther correct pattern to suggest and document here? Also, should this be using InvokeStreamAsync as opposed to InvokeMethodAsync?
| `HubOptions.MaximumParallelInvocationsPerClient` controls non-streaming hub invocations only. It does not apply to streaming hub invocations. Streaming invocations are expected to be long-running and can run concurrently. To enforce a per-connection limit for streaming invocations, use a hub filter to track active stream-returning hub methods. | ||
|
|
||
| ```csharp | ||
| using System.Collections.Concurrent; |
There was a problem hiding this comment.
I think we also need:
using System.Threading.Channels;
| > This feature makes use of <xref:Microsoft.Extensions.DependencyInjection.IServiceProviderIsService>, which is optionally implemented by DI implementations. If the app's DI container doesn't support this feature, injecting services into hub methods isn't supported. | ||
|
|
||
| ### Keyed services support in dependency injection | ||
| ## Limit per-connection streaming invocations |
There was a problem hiding this comment.
This new section would need to move to after the sub section Keyed Services support in Dependancy Injection.
"Keyed services support" is conceptually a subtopic of "Inject services into a hub"
So the organizaiton would like like this:
## Inject services into a hub
(existing content)
### Keyed services support in Dependency Injection
(existing content)
## Limit per-connection streaming invocations
(new content)
## Handle events for a connection
3 participants
…s limit
Fixes #66719
Update SignalR docs to clarify that HubOptions.MaximumParallelInvocationsPerClient does not apply to streaming invocations and add guidance for limiting per-connection streaming concurrency via a hub filter.
Clarified MaximumParallelInvocationsPerClient behavior in configuration.md and versioned includes.
Added a StreamConcurrencyFilter example to hubs.md
Included guidance that streaming invocations are long-running and can run concurrently, and that hub filters are the right mechanism to enforce connection-level streaming limits
Internal previews