ipfs add fails when routing partner goes offline

[btc]

repro:

1. with daemon running, add large file
2. local daemon will provide keys to the network. observe the ID of the peer that receives these keys
3. kill the remote peer
4. add command hangs and fails

[whyrusleeping]

This is interesting... mostly likely because that peer doesnt get removed in our local routing table.

[btc]

a) Do we need the swarm/network/service to call up to the client letting it know about the disconnection?

b) Or should the client discover the disconnection/unreachability when the client attempts to send a message out over the service?

With (a), there is a TOCTOU concern. With (b), it's kind of an awkward interface.

[jbenet]

I agree b) is awkward. (which is why just sending messages is weird. we don't live in a world with constant connectivity yet).

I think TRTTD is a).

This is where we really should have events (pub/sub (or NSNotificationCenter) style).

1. routing table adds peer p. subscribes to connection events on the network. something like net.NotificationSubscribe(p, handler)
2. p goes offline.
3. singleConn discovers this. closes. only open connection, MultiConn closes. Swarm closes it. announces that event. (notifications here could be in the swarm or directly in the Network).
4. routing table receives event, and evicts p.

Not sure this is the easiest route to go here, given it introduces a whole new way of passing information around that's not there presently. But it may not be too bad.

I think we should eventually have a notification center (per node) abstraction.

[btc]

I'll fix this issue using a system-wide notification center or pubsub.

It won't be feature rich, but it'll be a foundation cognizant of broader constraints.

[jbenet]

@maybebtc add it to the IpfsNode, and either hand it down in the context, or add it to the major subsystems that need it.

[btc]

SGTM

[jbenet]

@briantigerchow this is still a problem eh?

[btc]

Don't know for sure. Probably.

Needs a test.

1. Construct two cores and bootstrap them
2. Add/Cat
3. Break the mock net link midway through.
4. Ensure the Add was successful by attempting to Cat locally.

I suppose we want routing, bitswap, blockservice, etc. to respond wisely to the network errors that occur.

[jbenet]

Absolutely. That sounds good to me.

[btc]

I'll DRI this one.

[btc]

fixed in #424