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

Experiment with websocket-star in js-ipfs fallback #63

Closed
lidel opened this issue Nov 29, 2018 · 11 comments
Closed

Experiment with websocket-star in js-ipfs fallback #63

lidel opened this issue Nov 29, 2018 · 11 comments

Comments

@lidel
Copy link
Member

lidel commented Nov 29, 2018

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 (ipfs/ipfs-companion#457).

Bonus: support multiple stars?

libp2p/js-libp2p-websocket-star#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 libp2p/js-libp2p-websocket-star-rendezvous#27 (comment):

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
Copy link
Contributor

fsdiogo commented Nov 30, 2018

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
Copy link
Member Author

lidel commented Nov 30, 2018

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
Copy link
Contributor

fsdiogo commented Nov 30, 2018

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
Copy link
Member Author

lidel commented Dec 4, 2018

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
Copy link
Member

olizilla commented Dec 5, 2018

The question seems to boil down to:

  1. 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.
  2. 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
Copy link
Member

alanshaw commented Dec 5, 2018

+1 for 2

@lidel
Copy link
Member Author

lidel commented Dec 5, 2018

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:

dns-fail-2018-12-05_18-04

It fails if port is closed/server is down:

fail-port-closed-2018-12-05_18-06

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
Copy link
Contributor

fsdiogo commented Dec 11, 2018

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
Copy link
Member Author

lidel commented Jan 6, 2019

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

@alanshaw
Copy link
Member

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 added a commit to ipfs/ipfs-companion that referenced this issue Jan 30, 2019
js-ipfs v0.32.2 solved issue mentioned in ipfs-shipyard/ipfs-share-files#63 (comment) by switching to `libp2p-websocket-star-multi` (details in ipfs/js-ipfs#1793).
@lidel
Copy link
Member Author

lidel commented Feb 4, 2021

Superseded by #58 (comment)

@lidel lidel closed this as completed Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants