options to configure static relays for autorelay #705

vyzo · 2019-08-16T10:01:46Z

So that we can avoid hitting the DHT; also changes the desired relays parameter to 1 (from 3).
See #694

albrow · 2019-08-19T18:53:08Z

Does the StaticRelays option mean that those are the only relays that can be used? Ideally, for our network we would like to use static relays to reduce the time to first connection but we still want to discover other relays over time. Otherwise our dedicated relay hosts would be overworked.

vyzo · 2019-08-19T19:11:09Z

With this patch yes, when specified only those will be used.
Perhaps we should have a fallback to the DHT, but the issue is that we are having excessive DHT traffic because of autorelay.

vyzo · 2019-08-19T19:11:19Z

ping @Stebalien

vyzo · 2019-08-19T19:12:11Z

Moving forward we also want to use relays discovered by passive discovery (when the option circuit.OptDiscovery is specified).

raulk · 2019-10-06T14:20:41Z

options.go

+			"/ip4/147.75.80.110/tcp/4001/p2p/QmbFgm5zan8P6eWWmeyfncR5feYEMPbht5b1FW1C37aQ7y",
+			"/ip4/147.75.195.153/tcp/4001/p2p/QmW9m57aiBDHAkKj9nmFSEn7ZqrcF1fZS4bipsTCHburei",
+			"/ip4/147.75.70.221/tcp/4001/p2p/Qme8g49gm3q4Acp7xWBKg3nAa9fxZ1YmyDJdyGgoG6LsXh",


Can we have these multiaddrs in a global var? You can initialize them with an IIFE, or in an init function.

p2p/host/relay/autorelay.go

raulk · 2019-10-06T14:24:18Z

p2p/host/relay/autorelay.go

