Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

limit http request number per source through NuGet.Config #4538

Closed
joelverhagen opened this issue Feb 8, 2017 · 5 comments
Closed

limit http request number per source through NuGet.Config #4538

joelverhagen opened this issue Feb 8, 2017 · 5 comments
Assignees
Labels
Milestone

Comments

@joelverhagen
Copy link
Member

joelverhagen commented Feb 8, 2017

Problem Statement

Today, NuGet has no consistent story about the number of parallel HTTP requests allowed.

In our Visual Studio extension, I am not aware of any HTTP request throttling. We probably end up using whatever ServicePointManager.DefaultConnectionLimit is set to. In fact, it's possible VS or some other component sets this value. Needs investigation.

I am investigating a user report where their restore in Visual Studio times out. This is a graph of the number of pending HTTP requests that NuGet restore makes. The Y axis is the number pending requests. The X axis is 1 unit per output log line.

image

It's not surprised that things start timing out when over 200 HTTP requests are in flight at the same time.

Workaround

nuget.exe and dotnet restore have -DisableParallel and --disable-parallel options, respectively, which set the maximum number of parallel HTTP calls down to 1. This is good for debugging but in practice is very slow.

If the restore is never completing in Visual Studio, use dotnet restore --disable-parallel or nuget.exe restore -DisableParallel from the command line to get one successful restore. Then, subsequent restores in Visual Studio should require many fewer HTTP requests, since all of the packages are already downloaded.

Current Code

We have a NetworkProtocolUtility.SetConnectionLimit API, but this only works on .NET Framework and sets the value to 64 on non-Mono environments (Mono is set to 1). This API is not called in VS (and should not until we know how it would effect other components in the VS process and if it's even the right solution).

HttpSource has an IThrottle parameter which allows controlling the total number of pending HTTP requests. This is used for --disable-parallel by implementing IThrottle with a semaphore of size 1.

Package.config use maxDegreeOfParallelism which is set to Environment.ProcessorCount by default for dependency fetching and package download & extraction

PackageReference throttling in SourceRepositoryDependencyProvider only on mac for dependency resolving and use MaxDegreeOfConcurrency for package download and extraction. The default value for both are 16

Suggested Implementation

We should throttle the number of concurrent HTTP requests that NuGet makes. We should set a sane default and allow configuration in NuGet.config. Our goal should be that most users shouldn't have to care about this configuration value because it will provide a good balance of performance and stability.

We should also determine whether there should be throttling at a source-level (e.g. up to 8 parallel HTTP requests against nuget.org, not effecting the count against myget.org). This models ServicePointManager.DefaultConnectionLimit which is on a per remote host basis.

Spec: https://github.com/NuGet/Home/wiki/Config-Max-Http-Request-In-NuGet

@joelverhagen joelverhagen added this to the 4.0 RTM milestone Feb 8, 2017
@mrward
Copy link
Member

mrward commented Feb 9, 2017

Would this affect the use of the DefaultMaxDegreeOfParallelism? There is another issue about allowing the DefaultMaxDegreeOfParallelism to be configurable. That particular problem is more to do with limiting the number of tasks/threads being used when on Mono. However when dropping that value down to 1 the number of http requests is also reduced. Reducing/throttling the number of http requests may be an alternative way to fix the problem that can affect Xamarin Studio on Mono.

@joelverhagen
Copy link
Member Author

The plumbing to control the general maximum degree of parallelism is not in place today. I was not aware of a generic way in .NET to control this. I see ParallelOptions.MaxDegreeOfParallelism, but this seems to effect Parallel.ForEach. In NuGet, we generally achieve parallelism by creating N Task and waiting on them all using Task.WhenAll or a concurrent bag to pick them one at a time rather than using the Parallel static methods.

Just to clarify, are you suggesting that we implement a custom DefaultMaxDegreeOfParallelism property which controls the N number of tasks we run? Or is there some other approach you are suggesting?

Thanks for linking these two issues. We definitely should be thinking about them both!

@mrward
Copy link
Member

mrward commented Feb 9, 2017

The other GitHub issue was just asking for a way to be able to specify DefaultMaxDegreeOfParallelism somehow instead of if it being constant.

My main interest is whether restricting the number of http requests would fix problems that sometimes happen when running NuGet on Mono and if this is implemented whether we still need a way to set the DefaultMaxDegreeOfParallelism.

One of my colleagues was having problems with Mono on Linux running the NuGet restore which was fixed by setting the -DisableParallelProcessing flag. This flag is automatically set to true in the install command when running on Mono but the restore command does not do this. This flag then does two things:

  1. Sets the number of parallel tasks to 1 when restoring
  2. Throttles the number of http requests.

With Xamarin Studio 6.1 we set DefaultMaxDegreeOfParallelism to 1 and did not set the http throttling. Xamarin Studio 6.1 had a restricted number of thread pool threads which could result in Mono getting stuck. This restriction has been lifted in Xamarin Studio 6.2 where we are using NuGet 3.5 unmodified and I have not run into the same problem with Mono.

@joelverhagen joelverhagen modified the milestones: 4.0.1, 4.0 RTM Feb 9, 2017
@joelverhagen joelverhagen self-assigned this Feb 9, 2017
@joelverhagen joelverhagen removed their assignment May 2, 2017
@emgarten
Copy link
Member

I did some testing of different throttle limits. From corpnet (very fast) throttling only slowed things down. The test was done using restore on nuget.sln which has five feeds, this should have hit a very high number of network calls.

Throttle max Avg time in seconds
Unlimited 36
64 42
4 50
1 142

I'd like to see this done on a slower network connection, but from my initial investigation I'm not sure that throttling is the right way to go.

@emgarten emgarten modified the milestones: Future-0, Backlog Nov 9, 2017
@emgarten emgarten added Area:HttpCommunication Priority:1 High priority issues that must be resolved in the current sprint. labels Nov 9, 2017
@emgarten
Copy link
Member

emgarten commented Nov 9, 2017

There was a recent issue where azure web sites limited the number of sockets to 300. NuGet was going beyond that limit.

We should set a reasonable limit such as 200 or less.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants