Enable switching DHT between client and server modes #469

aschmahmann · 2020-02-28T22:07:02Z

This is needed to support libp2p peers that want to switch between client and server DHT modes while the peers are operational.

Exposes a SetMode(DHTMode) that allows manually setting the mode
Adds a utility function RoutabilityBasedModeSwitching(*dht) that will use the libp2p eventbus to automatically trigger mode switches (e.g. based on AutoNat emitting LocalRoutability events)

There are other ways we could expose this functionality such as:

Adding a function option to the DHT instead of requiring RoutabilityBasedModeSwitching to be called on a DHT instance
- Cannot do naively without refactoring since the DHT depends on the options package, so the options package cannot depend on the DHT
- Could move the DHTMode to be declared outside of the dht package (e.g. options or a new one) and then define an interface with enough of the DHT functionality supported to make this work
Add an option to the DHT for a function that returns a channel of DHTMode
- Requires moving DHTMode to another package
- Requires an extra goroutine to pull data off the eventbus and put it into a channel

All of these options could potentially run into issues if events get dropped off the eventbus, so following recommendations from the stabilize branch I've set the eventbus subscription buffer size to 256. Depending on the risk associated with dropping eventbus messages we may either want a way to define a stateful event channel (e.g. when a new event comes in throw away existing events and add the new one), or have some way of detecting dropped messages and then checking the relevant service.

dht.go

util.go

dht.go

dht_test.go

dht.go

dht_test.go

dht.go

util.go

dht.go

dht_net.go

dht.go

aschmahmann

Next draft is up, let me know what you think so we can get this 🚢

dht.go

dht_test.go

aschmahmann · 2020-03-05T05:56:39Z

ext_test.go

@@ -31,7 +31,7 @@ func TestGetFailures(t *testing.T) {
 	host1 := bhost.New(swarmt.GenSwarm(t, ctx, swarmt.OptDisableReuseport))
 	host2 := bhost.New(swarmt.GenSwarm(t, ctx, swarmt.OptDisableReuseport))

-	d, err := New(ctx, host1, opts.DisableAutoRefresh())
+	d, err := New(ctx, host1, opts.DisableAutoRefresh(), opts.Mode(opts.ModeServer))


Yep, so your tests break here too.

subscriber_notifee.go

aschmahmann · 2020-03-05T06:29:58Z

subscriber_notifee.go

+			&event.EvtLocalRoutabilityPublic{},
+			&event.EvtLocalRoutabilityPrivate{},
+			&event.EvtLocalRoutabilityUnknown{},


See #469 (comment)

subscriber_notifee.go

opts/options.go

aarshkshah1992

@aschmahmann Great stuff ! :)

subscriber_notifee.go

dht_test.go

subscriber_notifee.go

dht_test.go

subscriber_notifee.go

Stebalien · 2020-03-06T07:23:28Z

subscriber_notifee.go

+	defer dht.plk.Unlock()
+	if dht.host.Network().Connectedness(e.Peer) != network.Connected {
+		return
+	}


Note: I believe both this check and this lock can go away once we switch to the new routing table logic. At that point, it's actually perfectly fine to add disconnected peers to our routing table.

@aarshkshah1992 can you confirm, I haven't had time to look through the routing table PR yet.

I talked with him about this and I was wrong. We can make this possible with a small tweak to the routing table, but we'll handle that later.

(small tweak == get rid of live/dead states, just call a "checkPeer" function on all peers every tick)

subscriber_notifee.go

Stebalien · 2020-03-06T08:00:24Z

subscriber_notifee.go

+			case event.EvtPeerProtocolsUpdated:
+				handlePeerProtocolsUpdatedEvent(dht, evt)
+			case event.EvtLocalReachabilityChanged:
+				handleLocalReachabilityChangedEvent(dht, evt)


default: log error?

Stebalien · 2020-03-06T08:02:10Z

subscriber_notifee.go

+	defer dht.plk.Unlock()
+	if dht.host.Network().Connectedness(e.Peer) != network.Connected {
+		return
+	}


I talked with him about this and I was wrong. We can make this possible with a small tweak to the routing table, but we'll handle that later.

(small tweak == get rid of live/dead states, just call a "checkPeer" function on all peers every tick)

…EvtLocalRoutability event on the libp2p eventbus

…not simply rely on the eventbus, instead use the eventbus to trigger checking the peerstore for which protocols the peer currently supports

… modes

…nes into a single goroutine. made forced updates when we have small numbers of peers in the routing table more consistent

…ng through a subscription

* created Mode(ModeOpt) option for choosing between auto/client/server modes * Auto mode internally switches the DHT between client and server modes based on the EvtLocalReachabilityChanged event emitted on the event bus (e.g. by AutoNAT) * routing table management of peers that switch between client and server mode while we are connected to them (i.e. are in auto mode) * removed Client(bool) option, becoming a DHT client is specified using Mode(ModeClient) instead

willscott reviewed Mar 2, 2020

View reviewed changes

dht.go Outdated Show resolved Hide resolved

dht.go Outdated Show resolved Hide resolved

aschmahmann commented Mar 2, 2020

View reviewed changes

util.go Outdated Show resolved Hide resolved

aschmahmann commented Mar 2, 2020

View reviewed changes

util.go Outdated Show resolved Hide resolved

Stebalien reviewed Mar 3, 2020

View reviewed changes

dht.go Outdated Show resolved Hide resolved

dht.go Outdated Show resolved Hide resolved

