NuGet Install or NuGet restore should support retry #1120

Closed
yishaigalatzer opened this Issue Aug 5, 2015 · 6 comments

Projects

None yet

6 participants

@yishaigalatzer

For V2 - We should support a retry mechanism?
For V3 - We should also consider fallback to other source as part of retry?

Lets review the design and file appropriate follow up bugs for things we don't think we can address right away

@yishaigalatzer yishaigalatzer added this to the 3.1.0-commandline milestone Aug 5, 2015
@yishaigalatzer yishaigalatzer changed the title from NuGet Install or NuGet restore should support restore to NuGet Install or NuGet restore should support retry Aug 6, 2015
@johnataylor
Member

There are some things to consider here:

  1. Regular best-practice http error handling. If you get a 5XX you generally retry. In particular a 503 may even include a standard http Retry-After header. If such a header is present the client should retry after the interval specified. But even if this header is present the client might chose to ignore it and retry sooner. It’s up to the client really. A smart client might implement some kind of back off policy, for example, retry immediately and then exponentially up to some max like 5mins. at which perhaps it retries on that period.

  2. Generally you wouldn’t retry on a 4XX, for example a 404. For example in the feed2catalog job we retry on everything except a 404. However, we might decide to do that because we have some particular knowledge about how our service works. In other words we might have, implicitly, an application protocol that we are following. For example, we might know that there might be delays for things to exist (because, for example, we can’t control ordering in a scalable way) and so we decide the application protocol is to retry even on a 404 from certain calls. You could even imagine a scenario of retrying on a 403 because sometimes access permissions take time to propagate across servers. Generally you wouldn’t automatically retry for a 403.

  3. It’s important that a client doesn’t go crazy retrying too fast. Firstly it’s not going to help if the server is having trouble. And secondly, the client doesn’t want to get itself labelled as an attacker and blocked by the server.

  4. We have an additional application level protocol specifically designed to increase the availability of the search endpoints to that of the static endpoints. This is the “round-robin” behavior with respect to the index.json entries. Really we have only taken baby-steps here, but even those should be a big help. And because we used JSON-LD to express ourselves we have a lot of potential to easily expand on this behavior. For example, we have a human-readable comment in the file that describes things as “primary” and “secondary.” But perhaps we will decide that it would be useful for the code to understand this distinction: at which point we simply add a machine-readable property asserting that as part of the description of that endpoint. Other properties might indicate the geographical location of the service: something else the client might like to consider.

  5. Something to note is that our application protocol we run with respect to the index.json file is additive to basic http error handling. In other words, for example, even just getting the index.json file in the first place would mean being able to handle 503 etc.

  6. The exact behavior depends on the scenario. For example, nuget.exe is targeting build machines, so it would be reasonable to expect it to have relative sophisticated retry logic, perhaps even exposing command line options that allowed the user to specify some of the parameters of the retry. However, the PowerShell and the UI generally have the interactive user right there. So for those its less of a priority because the average persistent developer will just have another go if it fails. Having said that its pretty cool that the search automatically falls back to the secondary.

@emgarten
Contributor
emgarten commented Aug 8, 2015

@johnataylor that is a good write up. I'll take a look at supporting Retry-After. I agree with just retrying on 5xx errors here. A 404 should be a clear indication that the package does not exist, retrying on that would decrease perf dramatically.

An environment variable or nuget.config setting might be the easiest way to make this consistent across all of the clients for users who need to go beyond the default number of retries. I would also expect retries to wait longer and longer between checks as it goes to avoid hammering the server.

@yishaigalatzer

We should be careful with 404 at least on our servers we have cases that because of caching a second 404 wins

-----Original Message-----
From: "Justin Emgarten" notifications@github.com
Sent: ‎8/‎7/‎2015 5:15 PM
To: "NuGet/Home" Home@noreply.github.com
Cc: "Yishai Galatzer" yishai_galatzer@yahoo.com
Subject: Re: [Home] NuGet Install or NuGet restore should support retry(#1120)

@johnataylor that is a good write up. I'll take a look at supporting Retry-After. I agree with just retrying on 5xx errors here. A 404 should be a clear indication that the package does not exist, retrying on that would decrease perf dramatically.
An environment variable or nuget.config setting might be the easiest way to make this consistent across all of the clients for users who need to go beyond the default number of retries. I would also expect retries to wait longer and longer between checks as it goes to avoid hammering the server.

Reply to this email directly or view it on GitHub.

@emgarten
Contributor
emgarten commented Aug 8, 2015

@yishaigalatzer are you saying we should retry on 404? There are a lot of scenarios where the package does not exist and a 404 is the correct response.

@yishaigalatzer

I'm saying it is worth a discussion. I understand the scenarios. I'm just not 100% convinced either way

-----Original Message-----
From: "Justin Emgarten" notifications@github.com
Sent: ‎8/‎7/‎2015 7:07 PM
To: "NuGet/Home" Home@noreply.github.com
Cc: "Yishai Galatzer" yishai_galatzer@yahoo.com
Subject: Re: [Home] NuGet Install or NuGet restore should support retry(#1120)

@yishaigalatzer are you saying we should retry on 404? There are a lot of scenarios where the package does not exist and a 404 is the correct response.

Reply to this email directly or view it on GitHub.

@emgarten
Contributor
@emgarten emgarten closed this Aug 18, 2015
@RanjiniM RanjiniM added the 2 - Working label Sep 1, 2015
@RanjiniM RanjiniM assigned bhuvak and emgarten and unassigned emgarten and bhuvak Sep 1, 2015
@emgarten emgarten added 3 - Done and removed 2 - Working labels Sep 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment