Experiment with websocket-star in js-ipfs fallback

lidel commented 5 years ago

This may be a low hanging fruit and a safer way to improve performance than #58 until we get relay autodiscovery enabled in the future.

@ `ws-star` Client

README at js-libp2p-websocket-star states that:

We host a rendezvous server at /dns4/ws-star.discovery.libp2p.io that can be used for practical demos and experimentation, it should not be used for apps in production.

It is enabled by passing it in js-ipfs config like this:

{
  "config": {
    "Addresses": {
      "Swarm": ["/dns4/ws-star.discovery.libp2p.io/tcp/443/wss/p2p-websocket-star"]
    }
  }
}

AFAIK peerpad hosts uses own star server(s) (see below), we probably could do the same and use them in Companion as well (https://github.com/ipfs-shipyard/ipfs-companion/issues/457).

Bonus: support multiple stars?

https://github.com/libp2p/js-libp2p-websocket-star/issues/61:

mkg20001/js-libp2p-websocket-star-multi substantially improves rendezvous server handling by allowing multiple connections and not suiciding startup if one of them is down. This dramatically improves stability and usability and should be the default.

@ `ws-star` Server

IIUC this is something we get out of the box right now because @victorb mentioned that:

new version of ws-star has been deployed! Making discover of peers muuuuuch faster

Relevant change https://github.com/libp2p/js-libp2p-websocket-star-rendezvous/pull/27#issue-233670284:

For peer-star-app we would like to be informed of all other peers that are already connected to the rendezvous server when we connect to it. More details on this PR

fsdiogo commented 5 years ago

Thanks for opening this @lidel.

This could work, but only when using js-ipfs fallback, as ipfs-redux-bundle doesn't allow us to change the config of a local daemon.

lidel commented 5 years ago

Yes, this is only for js-ipfs fallback running in page context, however we need to make it work really, really well.

Fallback will used by "civilians" without local daemon however it creates the first impression of IPFS that could empower or hurt entire ecosystem.

We need to be extra careful and mindful about this. cc @ipfs-shipyard/gui

fsdiogo commented 5 years ago

There's a scenario about this that makes me a bit uncomfortable: if we do make it work well, it will be great for the entry user. Then if we're lucky, maybe he'll want to learn more about IPFS and even install it and run a local daemon. After that, the speed of the fetches will decrease significantly.

What I'm saying is that the user will be put off after trying the real IPFS experience OR feel a bit cheated after figuring out how the app is working.

We could make it work only the js-ipfs fallback, but I don't like that solution to be honest.

Does this makes sense?

lidel commented 5 years ago

I understand the concern, but I feel we don't really have much choice here: if initial experience is bad, people won't bother with looking at running daemon or the app for the second time.

Note that people who decide to run local daemon will have some sort of "IPFS enthusiasm" motivating them to do so, and will most likely be willing to forgive more that regular web users.

Also, if the discovery via local daemon is unacceptably slower then it needs to be addressed upstream (and there is an ongoing/planned work to address dialing/discovery performance in libp2p/go-ipfs) and it is actually better if we notice it during dogfooding.

olizilla commented 5 years ago

The question seems to boil down to:

Use a vanilla js-ipfs config which will often be slow. We then use it as a refernce point to drive improvements in libp2p and ipfs.
Use a custom config with a centralised discovery service, which should make the service faster.

I think we're haggling over where we put the potential dissapointment. Unacceptably slow is unacceptable. If we stick with option one, we will likely put people off at the first step.

If we go with option 2, it should leave us with a share service that is usable, and we have an app that is worth talking about. It would be on us to do a good job of explaining the trade-offs that are being made and why. If we frame it around "we plan to make this service even more decentralised with every new release of js-ipfs" kind of thing, then we can set ourselves up for an exciting announcement for the day when we can remove the ws* discovery services without harming the perfomance. And we can talk about what is the minium viable set up for an IPFS based service today and update that info as new features land.

Other things in favour of option 2.

js-ipfs currently uses preload nodes which are a centralized crutch to deal with the discovery between browser nodes problem, so we should not suggest that this is a fully distributed solution just yet.
The points of centralization are generic / fungible / anyone can run one. This is not locking you in to a closed system, this is providing a basic service to a support p2p discovery. We should investigate https://github.com/mkg20001/js-libp2p-websocket-star-multi and have a way to let multiple providers add additional nodes.

@fsdiogo makes a good point that it's going to be awkward if the app is slower when we use a local IPFS node. I'm starting to think that calling out to a local IPFS node for services like this might be more problematic than valuable. Particulaly while users have to add additional CORS config per site to do so, which is awkward, and arguably sets a bad precedent. Do we really want users to set CORS exeptions for every site they visit?

I think we need to be pushing people to IPFS Companion and stop using js-ipfs-http-client directly in web apps. @lidel It'd be rad if we had a way to signal to Companion that a site had ws-* discovery infra in place that it could take advantage of. Difficult right now perhaps, but rad.

alanshaw commented 5 years ago

+1 for 2

lidel commented 5 years ago

I've played with some failure scenarios and current error handling around ws-star in js-ipfs (0.33.1) is quite brittle:

js-ipfs fails to start if DNS lookup for ws-star fails:

It fails if port is closed/server is down:

So my position is that it is not safe to enable it as it is now. We need to make it more robust first, as suggested in libp2p/js-libp2p-websocket-star#61

[..] improve rendezvous server handling by allowing multiple connections and not suiciding startup if one of them is down

fsdiogo commented 5 years ago

The not suiciding startup if one of them is down has to happen ASAP, or as you said it's simply not safe to enable this.

About the allowing multiple connections that would be good, but upon talking with @victorb he brung up an important point: if some users connect to a server and others connect to a different one, they won't be able to find each other.

Maybe we can use just one (the most reliable?) because the next release of js-ipfs will probably help with peer discovery, is it possible it will be good to the point of not having to use a signalling server? 🤔

lidel commented 5 years ago

FYSA @mkg20001 works on js-libp2p-stardust which aims to replace js-libp2p-websocket-star (see discussion in https://github.com/libp2p/js-libp2p-websocket-star/issues/70) and fix most of the pain points before auto discovery via rendezvous lands.

alanshaw commented 5 years ago

PSA: JS IPFS 0.34.2 uses libp2p-websocket-star-multi which will temporarily help with suicide on startup until a solution lands in libp2p.

lidel commented 3 years ago

Superseded by https://github.com/ipfs-shipyard/ipfs-share-files/issues/58#issuecomment-773248433

ipfs-shipyard / ipfs-share-files