basic reprovider implementation #554

whyrusleeping · 2015-01-13T22:40:57Z

This PR currently has basic functionality, im not sure what else we would need, so im RFCR

btc · 2015-01-13T23:30:21Z

Godeps/Godeps.json

@@ -1,6 +1,6 @@
 {
 	"ImportPath": "github.com/jbenet/go-ipfs",
-	"GoVersion": "go1.3",
+	"GoVersion": "devel +9ef10fde754f Thu Dec 11 16:32:25 2014 +1100",


1.3 or (1.4)

btc · 2015-01-14T01:08:36Z

you'll want to edit the commit where context isn't vendored

https://build.protocol-dev.com/job/build-walk-branch/728/

jbenet · 2015-01-14T02:09:48Z

core/commands/mount_darwin.go

@@ -6,7 +6,7 @@ import (
 	"strings"
 	"syscall"

-	fuseversion "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-fuse-version"
+	fuseversion "github.com/jbenet/go-fuse-version"


you somehow unvendored go-fuse-version. are you running straight up make vendor?

Oh, you might not have the dep. it may be important to make it:

vendor: godep go get ./... godep save -r ./...

cc @briantigerchow

I'm all for anything that helps to reduce friction so long as it respects the developer's GOPATH.

I tend to use godep restore ./... to turn vendored dependencies into packages in my GOPATH, but I recognize that some may find this behavior inconvenient.

yeah, i just ran 'make vendor' not sure what happened there

sigh godep vendoring. i think that godep get ./... will:

only fetch things the user doesnt have (so wont break things for them)

will still require a godep update <path> to change to the version the user has.

why did this rewrite even happen? godep has no business turning that back into a non-vendored pkg...

@whyrusleeping this got changed back again-- i'll update this path

jbenet · 2015-01-14T02:22:28Z

beautifully simple implementation 👍

some cases may come up later where we want to send signals to reprovide, but nothing comes to mind.

Oh, one question: are provider records and provider's addresses kept in a distinct spot in dht? e.g.:

1. /ip4/1.2.3.4/..../Qmlorien... publishes key Qmkey

2. Qmcelebdil stores (Qmlorien -> Qmkey, and Qmlorien -> /ip4/1.2.3.4/..../Qmlorien...)

3. machine holding Qmlorien changes network (it's a laptop). 
  it reconnects at /ip4/4.3.2.1/.../Qmlorien

4. /ip4/4.3.2.1/.../Qmlorien talks to the world

5. through queries unrelated to Qmkey, Qmcelebdil finds out that
  (Qmlorien -> /ip4/4.3.2.1/..../Qmlorien...)

6. if Qmcelebdil serves provider records for Qmkey, and includes Qmlorien,
   it should give (Qmlorien -> /ip4/4.3.2.1/..../Qmlorien...)

jbenet · 2015-01-14T02:24:58Z

(this reminds me we need to evict user's addresses from the peerstore over time), tricky: only for addresses we find outside, bootstrap addrs should remain the same. and our own addrs should be up to date with our interfaces). laptop moving around will surely mess ipfs up

whyrusleeping · 2015-01-14T03:50:17Z

everything failed?

whyrusleeping · 2015-01-14T03:51:27Z

@briantigerchow your commit fixes things, no need to rebase or anything

jbenet · 2015-01-14T04:53:34Z

@whyrusleeping we need to rebase to make sure every commit passes tests.

whyrusleeping · 2015-01-14T05:20:43Z

oh yeah... thats right. duh

jbenet · 2015-01-14T05:54:40Z

@whyrusleeping can you answer question above, on "provider records and provider's addresses kept in a distinct spot"?

whyrusleeping · 2015-01-14T06:00:28Z

No, i dont think the dht stores addresses, thats entirely handled by the peerstore as far as i know

jbenet · 2015-01-14T06:05:31Z

interesting bitswap test hang: https://build.protocol-dev.com/job/gotest/666/console

jbenet · 2015-01-14T06:07:29Z

~~"fuck it" -jenkins~~ https://build.protocol-dev.com/job/build%20docker%20image/730/console

nevermind, that's correct. I read "docker image" as "docker test". my bad.

whyrusleeping · 2015-01-14T06:10:18Z

no idea on that bitswap hang, never seen that test hang before. ill look into it in a little bit, but i doubt we will see it again... (so there goes any hope of getting a debug log! )

make vendor

jbenet · 2015-01-14T06:13:24Z

I screwed up including davecheney's go-crosscompile. just removed.

godep should use git. omg. can't wait till we build a real dep manager with ipfs.

jbenet · 2015-01-14T06:18:41Z

Before merging this in, it should have a test. maybe:

spin up 2 nodes, A and B. A with a blockstore with a bunch of blocks. B should get provide messages.

whyrusleeping · 2015-01-15T03:22:23Z

I think I also want to add a command so a user can manually trigger the reprovide.

jbenet · 2015-01-15T21:52:29Z

@whyrusleeping is that test ready? if you cant get to it to day lmk and i'll do it (nbd, just want to push the ball forward)

whyrusleeping · 2015-01-15T21:58:08Z

Yeah, the test is pushed. The jenkins failure is one of the race conditions from #569

whyrusleeping · 2015-01-16T02:35:15Z

@jbenet RFM?

jbenet · 2015-01-16T03:10:20Z

@whyrusleeping not ready, @briantigerchow commits need squashing. I can do it for you if you want just lmk

whyrusleeping · 2015-01-16T03:37:37Z

@jbenet I can get to it in about 30 minutes, otherwise you can do it if you want.

whyrusleeping · 2015-01-16T06:48:36Z

Alright, all the tests are passing, but I have one issue, if you start the daemon and fail to bootstrap quickly enough, the reprovide will fail since there wont be any peers in the routing table to provide to. I think that the bootstrapping operation performed in core should block until it completes the first round of bootstrapping, this will solve the reprovides issue and provide what I think are better semantics to the creation of a node.

jbenet · 2015-01-16T07:10:03Z

if you start the daemon and fail to bootstrap quickly enough, the reprovide will fail since there wont be any peers in the routing table to provide to.

We should have a test case with precisely this. should this be an error? it probably should silently drop "no connections" errors coming from routing. (i.e. maybe log them, but should the reprovider semantics be fulfilled? @briantigerchow thoughts?

I think that the bootstrapping operation performed in core should block until it completes the first round of bootstrapping, this will solve the reprovides issue and provide what I think are better semantics to the creation of a node.

Hm, that may slow down other things. but might be good. @briantigerchow ?

btc · 2015-01-16T07:55:54Z

We should have a test case with precisely this. should this be an error? it probably should silently drop "no connections" errors coming from routing. (i.e. maybe log them, but should the reprovider semantics be fulfilled? @briantigerchow thoughts?

It's probably a good thing that routing returns an error if it was unable to provide the value to at least one peer. And errors shouldn't be dropped in this case. We happen to be in a position to handle them.

Here, it might be alright to retry until a routing.Provide succeeds (exponential back-off with a reasonable cap could work). Doesn't make sense to skip the key.

github.com/cenkalti/backoff would make it simple.

It might be sufficient to log a Warning to the console if routing.Provide is unsuccessful for more than N seconds (15? 30? 60?). Unlikely to happen. But it makes sure that things don't get swept under the rug.

Hm, that may slow down other things. but might be good. @briantigerchow ?

Same thoughts as you. I'd be down to try it and see how it affects usage. Low risk experiment.

whyrusleeping · 2015-01-17T03:43:35Z

I like the idea of a retry with a backoff. Ill hack that up

whyrusleeping · 2015-01-17T04:38:22Z

There we go! @jbenet final once over?

jbenet · 2015-01-17T11:02:59Z

@whyrusleeping LGTM! i'll merge.

basic reprovider implementation

fix incorrect error handling during provider record lookups

whyrusleeping added the status/in-progress In progress label Jan 13, 2015

btc reviewed Jan 13, 2015
View reviewed changes

whyrusleeping force-pushed the feat/reprovide branch from a43a5ae to dcce1f0 Compare January 14, 2015 00:22

whyrusleeping force-pushed the feat/reprovide branch from dcce1f0 to 2694e9b Compare January 14, 2015 01:45

jbenet reviewed Jan 14, 2015
View reviewed changes

jbenet force-pushed the feat/reprovide branch 2 times, most recently from 4d57624 to 2ae9340 Compare January 14, 2015 05:53

jbenet and others added 3 commits January 13, 2015 22:12

vendor fixed go-fuse-version

dc6b548

basic reprovider implementation

3269986

make vendor

reprovide: move core's reprovider into online services

f068c89

jbenet force-pushed the feat/reprovide branch from c918444 to f068c89 Compare January 14, 2015 06:12

add test for reprovider and slight refactor

a7650b2

jbenet mentioned this pull request Jan 15, 2015

Re-Provide Periodically + After startup #414

Closed

jbenet added this to the α milestone Jan 15, 2015

jbenet assigned whyrusleeping Jan 15, 2015

selected changes from CR

a6ec12a

whyrusleeping force-pushed the feat/reprovide branch from 491eaf9 to a6ec12a Compare January 16, 2015 06:17

add backoff retry to reprovider

acfc352

whyrusleeping force-pushed the feat/reprovide branch from 29daa82 to acfc352 Compare January 17, 2015 04:34

whyrusleeping added codereview and removed status/in-progress In progress labels Jan 17, 2015

core/cmds: fix go-fuse-version path

00b099c

jbenet added a commit that referenced this pull request Jan 17, 2015

Merge pull request #554 from jbenet/feat/reprovide

2963f48

basic reprovider implementation

jbenet merged commit 2963f48 into master Jan 17, 2015

jbenet removed codereview labels Jan 17, 2015

jbenet deleted the feat/reprovide branch January 17, 2015 11:03

ariescodescream pushed a commit to ariescodescream/go-ipfs that referenced this pull request Oct 23, 2021

Merge pull request ipfs#554 from libp2p/fix/553

4a4e60e

fix incorrect error handling during provider record lookups

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

basic reprovider implementation #554

basic reprovider implementation #554

whyrusleeping commented Jan 13, 2015

btc Jan 13, 2015

whyrusleeping Jan 14, 2015

btc commented Jan 14, 2015

jbenet Jan 14, 2015

btc Jan 14, 2015

whyrusleeping Jan 14, 2015

jbenet Jan 14, 2015

jbenet Jan 17, 2015

jbenet Jan 17, 2015

jbenet commented Jan 14, 2015

jbenet commented Jan 14, 2015

whyrusleeping commented Jan 14, 2015

whyrusleeping commented Jan 14, 2015

jbenet commented Jan 14, 2015

whyrusleeping commented Jan 14, 2015

jbenet commented Jan 14, 2015

whyrusleeping commented Jan 14, 2015

jbenet commented Jan 14, 2015

jbenet commented Jan 14, 2015

whyrusleeping commented Jan 14, 2015

jbenet commented Jan 14, 2015

jbenet commented Jan 14, 2015

whyrusleeping commented Jan 15, 2015

jbenet commented Jan 15, 2015

whyrusleeping commented Jan 15, 2015

whyrusleeping commented Jan 16, 2015

jbenet commented Jan 16, 2015

whyrusleeping commented Jan 16, 2015

whyrusleeping commented Jan 16, 2015

jbenet commented Jan 16, 2015

btc commented Jan 16, 2015

whyrusleeping commented Jan 17, 2015

whyrusleeping commented Jan 17, 2015

jbenet commented Jan 17, 2015

basic reprovider implementation #554

basic reprovider implementation #554

Conversation

whyrusleeping commented Jan 13, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

btc commented Jan 14, 2015

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

jbenet commented Jan 14, 2015

jbenet commented Jan 14, 2015

whyrusleeping commented Jan 14, 2015

whyrusleeping commented Jan 14, 2015

jbenet commented Jan 14, 2015

whyrusleeping commented Jan 14, 2015

jbenet commented Jan 14, 2015

whyrusleeping commented Jan 14, 2015

jbenet commented Jan 14, 2015

jbenet commented Jan 14, 2015

whyrusleeping commented Jan 14, 2015

jbenet commented Jan 14, 2015

jbenet commented Jan 14, 2015

whyrusleeping commented Jan 15, 2015

jbenet commented Jan 15, 2015

whyrusleeping commented Jan 15, 2015

whyrusleeping commented Jan 16, 2015

jbenet commented Jan 16, 2015

whyrusleeping commented Jan 16, 2015

whyrusleeping commented Jan 16, 2015

jbenet commented Jan 16, 2015

btc commented Jan 16, 2015

whyrusleeping commented Jan 17, 2015

whyrusleeping commented Jan 17, 2015

jbenet commented Jan 17, 2015