search

NuGet / NuGetGallery

NuGet Gallery is a package repository that powers https://www.nuget.org. Use this repo for reporting NuGet.org issues.
https://www.nuget.org/
Apache License 2.0
1.52k stars 643 forks source link

[Reliability]Investigate and update HttpClient Usage in GithubVulnerability Jobs #9763

Open ryuyu opened 6 months ago

ryuyu commented 6 months ago

Both GithubVulnerabilities2Db and GithubVulnerabilities2v3 use HttpClient in a similar way that was initially introduced to work around the issue mentioned in https://github.com/NuGet/NuGetGallery/issues/9194

In https://github.com/NuGet/NuGetGallery/pull/9740, @joelverhagen mentioned that this might potentially lead to DNS issues during the lifetime of the job (see https://github.com/NuGet/NuGetGallery/pull/9740#discussion_r1409935782)

At runtime, this can potentially cause failures, but since the job will be recreated if it crashes, thus recreating the HttpClient, it shouldn't block for now (however should still be fixed).

This issue exists in both jobs and should be fixed for both in the similar manner.

ryuyu commented 6 months ago

@erdembayar made a great comment on the usage of HttpClient. See https://github.com/NuGet/NuGetGallery/pull/9740#discussion_r1431888692 for the original comment.

Echoed here: There is a limit on the number of HttpClient instances you can create, and disposing of them does not actually remove the instance of HttpClient. Eventually, you will either exhaust the number of instances you can create or experience low resource usage. That is why it is recommended to use a single instance and reuse it https://codereview.stackexchange.com/questions/69950/single-instance-of-reusable-httpclient.

It looks like there is another pattern leveraging HttpClientFactory ( https://www.c-sharpcorner.com/article/httpclientfactory-vs-httpclient-to-implement-api-calls/ ) which is worth investigating.