dht.go Outdated Show resolved Hide resolved

dht.go Outdated Show resolved Hide resolved

dht.go Outdated Show resolved Hide resolved

aschmahmann mentioned this pull request Mar 3, 2020

Utilize identify events to add peers to the routing table #472

Merged

aschmahmann commented Mar 3, 2020

View reviewed changes

dht.go Outdated Show resolved Hide resolved

aschmahmann commented Mar 3, 2020

View reviewed changes

dht.go Outdated Show resolved Hide resolved

aschmahmann commented Mar 3, 2020

View reviewed changes

dht_test.go Show resolved Hide resolved

aschmahmann self-assigned this Mar 3, 2020

aschmahmann mentioned this pull request Mar 3, 2020

Proposed DHTv2 Changes #473

Merged

Stebalien requested changes Mar 4, 2020

View reviewed changes

Stebalien force-pushed the cypress branch from 3e405f7 to bf5f87a Compare March 4, 2020 03:50

Stebalien force-pushed the feat/mode-switching branch from 28fea97 to a80f6c1 Compare March 4, 2020 03:50

aarshkshah1992 suggested changes Mar 4, 2020

View reviewed changes

aarshkshah1992 reviewed Mar 4, 2020

View reviewed changes

dht.go Outdated Show resolved Hide resolved

aarshkshah1992 suggested changes Mar 4, 2020

View reviewed changes

dht.go Outdated Show resolved Hide resolved

dht.go Outdated Show resolved Hide resolved

aarshkshah1992 mentioned this pull request Mar 4, 2020

Stateful eventbus libp2p/go-libp2p-autonat#51

Closed

aschmahmann force-pushed the feat/mode-switching branch 2 times, most recently from 0930def to 0aa4675 Compare March 5, 2020 06:02

Stebalien force-pushed the cypress branch 2 times, most recently from 22b3963 to 5d313b1 Compare March 5, 2020 06:26

aschmahmann commented Mar 5, 2020

View reviewed changes

opts/options.go Show resolved Hide resolved

aschmahmann force-pushed the feat/mode-switching branch from ac9abda to c649402 Compare March 5, 2020 07:57

aschmahmann requested review from Stebalien and aarshkshah1992 March 5, 2020 07:58

aarshkshah1992 approved these changes Mar 5, 2020

View reviewed changes

subscriber_notifee.go Show resolved Hide resolved

dht_test.go Show resolved Hide resolved

aschmahmann force-pushed the feat/mode-switching branch from c649402 to 9138a92 Compare March 5, 2020 17:22

aschmahmann mentioned this pull request Mar 5, 2020

Upgrade DHT version #479

Merged

aschmahmann force-pushed the feat/mode-switching branch from 74ca902 to d7523e1 Compare March 6, 2020 05:05

Stebalien reviewed Mar 6, 2020

View reviewed changes

Stebalien approved these changes Mar 6, 2020

View reviewed changes

whyrusleeping and others added 12 commits March 6, 2020 14:41

dht mode toggling (modulo dynamic switching) (#350)

02f4a9b

added utility function to update the DHT mode based on firing of the …

da534c0

…EvtLocalRoutability event on the libp2p eventbus

finish initialization before starting internal goroutines

6bde6df

for handling other peers switching between client and server mode do …

2040889

…not simply rely on the eventbus, instead use the eventbus to trigger checking the peerstore for which protocols the peer currently supports

test: add test for when remote peers switch between client and server…

ea2a6ab

… modes

fmt: group dht proc creation with usage

e4f5460

created DHT Auto/Client/Server options. combined all eventbus gorouti…

ebecc9f

…nes into a single goroutine. made forced updates when we have small numbers of peers in the routing table more consistent

import reordering

dce9e6a

added a comment

5a72a0f

use updated autonat events

7b1a77f

switch to a single subscription for all relevant events

f3cb345

add more logging and be defensive of invalid or incorrect events comi…

a65ba4e

…ng through a subscription

aschmahmann force-pushed the feat/mode-switching branch from 5577ec7 to a65ba4e Compare March 6, 2020 19:45

aschmahmann merged commit c24a52f into cypress Mar 6, 2020

Stebalien deleted the feat/mode-switching branch March 10, 2020 00:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable switching DHT between client and server modes #469

Enable switching DHT between client and server modes #469

aschmahmann commented Feb 28, 2020

aschmahmann left a comment

aschmahmann Mar 5, 2020

aschmahmann Mar 5, 2020

aarshkshah1992 left a comment

Stebalien Mar 6, 2020

aschmahmann Mar 6, 2020

Stebalien Mar 6, 2020

Stebalien Mar 6, 2020

Stebalien Mar 6, 2020

Enable switching DHT between client and server modes #469

Enable switching DHT between client and server modes #469

Conversation

aschmahmann commented Feb 28, 2020

aschmahmann left a comment

Choose a reason for hiding this comment

aschmahmann Mar 5, 2020

Choose a reason for hiding this comment

aschmahmann Mar 5, 2020

Choose a reason for hiding this comment

aarshkshah1992 left a comment

Choose a reason for hiding this comment

Stebalien Mar 6, 2020

Choose a reason for hiding this comment

aschmahmann Mar 6, 2020

Choose a reason for hiding this comment

Stebalien Mar 6, 2020

Choose a reason for hiding this comment

Stebalien Mar 6, 2020

Choose a reason for hiding this comment

Stebalien Mar 6, 2020

Choose a reason for hiding this comment