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

p2p: Seed node will disconnect from persistent peer if inbound #3362

Closed
unclezoro opened this issue Feb 28, 2019 · 3 comments
Closed

p2p: Seed node will disconnect from persistent peer if inbound #3362

unclezoro opened this issue Feb 28, 2019 · 3 comments
Assignees
Labels
C:p2p Component: P2P pkg T:bug Type Bug (Confirmed)
Milestone

Comments

@unclezoro
Copy link
Contributor

refer to #3304

For a seed node that want catch up with chain, it will only connected to a persistent peer,
but even a peer is a persistent peer of seed according to it's config file, in the runtime,
the peer may be inbounded peer(which is not persistent) or address pick from addressbook(also not persistent), witch cause the seed node disconnected to its persistent peer.

@ebuchman
Copy link
Contributor

ebuchman commented Mar 2, 2019

So is this specifically about persistent_peer setting when seed_mode=true? Are you saying that the seed will disconnect from an inbound peer even if that peer is listed in the persistent_peers? Or is there something else here?

@ebuchman ebuchman added the C:p2p Component: P2P pkg label Mar 2, 2019
@ebuchman ebuchman added this to the v0.31.1 milestone Mar 2, 2019
@unclezoro
Copy link
Contributor Author

the seed will disconnect from an inbound peer even if that peer is listed in the persistent_peers exactly.

@melekes melekes self-assigned this Mar 25, 2019
@xla xla changed the title peer.persistent is not accurate p2p: Seed node will disconnect from persistent peer if inbound Mar 28, 2019
@xla xla added the T:bug Type Bug (Confirmed) label Mar 28, 2019
melekes added a commit that referenced this issue Apr 25, 2019
Previously only outbound peers can be persistent. Now, even if the peer
is inbound, if it's marked as persistent, when/if conn is lost,
Tendermint will try to reconnect.

Plus, seed won't disconnect from inbound peer if it's marked as
persistent. Fixes #3362
melekes added a commit that referenced this issue May 3, 2019
## Description

Previously only outbound peers can be persistent.

Now, even if the peer is inbound, if it's marked as persistent, when/if conn is lost,
Tendermint will try to reconnect. This part is actually optional and can be reverted.

Plus, seed won't disconnect from inbound peer if it's marked as
persistent. Fixes #3362

## Commits

* make persistent prop independent of conn direction

Previously only outbound peers can be persistent. Now, even if the peer
is inbound, if it's marked as persistent, when/if conn is lost,
Tendermint will try to reconnect.

Plus, seed won't disconnect from inbound peer if it's marked as
persistent. Fixes #3362

* fix TestPEXReactorDialPeer test

* add a changelog entry

* update changelog

* add two tests

* reformat code

* test UnsafeDialPeers and UnsafeDialSeeds

* add TestSwitchDialPeersAsync

* spec: update p2p/config spec

* fixes after Ismail's review

* Apply suggestions from code review

Co-Authored-By: melekes <anton.kalyaev@gmail.com>

* fix merge conflict

* remove sleep from TestPEXReactorDoesNotDisconnectFromPersistentPeerInSeedMode

We don't need it actually.
@melekes
Copy link
Contributor

melekes commented May 3, 2019

Should be fixed by #3593.

@melekes melekes closed this as completed May 3, 2019
brapse pushed a commit to brapse/tendermint that referenced this issue Jun 5, 2019
)

## Description

Previously only outbound peers can be persistent.

Now, even if the peer is inbound, if it's marked as persistent, when/if conn is lost,
Tendermint will try to reconnect. This part is actually optional and can be reverted.

Plus, seed won't disconnect from inbound peer if it's marked as
persistent. Fixes tendermint#3362

## Commits

* make persistent prop independent of conn direction

Previously only outbound peers can be persistent. Now, even if the peer
is inbound, if it's marked as persistent, when/if conn is lost,
Tendermint will try to reconnect.

Plus, seed won't disconnect from inbound peer if it's marked as
persistent. Fixes tendermint#3362

* fix TestPEXReactorDialPeer test

* add a changelog entry

* update changelog

* add two tests

* reformat code

* test UnsafeDialPeers and UnsafeDialSeeds

* add TestSwitchDialPeersAsync

* spec: update p2p/config spec

* fixes after Ismail's review

* Apply suggestions from code review

Co-Authored-By: melekes <anton.kalyaev@gmail.com>

* fix merge conflict

* remove sleep from TestPEXReactorDoesNotDisconnectFromPersistentPeerInSeedMode

We don't need it actually.
brapse pushed a commit to brapse/tendermint that referenced this issue Jun 5, 2019
)

## Description

Previously only outbound peers can be persistent.

Now, even if the peer is inbound, if it's marked as persistent, when/if conn is lost,
Tendermint will try to reconnect. This part is actually optional and can be reverted.

Plus, seed won't disconnect from inbound peer if it's marked as
persistent. Fixes tendermint#3362

## Commits

* make persistent prop independent of conn direction

Previously only outbound peers can be persistent. Now, even if the peer
is inbound, if it's marked as persistent, when/if conn is lost,
Tendermint will try to reconnect.

Plus, seed won't disconnect from inbound peer if it's marked as
persistent. Fixes tendermint#3362

* fix TestPEXReactorDialPeer test

* add a changelog entry

* update changelog

* add two tests

* reformat code

* test UnsafeDialPeers and UnsafeDialSeeds

* add TestSwitchDialPeersAsync

* spec: update p2p/config spec

* fixes after Ismail's review

* Apply suggestions from code review

Co-Authored-By: melekes <anton.kalyaev@gmail.com>

* fix merge conflict

* remove sleep from TestPEXReactorDoesNotDisconnectFromPersistentPeerInSeedMode

We don't need it actually.
brapse pushed a commit to brapse/tendermint that referenced this issue Jun 5, 2019
)

## Description

Previously only outbound peers can be persistent.

Now, even if the peer is inbound, if it's marked as persistent, when/if conn is lost,
Tendermint will try to reconnect. This part is actually optional and can be reverted.

Plus, seed won't disconnect from inbound peer if it's marked as
persistent. Fixes tendermint#3362

## Commits

* make persistent prop independent of conn direction

Previously only outbound peers can be persistent. Now, even if the peer
is inbound, if it's marked as persistent, when/if conn is lost,
Tendermint will try to reconnect.

Plus, seed won't disconnect from inbound peer if it's marked as
persistent. Fixes tendermint#3362

* fix TestPEXReactorDialPeer test

* add a changelog entry

* update changelog

* add two tests

* reformat code

* test UnsafeDialPeers and UnsafeDialSeeds

* add TestSwitchDialPeersAsync

* spec: update p2p/config spec

* fixes after Ismail's review

* Apply suggestions from code review

Co-Authored-By: melekes <anton.kalyaev@gmail.com>

* fix merge conflict

* remove sleep from TestPEXReactorDoesNotDisconnectFromPersistentPeerInSeedMode

We don't need it actually.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:p2p Component: P2P pkg T:bug Type Bug (Confirmed)
Projects
None yet
Development

No branches or pull requests

4 participants