Skip to content

p2p: Stop peers gracefully when stopping switch#3729

Merged
melekes merged 2 commits intotendermint:developfrom
SebastianElvis:stop-peers-gracefully
Jun 17, 2019
Merged

p2p: Stop peers gracefully when stopping switch#3729
melekes merged 2 commits intotendermint:developfrom
SebastianElvis:stop-peers-gracefully

Conversation

@SebastianElvis
Copy link
Contributor

This PR fixed TODO: gracefully disconnect from peers. in p2p/switch.go.

  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md

@codecov-io
Copy link

codecov-io commented Jun 17, 2019

Codecov Report

Merging #3729 into develop will increase coverage by 0.05%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #3729      +/-   ##
===========================================
+ Coverage    63.93%   63.98%   +0.05%     
===========================================
  Files          241      241              
  Lines        19856    19985     +129     
===========================================
+ Hits         12694    12787      +93     
- Misses        6125     6156      +31     
- Partials      1037     1042       +5
Impacted Files Coverage Δ
node/node.go 63.81% <ø> (ø) ⬆️
p2p/switch.go 70.62% <100%> (-0.25%) ⬇️
privval/socket_listeners.go 86.2% <0%> (-3.45%) ⬇️
mempool/clist_mempool.go 81.54% <0%> (-2.17%) ⬇️
blockchain/reactor.go 71.49% <0%> (-1.87%) ⬇️
blockchain/pool.go 80.59% <0%> (-1.32%) ⬇️
libs/db/prefix_db.go 55.62% <0%> (-0.57%) ⬇️
privval/signer_validator_endpoint.go 85.55% <0%> (ø) ⬆️
consensus/reactor.go 71.71% <0%> (+0.11%) ⬆️
p2p/pex/addrbook.go 68% <0%> (+0.5%) ⬆️
... and 3 more

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

Although I am not sure this PR actually changes anything except removing duplicated code. I think we already do "gracefully disconnect from peers", so the TODO can be removed.

p2p/switch.go Outdated
sw.metrics.Peers.Add(float64(-1))
}
sw.StopPeerGracefully(p)
sw.metrics.Peers.Add(float64(-1))
Copy link
Contributor

Choose a reason for hiding this comment

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

StopPeerGracefully already decrements this counter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

p2p/switch.go Outdated
if sw.peers.Remove(p) {
sw.metrics.Peers.Add(float64(-1))
}
sw.StopPeerGracefully(p)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to call sw.stopAndRemovePeer(peer, nil) to avoid excessive logging

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

@SebastianElvis
Copy link
Contributor Author

SebastianElvis commented Jun 17, 2019

👍

Although I am not sure this PR actually changes anything except removing duplicated code. I think we already do "gracefully disconnect from peers", so the TODO can be removed.

Yeah, it seems that the original code works.
Anyway, this PR can remove some duplicated code...

@melekes melekes merged commit ed18ffd into tendermint:develop Jun 17, 2019
unclezoro pushed a commit to unclezoro/tendermint that referenced this pull request Sep 6, 2019
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