Skip to content

return connected peers as providers - #686

Merged
jbenet merged 6 commits into
masterfrom
exchange-with-connected
Jan 29, 2015
Merged

return connected peers as providers#686
jbenet merged 6 commits into
masterfrom
exchange-with-connected

Conversation

@jbenet

@jbenet jbenet commented Jan 29, 2015

Copy link
Copy Markdown
Member

Also:

  • epictest: added test for bitswap wo routing
  • fixed three-legged-cat (i think)

@jbenet jbenet added the status/in-progress In progress label Jan 29, 2015
@jbenet
jbenet force-pushed the exchange-with-connected branch from 1456ec1 to 64191c1 Compare January 29, 2015 09:10
@jbenet

jbenet commented Jan 29, 2015

Copy link
Copy Markdown
Member Author

@briantigerchow

PASS: TestThreeLeggedCat1KBInstantaneous (0.01s)

I made the required change to make Bootstrap sync.

Comment thread exchange/bitswap/network/ipfs_impl.go Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not range over connectedPeers what if its value changes during the time between calls? Could hang if a peer is added in the meantime

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch. addressed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@whyrusleeping nice catch

dialing 4000 connections somehow keeps choking both travis and
jenkins. dialing this down to 500
@btc

btc commented Jan 29, 2015

Copy link
Copy Markdown
Contributor

LGTM

jbenet added a commit that referenced this pull request Jan 29, 2015
return connected peers as providers
@jbenet
jbenet merged commit 32a68c6 into master Jan 29, 2015
@jbenet jbenet removed the status/in-progress In progress label Jan 29, 2015
@jbenet
jbenet deleted the exchange-with-connected branch January 29, 2015 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants