dht fixes #192

jbenet · 2014-10-21T10:03:40Z

Fixing dht issues.

jbenet · 2014-10-21T10:05:20Z

routing/dht/dht.go

@@ -272,7 +271,7 @@ func (dht *IpfsDHT) getValueOrPeers(ctx context.Context, p peer.Peer,
 	// Perhaps we were given closer peers
 	var peers []peer.Peer
 	for _, pb := range pmes.GetCloserPeers() {
-		pr, err := dht.addPeer(pb)
+		pr, err := dht.peerFromInfo(pb)


@whyrusleeping turns out that we don't want ensureConnectedToPeer here, because we won't connect to all of these peers. That's why there was addPeer. But addPeer is now equivalent to peerFromInfo, so i got rid of it as suggested :) 👍

(You were right though, that we do need to connect to these peers. see note below in query)

jbenet · 2014-10-21T10:24:34Z

@whyrusleeping i found the addresses issue. Then Listening, we don't learn the dialer's address. Because this address we're getting is the one they dialed from, not their proper one.

Options:

peers send their listen addrs in the version handshake (pre secure channel 👎 )
peers send their listen addrs after the secure channel is established. a sort of post handshake. this is probably where all the service negotiation belongs as well.
peers run a "node / peer info" service that just returns basic public information about themselves:

peer info:

peer.ID
peer.PubKey (already should be retrieved in secure handshake, but might be good to have too)
peer Listen Address
peer API Address (if exposed)

Oh yeah, that reminds me. The peer.Addresses really should be qualified. Something like:

// in peer/

type AddrType string

const (
  APIAddr = "api"
  ListenAddr = "listen" // rename config.Addresses.Swarm -> config.Addresses.Listen ?
  DialAddr = "dial" // may not even need to track these.
)

type Addrs map[string]ma.Multiaddr

func (* peer) Addresses() *Addrs { ... }

Thoughts on both (a) peer info service, and (b) qualified addrs?

whyrusleeping · 2014-10-21T16:40:39Z

Huh, so this is one of the same problems weve bee fighting for a while now... Im a fan of the option of just sending the addresses over during the handshake, its one less step and reduces complexity IMO. If you want to do the 'peer info' service though, i feel we should mold that into diagnostics, send a diagnostics message to the peer and ensure its not recursive, although, some people might not be okay with serving diagnostics information.. so that becomes weird too...

As for qualified addresses, im not so sure we need them... i dont beleive we should care about other peers API ports, that just seems weird to me, and the address they dialed from shouldnt really matter because it means so many different things depending on the context (udp vs tcp vs anything else)

The query-- once it's actually attempting to connect to a peer-- should be the one connecting.

dht doesn't need the whole network interface, only needs a Dialer. (much reduced surface of possible errors)

jbenet · 2014-10-22T12:38:42Z

@whyrusleeping i think i fixed it. i no longer get "no addresses on peer being sent!".

Though this dhtHell test takes forever and lags my computer, invariably making me exit it.

100
[4-99]->[0-3]
--
17 store key value
17 provide key
80 store key2 value2
80 provide key
[11-99] get key
[11-79] get key2
==

I do see some:

2014-10-22 05:37:39.541895 ERROR service service.go:236:    no request key [18 32 230 111 131 198 39 197 4 114 214 189 191 65 198 187 120 133 43 68 22 76 110 13 25 250 59 47 148 200 93 68 107 252 94 109 136 164] (timeout?)

jbenet · 2014-10-22T13:12:54Z

@whyrusleeping quick CR + test it out pls

whyrusleeping · 2014-10-22T17:25:02Z

The no request key happens when a node replies to a request that we dont care about anymore, seems pretty normal to me, especially if we ask two people for a value, and one returns first. We might want to handle that a little better, just to avoid logging confusion, but its not harming us any.

I ran a bunch of large concurrent "get" tests in dhthell with expect, and they all passed. Gonna try for ~500 nodes later when i get back to my desktop.

What specs does the machine youre running this on have? (the 100 node test you said was slow)

whyrusleeping · 2014-10-22T17:51:25Z

(the reason i ask about specs, is because youre probably running out of RAM and swapping hard, thus causing the slowness)

whyrusleeping · 2014-10-22T17:55:13Z

Creating node with id:  'QmUJwAW9DxxfSq9Bj1yJV29LCmiGLoJBzbqiwx9j8sepCQ'
Creating node with id: 'QmWzWuqmja92BXRHsUk2WDiQSzGcQqD3Z3HFTYgpNsQgh5'
Creating node with id: 'QmRhifxubZsjoxmfAqR4wDKQYdkYgqAWgfaBG5tNeFbER8'
runtime: memory allocated by OS (0xa8fde000) not in usable range [0x10800000,0x90800000)
runtime: out of memory: cannot allocate 1048576-byte block (1879048192 in use)
runtime: memory allocated by OS (0xa8fde000) not in usable range [0x10800000,0x90800000)
runtime: out of memory: cannot allocate 1048576-byte block (1879048192 in use)
runtime: memory allocated by OS (0xa8fde000) not in usable range [0x10800000,0x90800000)
runtime: out of memory: cannot allocate 1048576-byte block (1879048192 in use)
fatal error: out of memory

goroutine 16114 [running]:
runtime.throw(0x5d3bed)
    /home/whyrusleeping/go/src/pkg/runtime/panic.c:520 +0x5c fp=0xa92e9e64 sp=0xa92e9e58
largealloc(0x1, 0xa92e9ec4)
    /home/whyrusleeping/go/src/pkg/runtime/malloc.goc:226 +0xb0 fp=0xa92e9e88 sp=0xa92e9e64
runtime.mallocgc(0x100000, 0x3300f1, 0x1)
    /home/whyrusleeping/go/src/pkg/runtime/malloc.goc:169 +0x94 fp=0xa92e9ec0 sp=0xa92e9e88
cnew(0x3300f0, 0x100000, 0x1)
    /home/whyrusleeping/go/src/pkg/runtime/malloc.goc:836 +0xc8 fp=0xa92e9ed0 sp=0xa92e9ec0
runtime.cnewarray(0x3300f0, 0x100000)
    /home/whyrusleeping/go/src/pkg/runtime/malloc.goc:849 +0x3c fp=0xa92e9ee0 sp=0xa92e9ed0
makeslice1(0x327230, 0x100000, 0x100000, 0xa92e9f24)
    /home/whyrusleeping/go/src/pkg/runtime/slice.goc:55 +0x50 fp=0xa92e9eec sp=0xa92e9ee0
runtime.makeslice(0x327230, 0x100000, 0x0, 0x100000, 0x0, 0x0, 0x100000, 0x100000)
    /home/whyrusleeping/go/src/pkg/runtime/slice.goc:36 +0x1c8 fp=0xa92e9f0c sp=0xa92e9eec
github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-msgio.(*Chan).ReadFrom(0x181c0e60, 0xb6d4b460, 0x181c0e20, 0x100000)
    /home/whyrusleeping/gopkg/src/github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-msgio/chan.go:28 +0x154 fp=0xa92e9f9c sp=0xa92e9f0c
github.com/jbenet/go-ipfs/net/conn.func·002()
    /home/whyrusleeping/gopkg/src/github.com/jbenet/go-ipfs/net/conn/conn.go:76 +0xd8 fp=0xa92e9fcc sp=0xa92e9f9c
runtime.goexit()
    /home/whyrusleeping/go/src/pkg/runtime/proc.c:1445 fp=0xa92e9fcc sp=0xa92e9fcc
created by github.com/jbenet/go-ipfs/net/conn.newSingleConn
        /home/whyrusleeping/gopkg/src/github.com/jbenet/go-ipfs/net/conn/conn.go:78 +0x2d0`

FWIW, my chromebook cant run 100 nodes either.

whyrusleeping · 2014-10-22T18:05:07Z

net/handshake/handshake1.go

 }

 // ErrVersionMismatch is returned when two clients don't share a protocol version
 var ErrVersionMismatch = errors.New("protocol missmatch")

-// Compatible checks wether two versions are compatible
+// Handshake1Compatible checks wether two versions are compatible


nit: whether

addressed in 88cc1e2

whyrusleeping · 2014-10-22T18:10:06Z

A couple comments, and i think we should have a README.md in the handshake package to clearly document the protocol. Just to make things even clearer and easier for anyone wanting to audit it,

jbenet · 2014-10-22T19:48:30Z

Yeah probably memory. what's the issue though? are we leaking all the things? we should be able to run 100 nodes no problem just sending messages. msgs should be garbage collected, etc.

jbenet · 2014-10-22T19:53:14Z

A couple comments, and i think we should have a README.md in the handshake package to clearly document the protocol. Just to make things even clearer and easier for anyone wanting to audit it,

+:100:

jbenet · 2014-10-22T19:58:04Z

@whyrusleeping wat: https://travis-ci.org/jbenet/go-ipfs/jobs/38748767 never seen that before

whyrusleeping · 2014-10-22T20:47:56Z

yeah, thats a bug i need to fix, see #141
I left it out because it was complicated code, and was (supposed to be) very rare. but now that ive seen it two times in the wild, ill have to go fix it.

whyrusleeping · 2014-10-22T20:54:32Z

essentially, whats happenening is this:
We fill up one bucket of the routing table, so we go to split it, when the split happens, all the peers in that bucket move to the new bucket, thus putting us in a situation where we need to split again, this can happen up to (KEYLEN - 1) times in a row... So i need a loop, or recursion...

dht fixes

jbenet added the status/in-progress In progress label Oct 21, 2014

jbenet reviewed Oct 21, 2014
View reviewed changes

jbenet force-pushed the dhtfixes branch from 7cdaa3e to 7f22750 Compare October 21, 2014 22:20

whyrusleeping and others added 6 commits October 22, 2014 03:24

working on debugging dht issues

c070c2f

nicer, greppable logging

b66b260

this shouldn't connect quite yet.

6f8d23c

query wasnt ensuring conn

c3df397

The query-- once it's actually attempting to connect to a peer-- should be the one connecting.

notes

cc9f276

Dialer for dht

cc5c181

dht doesn't need the whole network interface, only needs a Dialer. (much reduced surface of possible errors)

jbenet force-pushed the dhtfixes branch from 7f22750 to cc5c181 Compare October 22, 2014 10:24

jbenet added 4 commits October 22, 2014 04:56

handshake1 renaming

97c66dd

handshake3 (addrs)

701035d

updated vendoring

54032ce

dht test fix (net)

a8adf2f

whyrusleeping reviewed Oct 22, 2014
View reviewed changes

spell nit

88cc1e2

minimal doc on handshake

6d8a1d1

fix for #141, routing table segmentation

b7744b2

jbenet added a commit that referenced this pull request Oct 23, 2014

Merge pull request #192 from jbenet/dhtfixes

bd5a1c0

dht fixes

jbenet merged commit bd5a1c0 into master Oct 23, 2014

jbenet removed the status/in-progress In progress label Oct 23, 2014

jbenet deleted the dhtfixes branch October 23, 2014 07:15

aschmahmann mentioned this pull request Aug 23, 2021

Release v0.10 #8176

Closed

62 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dht fixes #192

dht fixes #192

jbenet commented Oct 21, 2014

jbenet Oct 21, 2014

jbenet Oct 21, 2014

jbenet commented Oct 21, 2014

whyrusleeping commented Oct 21, 2014

jbenet commented Oct 22, 2014

jbenet commented Oct 22, 2014

whyrusleeping commented Oct 22, 2014

whyrusleeping commented Oct 22, 2014

whyrusleeping commented Oct 22, 2014

whyrusleeping Oct 22, 2014

jbenet Oct 22, 2014

whyrusleeping commented Oct 22, 2014

jbenet commented Oct 22, 2014

jbenet commented Oct 22, 2014

jbenet commented Oct 22, 2014

whyrusleeping commented Oct 22, 2014

whyrusleeping commented Oct 22, 2014

dht fixes #192

dht fixes #192

Conversation

jbenet commented Oct 21, 2014

jbenet Oct 21, 2014

Choose a reason for hiding this comment

jbenet Oct 21, 2014

Choose a reason for hiding this comment

jbenet commented Oct 21, 2014

whyrusleeping commented Oct 21, 2014

jbenet commented Oct 22, 2014

jbenet commented Oct 22, 2014

whyrusleeping commented Oct 22, 2014

whyrusleeping commented Oct 22, 2014

whyrusleeping commented Oct 22, 2014

whyrusleeping Oct 22, 2014

Choose a reason for hiding this comment

jbenet Oct 22, 2014

Choose a reason for hiding this comment

whyrusleeping commented Oct 22, 2014

jbenet commented Oct 22, 2014

jbenet commented Oct 22, 2014

jbenet commented Oct 22, 2014

whyrusleeping commented Oct 22, 2014

whyrusleeping commented Oct 22, 2014