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

Improve or remove Blacklisting logic #2640

Closed
0x53A opened this issue Aug 20, 2017 · 14 comments
Closed

Improve or remove Blacklisting logic #2640

0x53A opened this issue Aug 20, 2017 · 14 comments
Labels

Comments

@0x53A
Copy link
Contributor

0x53A commented Aug 20, 2017

image

https://travis-ci.org/fsprojects/Paket/jobs/266553668#L7258

@matthid
Copy link
Member

matthid commented Aug 20, 2017

yeah potentially.
maybe we need to remember the errors and print them here..

@0x53A
Copy link
Contributor Author

0x53A commented Aug 21, 2017

Aha, I think this is related to that boring-ssl crap. If the first request hits the mono bug, then the source gets blacklisted.

#2637 may lessen the impact of this, but ultimately the paket behavior is probably ok.

@0x53A 0x53A closed this as completed Aug 21, 2017
@matthid
Copy link
Member

matthid commented Aug 21, 2017

Yes that's what I thought, but the message is still not optimal...

@matthid matthid reopened this Aug 21, 2017
@Desarc
Copy link

Desarc commented Aug 22, 2017

We are also having sporadic issues with all sources being blacklisted at the start of the resolution, which causes Paket to seemingly get stuck in an infinite loop. This is not Mono-related however, since we build on .NET, but seems to be performance-related:

log.txt (short version of a much longer log)

We use an on-premise Proget server that sometimes struggles with response times. With older versions (x < 5.86.0) we have not recorded this error, but we see it sometimes with the newer versions. It usually works on the second try though, and we often have trouble reproducing these errors ourselves.

Is my assumption that this is performance-related correct? If so, how can we fix this, other than improving the performance of our Proget server (we are working on that...)?

@0x53A
Copy link
Contributor Author

0x53A commented Aug 25, 2017

There are also issues with feeds getting blacklisted, if a package was not yet indexed:

I uploaded https://www.nuget.org/packages/0x53A.ReferenceAssemblies.net45/0.1.0 a few minutes ago:

 - 0x53A.ReferenceAssemblies.net45 0.1.0
Possible Performance degration, blacklist '1_https://www.nuget.org/api/v2/Packages(Id='0x53a.referenceassemblies.net45',Version='0.1.0')'
Possible Performance degration, blacklist '1_https://www.nuget.org/api/v2/Packages(Id='0x53a.referenceassemblies.net45',Version='0.1.0')'
Possible Performance degration, blacklist '1_https://www.nuget.org/api/v2/Packages(Id='0x53A.ReferenceAssemblies.net45',Version='0.1.0')'
Possible Performance degration, blacklist '1_https://www.nuget.org/api/v2/Packages(Id='0x53A.ReferenceAssemblies.net45',Version='0.1.0')'
Possible Performance degration, blacklist '2_https://www.nuget.org/api/v2/Packages?$filter=(Id eq '0x53A.ReferenceAssemblies.net45') and (NormalizedVersion eq '0.1.0')'
 - FSharp.Formatting 3.0.0-beta07
 - RoslynTools.MSBuild 0.4.0-alpha
Possible Performance degration, V3 was not working: Value cannot be null.
Parameter name: uriString
 - RoslynTools.ReferenceAssemblies 0.1.1
Possible Performance degration, V3 was not working: Value cannot be null.
Parameter name: uriString
 - FAKE 4.63

image

@matthid
Copy link
Member

matthid commented Sep 3, 2017

Yes this is real - especially when a package is not existing on a source.

The huge problem is that we cannot trust a source that a 404 is correct (as some sources always do that for some V2 urls).

To be honest I have no idea anymore what to do. We need to somehow intelligently detect that a source is trustworthy and "trust" 404 responses as well.

@matthid
Copy link
Member

matthid commented Sep 3, 2017

I'm slowly starting to think that the only sane way forward is:

  • Implement what nuget.exe is doing exactly (figure that out by using fiddler on various sources)
  • Use this logic to validate before blacklisting a source or just remove the current logic

@matthid matthid changed the title [Travis] Blacklisting is potentially too eager Improve or remove Blacklisting logic Sep 3, 2017
@matthid matthid added the bug label Sep 3, 2017
@matthid
Copy link
Member

matthid commented Sep 3, 2017

Yes this is real - especially when a package is not existing on a source.

Thinking about this again, maybe the problem is something different.
We should not try to call GetDetails on package versions only returned by another source. I always thought this is already implemented but maybe this is broken in the special case when the other source is a local directory...

@forki
Copy link
Member

forki commented Sep 4, 2017 via email

@matthid
Copy link
Member

matthid commented Sep 4, 2017

@forki look at my last comment, maybe there is another bug here. Because when I implemented it the assumption was: when we make the request we already know the package exists on this source (because we got the version from it). Therefore we can follow: 404 -> broken Uri.

@forki
Copy link
Member

forki commented Sep 4, 2017

yeah that assumption is sound.

@matthid
Copy link
Member

matthid commented Sep 4, 2017

I'm totally ignoring "This package has not been indexed yet", because IMHO that is clearly a server bug (and is only a transient error)

@matthid
Copy link
Member

matthid commented Sep 4, 2017

@Desarc Can you post your paket.dependencies or tell us if you had any "pinned" packages? Just want to ensure we actually solved the issue you are seeing.

@Desarc
Copy link

Desarc commented Sep 5, 2017

We get this in many different projects, with many different paket.dependencies configurations. It's generally a mix of pinning some packages to specific versions, while specifying version ranges for other packages (with ~> or @~>, typically internal packages). We also mostly use strategy: max.

I don't know if it's relevant, but we usually don't use groups, and list multiple sources. The number of packages listed in one paket.dependencies can be anything from a few to over 100 (not counting unspecified transitive dependencies).

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

4 participants