Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check max peer is not passed. #1897

Merged
merged 9 commits into from
Jan 11, 2020
Merged

Conversation

mfornet
Copy link
Member

@mfornet mfornet commented Dec 27, 2019

Make number of active peers don't exceed threshold from config.

Copy link
Collaborator

@bowenwang1996 bowenwang1996 left a comment

Choose a reason for hiding this comment

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

Can we add a test for this?

chain/network/src/peer_manager.rs Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 27, 2019

Codecov Report

Merging #1897 into staging will decrease coverage by 0.08%.
The diff coverage is 95.77%.

Impacted file tree graph

@@             Coverage Diff             @@
##           staging    #1897      +/-   ##
===========================================
- Coverage    86.59%   86.51%   -0.09%     
===========================================
  Files          166      167       +1     
  Lines        31572    31399     -173     
===========================================
- Hits         27341    27165     -176     
- Misses        4231     4234       +3
Impacted Files Coverage Δ
chain/network/src/peer.rs 74.9% <ø> (+0.14%) ⬆️
chain/network/src/test_utils.rs 93.1% <ø> (-2.46%) ⬇️
near/src/config.rs 95.2% <100%> (ø) ⬆️
chain/network/tests/routing.rs 100% <100%> (+2.08%) ⬆️
chain/network/tests/churn_attack.rs 100% <100%> (ø)
chain/network/tests/peer_handshake.rs 97.39% <100%> (-0.05%) ⬇️
chain/network/src/types.rs 75.4% <100%> (ø) ⬆️
chain/network/src/peer_manager.rs 77.65% <75%> (-0.5%) ⬇️
chain/network/tests/infinite_loop.rs 95% <95%> (ø)
chain/network/tests/runner/mod.rs 96.11% <96.11%> (ø)
... and 13 more

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 f1d858c...73d3868. Read the comment docs.

chain/network/tests/routing.rs Outdated Show resolved Hide resolved
runner.push(Action::CheckRoutingTable(0, vec![(1, vec![1]), (2, vec![2])]));
runner.push(Action::CheckRoutingTable(1, vec![(0, vec![0]), (2, vec![2])]));
runner.push(Action::CheckRoutingTable(2, vec![(1, vec![1]), (0, vec![0])]));
runner.push(Action::CheckRoutingTable(3, vec![]));
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is the number of peers checked?

Copy link
Member Author

@mfornet mfornet Dec 30, 2019

Choose a reason for hiding this comment

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

There routing table for peer 3 is empty since connection failed to establish.
runner.push(Action::CheckRoutingTable(3, vec![])); is enough to notice that peer3 have no route to any other peer in the system.

From other routing table we see that the triangle is still alive and new node is not connected. This test would fail before this PR.

@nearprotocol-bulldozer nearprotocol-bulldozer bot merged commit 179ee6d into staging Jan 11, 2020
@nearprotocol-bulldozer nearprotocol-bulldozer bot deleted the limit_max_peer branch January 11, 2020 00:29
SkidanovAlex pushed a commit that referenced this pull request Jan 29, 2020
* Check max peer is not passed.
* Solve racing incoming connections issues.
* Add test
* Update test
* Fix fmt
* Refactor network tests
* Refactor account propagation + infinite loop

Remove some println
* Fix peer_recover
* Move churn attack test out of routing
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