search

phoreproject / graphene

Phore Synapse working repository
MIT License
13 stars 6 forks source link

Evict connection #67

Closed wqking closed 5 years ago

wqking commented 5 years ago

Used the similar mechanism in Bitcoin to evict peers when the peers are full.

codecov-io commented 5 years ago

Codecov Report

Merging #67 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #67   +/-   ##
=======================================
  Coverage   34.62%   34.62%           
=======================================
  Files          23       23           
  Lines        3451     3451           
=======================================
  Hits         1195     1195           
  Misses       2121     2121           
  Partials      135      135

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 08724dd...816acef. Read the comment docs.

meyer9 commented 5 years ago

This looks great, but I'm concerned with how it interacts with the libp2p connection manager. Can you make sure this doesn't conflict with that?

wqking commented 5 years ago

I checked libp2p connection manager. I don't think this conflicts with it. And even if it conflicts with it, it can be easily solved by setting higher maxPeers (or just set 0 to disable it) to libp2p connection manager.

wqking commented 5 years ago

BTW I didn't test the eviction, it's not easy to simulate that many connections. Maybe I should run many nodes locally to test it?

meyer9 commented 5 years ago

I'll merge this in and then write an integration test for it. It will be good to see if integration tests work properly.