@@ -245,6 +248,10 @@ func (ar *AutoRelay) connect(ctx context.Context, pi peer.AddrInfo) bool {
 }

 func (ar *AutoRelay) discoverRelays(ctx context.Context) ([]peer.AddrInfo, error) {
+	if len(ar.static) > 0 {


I don't think setting static relays should be exclusive from dynamic relay discovery. You probably want static relays for a speedier bootup, but as you dynamically discover relays, you should preferentially switch to those.

I see that @albrow expected the same thing: #705 (comment)

This defeats the purpose; the main impetus for the static relay configuration is to avoid hitting the DHT altogether.
We might want to leverage passive discovery with CanHop in the relay module, but I don't think it's a good idea to have both static relays and hit the dht.

At the moment, auto relay discovery is pretty much broken. We have no way to distinguish between good relays and bad relays.

The current "automatically pick a relay" feature is broken. Unfortunately, you're likely to pick a random configured relay with a low connection limit. When that happens, you'll have to keep churning through relays till one sticks.

Really, this is a feature where we either need "blessed" relays or some kind of reputation system.

vyzo · 2019-10-08T10:00:54Z

Updated to use a global variable in the autorelay module for the default relays; also renamed the DefaultRelays to DefaultStaticRelays and made StaticRelays append instead of overwriting.

and rename DefaultRelays option to DefaultStaticRelays.

Stebalien

I'd like to include this in a go-ipfs patch release to reduce the stress on the network. @raulk, can we merge this with no changes for now?

Adding support for both static and dynamic relays at the same time is non-trivial.

Stebalien · 2020-01-17T01:58:27Z

p2p/host/relay/autorelay.go

@@ -245,6 +248,10 @@ func (ar *AutoRelay) connect(ctx context.Context, pi peer.AddrInfo) bool {
 }

 func (ar *AutoRelay) discoverRelays(ctx context.Context) ([]peer.AddrInfo, error) {
+	if len(ar.static) > 0 {


The current "automatically pick a relay" feature is broken. Unfortunately, you're likely to pick a random configured relay with a low connection limit. When that happens, you'll have to keep churning through relays till one sticks.

Really, this is a feature where we either need "blessed" relays or some kind of reputation system.

raulk

Armed with a much better understanding now, I agree that automatic discovery works less ideally than we’d want, anyway.

If somebody is using autorelay, they’re probably better off using static relays for predictability.

I don’t think we need to go as far as reputational systems to make this functional at first: a BIND/ALLOCATE operation could go a long way.

I’m ok merging this as-is, but I’d really like static relays used while we’re discovering dynamic relays (assuming the discovery works well), and dropping them as soon as we’ve bound to dynamic ones. If those fail, we can fall back to the static ones — making these relays useful for fallback and “fast start”.

Stebalien · 2020-01-17T13:22:12Z

Agreed. Static relays are really just an emergency measure until we can implement a better system.

RubenKelevra · 2020-01-31T16:52:11Z

Agreed. Static relays are really just an emergency measure until we can implement a better system.

How about flipping the process, having a node subscribe to pubsub when they need a relay.

All nodes which offer the relay service will send a message on this pubsub on how many connections they can accept.

The number is multiplied with a number x (which is something like 0.01 in the beginning) and is the probably that the node will try to connect to this relay.

On connection to the relay we send a 'request' for relay services. When the relay is already full, it will send a deny message and close the connection.

On each failure to connect to a relay, the number x is cut down in half, to simply estimate the 'load' on all the relays.

After a random period between 30 and 600 minutes the number x is multiplied by 2 again.

Stebalien · 2020-01-31T17:11:39Z

We should advertise relays through pubsub (and maybe even just have relays advertise other relays). However, there's still the issue of reputation.

RubenKelevra · 2020-01-31T17:44:59Z

@Stebalien I think that's an issue for another time. I think it's best to just replace the DHT method with something that's responding dynamically to the network size, by lowering the connection attempts.

The optimal solution for fixing this would be to add peers in a Web Of Trust fashion and allow 'friends' to use the relay function.

We could also hold those connections all the time and bootstrap with the friend connections first, instead of the static IPs and domains we currently use.

Stebalien · 2020-01-31T17:55:12Z

I'm not sure if that's an issue we can punt on. It takes a bit of effort to attack the DHT. It would take very little effort to add enough bad relays to affect the network.

We don't really need a full WoT system, even just something that verifies that a relay is doing the right thing would be enough. For example, some way to ask other peers to dial back through the relay and verify that it's working.

RubenKelevra · 2020-01-31T18:43:18Z

@Stebalien if we change the amount of desired relays back to 3, we could check each of those relays by requesting some data:

One relay is asked to provide some random stuff, and we request it thru both other relays.

We cycle this thru all 3 relays to confirm all relays are working fine.

Another possibility is to request everything round robin thru the 3 relays, and if we identify a relay which is sending data which doesn't match the checksums for example, we drop that connwc and create a new one.

aschmahmann · 2020-02-28T06:03:22Z

p2p/host/relay/autorelay.go

 	ar := &AutoRelay{
 		host:       bhost,
 		discover:   discover,
 		router:     router,
 		addrsF:     bhost.AddrsFactory,
+		static:     static,


@vyzo I know I'm late to the party here, but this felt a bit small for an issue.

Why did we bother changing the constructor and using a []Addr.Info here instead of just creating a Discoverer implementation that returns a the static list?

Sure it can be more efficient to use the slice, but now we have this thing we're going to be stuck removing.

vyzo requested review from Stebalien and raulk August 16, 2019 10:01

options to configure known relays for autorelay

2358488

raulk requested changes Oct 6, 2019

View reviewed changes

use a global variable for default relays

9cd56c0

and rename DefaultRelays option to DefaultStaticRelays.

vyzo mentioned this pull request Nov 3, 2019

AutoNAT only adds AutoNAT service peers on *new* connection libp2p/go-libp2p-autonat#28

Closed

Stebalien approved these changes Jan 17, 2020

View reviewed changes

raulk approved these changes Jan 17, 2020

View reviewed changes

Stebalien merged commit 76944c4 into master Jan 17, 2020

This was referenced Jan 17, 2020

Release v0.4.23 ipfs/kubo#6836

Closed

Release v0.4.23 ipfs/kubo#6837

Closed

aschmahmann reviewed Feb 28, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

options to configure static relays for autorelay #705

options to configure static relays for autorelay #705

vyzo commented Aug 16, 2019

albrow commented Aug 19, 2019

vyzo commented Aug 19, 2019

vyzo commented Aug 19, 2019

vyzo commented Aug 19, 2019

raulk Oct 6, 2019

vyzo Oct 6, 2019

raulk Oct 6, 2019

raulk Oct 6, 2019

vyzo Oct 6, 2019

Stebalien Jan 17, 2020

Stebalien Jan 17, 2020

vyzo commented Oct 8, 2019

Stebalien left a comment

Stebalien Jan 17, 2020

raulk left a comment

Stebalien commented Jan 17, 2020

RubenKelevra commented Jan 31, 2020 •

edited

Loading

Stebalien commented Jan 31, 2020

RubenKelevra commented Jan 31, 2020

Stebalien commented Jan 31, 2020

RubenKelevra commented Jan 31, 2020

aschmahmann Feb 28, 2020

options to configure static relays for autorelay #705

options to configure static relays for autorelay #705

Conversation

vyzo commented Aug 16, 2019

albrow commented Aug 19, 2019

vyzo commented Aug 19, 2019

vyzo commented Aug 19, 2019

vyzo commented Aug 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vyzo commented Oct 8, 2019

Stebalien left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raulk left a comment

Choose a reason for hiding this comment

Stebalien commented Jan 17, 2020

RubenKelevra commented Jan 31, 2020 • edited Loading

Stebalien commented Jan 31, 2020

RubenKelevra commented Jan 31, 2020

Stebalien commented Jan 31, 2020

RubenKelevra commented Jan 31, 2020

Choose a reason for hiding this comment

RubenKelevra commented Jan 31, 2020 •

edited

Loading