Skip to content
This repository has been archived by the owner on Apr 13, 2022. It is now read-only.

Minor updates for networking #369

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

polarker
Copy link

Three minor updates

  1. Enable pull model for incoming connections too. Right now, pull model is only enabled for outgoing connections.
  2. Both confirmed and unconfirmed connections are considered when checking the number of connections.
  3. Use remoteAddress as peer's address, if there is no address info in it's PeerInfo

@@ -67,10 +67,11 @@ class NetworkController(settings: NetworkSettings,
log.info(s"Declared address: ${scorexContext.externalNodeAddress}")

//bind to listen incoming connections
tcpManager ! Bind(self, bindAddress, options = Nil, pullMode = false)
tcpManager ! Bind(self, bindAddress, options = Nil, pullMode = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I am misunderstanding, I thought pull mode was purposely disabled here since to enable it would cause the network controller to respond to the TCP manager each time a new connection is attempted. But since each attempt to connect to peers is independent, there is no need to block subsequent connection requests.

Relevant documentation on pullMode in bind command
https://doc.akka.io/docs/akka/2.6.9/io-tcp.html?language=java#pull-mode-reading-for-inbound-connections

pullMode on connect makes sense to me as we want the PeerConnectionHandlers between remote peers to fully process and acknowledge messages to maintain a stable connection.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the late reply. Pull mode is supposed to throttle reading, not to throttle the number of connection requests. However, in Akka, both are throttled, so pull mode was disabled on purpose to not throttle the number of incoming connections.

In this pull request, pull mode is enabled to throttle only reading, and at the same time, sending a message to TCP manager to always accept new connections.

Base automatically changed from master to main February 3, 2021 23:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants