Use multiple registry fallbacks instead of one#1246
Conversation
|
Thanks for your pull request, @wilzbach! |
|
Btw we didn't we simply add a list of RegistryPackageSuppliers as default? foreach(ps; m_packageSuppliers){
try {
auto versions = ps.getVersions(p);Doing the fallbacking as "Meta" supplier as done now and improved by this PR and within the class itself seems redundant. |
MartinNowak
left a comment
MartinNowak
left a comment
There was a problem hiding this comment.
As we already see sometimes errors on Travis while downloading the installer script due to dlang.org and nightlies.dlang.org being down, it's imho a good idea to have multiple fallbacks by default for DUB.
Any concrete example?
You can't fix all network issues, in particular when they are on bigger links.
The nightly nginx runs uninterrupted since 8 months, the nightly build server has an uptime of 1.5 years. It runs on OVH's networks which has nothing in common with dlang.org.
code.dlang.org is located in Germany (seems to peer via Level3 in Düsseldorf)
code-mirror.dlang.io runs on OVH/France
code-mirror2.dlang.io seems to run in some DC in texas (Incero), but uses the same DNS as code-mirror.dlang.io (another failure point, e.g. misconfiguration)
dub-registry.herokuapp.com runs on Heroku (AWS US-East)
So indeed adding 2 US based fallbacks seems sensible.
This was discussed in the original PR, #1190 (comment). |
|
Is it possible to set up a CNAME record for codemirror3.dlang.io that points to the Heroku app, or do they do any internal dispatching based on the original name? It would be good to keep a central place where the mirrors can be replaced/removed, so that we don't have to rely on application updates until such modifications take effect. |
Custom CNames are free at Heroku, but SSL for them isn't. We could, however, setup a wildcard redirect. dlang.io can be managed via Namecheap - I will invite you later today. |
|
A wildcard redirect would work. Regarding switching to HTTPS for code.dlang.org. As a simple first step we could have |
@s-ludwig I added the Heroku App as code-mirror3.dlang.io, but I have to agree with @MartinNowak: by using the Heroku DNS, we have another fallback in case NameCheap has some issues (also wildcard redirect don't seem to be possible directly with NameCheap) But I did add "http://code.dlang.org/" as a fallback if SSL isn't supported and fixed the Travis errors for 2.066 (as GDC is at 2.074.0 in master and 2.068.0 with their last release, we could drop the support for 2.066) |
wilzbach
force-pushed
the
more-fallbacks
branch
from
57c0e08 to
27c5f5e
Compare
|
Alternatively, we could add some other mechanism for maintaining the mirror list in the future, such as requesting an up-to-date one from code.dlang.org, which would ideally be signed to avoid a worthwhile security target. BTW, package signing is also something we should also put on the (virtual) higher priority list as the eco system keeps growing. |
|
Let's not sweat this too much, adding a few mirrors is nice as it might help with certain issues. |
MartinNowak
left a comment
MartinNowak
left a comment
There was a problem hiding this comment.
Just nits, lets move on with this.
| { | ||
| return format("%s (fallback %s)", m_default.description, m_fallback.description); | ||
| import std.algorithm : map; | ||
| return format("%s (fallback %s)", m_default.description, m_fallbacks.map!(x => x.description)); |
There was a problem hiding this comment.
Not a blocker, but this is fairly long and detailed.
==== registry at https://code.dlang.org/ (fallback ["registry at http://code.dlang.org/", "registry at https://code-mirror.dlang.io/", "registry at https://code-mirror2.dlang.io/", "registry at https://dub-registry.herokuapp.com/"]) ====
There was a problem hiding this comment.
I know, but the PackageSupplier interface currently only supports a description attribute. We could add sth. like location in a follow-up?
source/dub/packagesupplier.d
Outdated
| try | ||
| return m_fallback.%1$s(args); | ||
| catch(Throwable) | ||
| assert(1); |
There was a problem hiding this comment.
Isn't {} what you should use for an empty statement?
There was a problem hiding this comment.
More importantly, Throwable is a bad idea, must be Exception - logging the error with logDiagnostic or maybe just logDebug could also be important for remote debugging .
| catch(Throwable) | ||
| assert(1); | ||
| } | ||
| return m_fallbacks.back.%1$s(args); |
There was a problem hiding this comment.
The recursive structure was setup to chain exceptions, so that users get a list of unreachable sites. Not too important though.
| new FallbackPackageSupplier( | ||
| new RegistryPackageSupplier(URL(defaultRegistryURL)), | ||
| new RegistryPackageSupplier(URL(fallbackRegistryURL)) | ||
| fallbackRegistryURLs.map!(x => cast(PackageSupplier) new RegistryPackageSupplier(URL(x))).array |
There was a problem hiding this comment.
Could have stayed with the recursive structure.
fallbackRegistryURLs.reduce!((agg, url) => new FallbackPackageSupplier(agg, new RegistryPackageSupplier(URL(x))))There was a problem hiding this comment.
Hmm, I thought the "linear" way is more intuitive, but I don't really mind. If you feel strongly the recursive structure, feel free to change it back.
wilzbach
force-pushed
the
more-fallbacks
branch
from
8713786 to
caf429d
Compare
Addressed the nits.
Don't they get this with |
4 participants
As we already see sometimes errors on Travis while downloading the installer script due to dlang.org and nightlies.dlang.org being down, it's imho a good idea to have multiple fallbacks by default for DUB.
https://code-mirror2.dlang.io is maintained by myself and the Heroku-App is based on dlang/dub-registry#231