microdot
microdot copied to clipboard
ServiceProxyProvider
looking at our stats looks like the network time is unjustified long, for example, a random query of one minute:
target.service | target.method | MAX(stats.network.time) |
---|---|---|
UserIdService | TryGetUserIds | 665.7730102539062 |
PolicyService | GetEffectiveSitePolicy | 132.62899780273438 |
SitesService | GetSite | 230.61199951171875 |
other stats of the service doesn't indicate any issues so I was searching for some time-consuming logic between RequestStartTimestamp marking and the actually httpClient.PostAsync and I have some concerns about this piece of code: https://github.com/gigya/microdot/blob/9bdf40cc9b9b224ce83afd1f5ca904a2ddee6ab2/Gigya.Microdot.ServiceProxy/ServiceProxyProvider.cs#L177-L223
-
in case of ServerCertificateVerification or ClientCertificateVerification configured, this lock contains inside the certificate loading which can take time, and we are syncly blocking any attempts to call the service which probably the intention, but what happens to the thread during this lock? https://github.com/gigya/microdot/blob/9bdf40cc9b9b224ce83afd1f5ca904a2ddee6ab2/Gigya.Microdot.ServiceProxy/HttpsAuthenticator.cs#L35 https://github.com/gigya/microdot/blob/9bdf40cc9b9b224ce83afd1f5ca904a2ddee6ab2/Gigya.Microdot.SharedLogic/Security/WindowsStoreCertificateLocator.cs#L79-L80
-
is it possible that async cache refresh is "syncly" waiting for this lock?https://github.com/gigya/microdot/blob/9bdf40cc9b9b224ce83afd1f5ca904a2ddee6ab2/Gigya.Microdot.ServiceProxy/Caching/AsyncCache.cs#L300-L314
-
in case of an unreachable error or any type of 'HTTP request exception', we are repeating the process but with tryHttps=false, I'm not sure I completely understood the flow, this is what I see:
-
creating a client for HTTPS: https://github.com/gigya/microdot/blob/9bdf40cc9b9b224ce83afd1f5ca904a2ddee6ab2/Gigya.Microdot.ServiceProxy/ServiceProxyProvider.cs#L194
-
after the request is failing, creating a client for HTTP: https://github.com/gigya/microdot/blob/9bdf40cc9b9b224ce83afd1f5ca904a2ddee6ab2/Gigya.Microdot.ServiceProxy/ServiceProxyProvider.cs#L462 https://github.com/gigya/microdot/blob/9bdf40cc9b9b224ce83afd1f5ca904a2ddee6ab2/Gigya.Microdot.ServiceProxy/ServiceProxyProvider.cs#L211-L214
-
in the next call, we are trying HTTPS again and creating a new client: https://github.com/gigya/microdot/blob/9bdf40cc9b9b224ce83afd1f5ca904a2ddee6ab2/Gigya.Microdot.ServiceProxy/ServiceProxyProvider.cs#L374
-
if the time fits we are creating two more clients for the reachability checks https://github.com/gigya/microdot/blob/9bdf40cc9b9b224ce83afd1f5ca904a2ddee6ab2/Gigya.Microdot.ServiceProxy/ServiceProxyProvider.cs#L194-L200
-
after the request is failing, creating a new client for HTTP, and so on and so on
- what am I missing here?
- are we performing every HTTP request twice?
- are we creating two new HTTP clients for every request?
- are we disposing all of these clients and handlers?
- this happens every reachability check, plus to calls attempts?
- what will happen when we are running parallel requests to the same service, even without the certificate, creating an HTTP client and handler is not the cheapest code, and do it each request and within a lock statement sounds expensive.
-
@daniel-lamberger @talweiss1982 @aviviadi what do you think?