Add ability to observe the number of connections halibut opens for ea… - #717
Add ability to observe the number of connections halibut opens for ea…#717pawelpabich wants to merge 4 commits into
Conversation
…ch polling subscription
| await IdentifyAsServerAsync(identity, cancellationToken); | ||
| await ProcessClientRequestsAsync(incomingRequestProcessor, cancellationToken); | ||
| break; | ||
| case RemoteIdentityType.Subscriber: |
There was a problem hiding this comment.
I've moved the connection limit enforcement closer to the code that it applies to.
- There is no need to perform work for other types of remote identities
- We were executing
IdentifyAsServerAsynceven for the default switch case, which results in an exception. This shows how easy it is to make a mistake if code that is case-specific is applied to all cases.
| /// <param name="currentCount"> | ||
| /// The number of active TCP connections for this subscriptionId immediately after the change | ||
| /// </param> | ||
| public void ConnectionsCountChangedFor(Uri subscriptionId, int previousCount, int currentCount); |
There was a problem hiding this comment.
I considrerd a pair of ConnectionOpendFor and ConnectionClosedFor methods but then we would keep yet another copy of <SubscriptionId, int> in the metric producer in the server. This might matter when the Server needs to deal with 20k of tentacles. Thoughts?
There was a problem hiding this comment.
Make it simpler for the caller is probably the best approach.
| readonly HalibutTimeoutsAndLimits timeoutsAndLimits; | ||
| readonly IConnectionsObserver connectionsObserver; | ||
|
|
||
| Dictionary<Uri, StrongBox<int>> activeConnectionCountPerSubscriptionId = new(); |
There was a problem hiding this comment.
This looks like the information you want.
Maybe have a method that will give you back a copy of this dictionary upon request.
|
|
||
| count.Value++; | ||
|
|
||
| connectionsObserver.ConnectionsCountChangedFor(subscriptionId, previousCount, count.Value); |
There was a problem hiding this comment.
I guess this means we need to be super careful about what ConnectionsCountChangedFor does since slow code here will impact ALL connecting tentacles one after the other.
2 participants
Related server PR https://github.com/OctopusDeploy/OctopusDeploy/pull/46535