[refer #379] Add inactiveConnectionDeadline configuration settings pa… #380
Conversation
src/main/resources/reference.conf
Outdated
| @@ -144,6 +144,9 @@ scorex { | |||
| # Synchronization status update interval for stable regime | |||
| syncStatusRefreshStable = 4m | |||
|
|
|||
| # Dropping dead connections timeout | |||
There was a problem hiding this comment.
Timeout for dropping dead connections
| name = peerInfo.peerSpec.nodeName, | ||
| connectionType = peerInfo.connectionType.map(_.toString) | ||
| ) | ||
| case None => throw new Exception("Peer is not connected") |
There was a problem hiding this comment.
We shouldn't get here, but throwing exceptions in API is not good, log.error is enough (thus connected peers will be returned)
There was a problem hiding this comment.
Sorry, I do not know how to replace throw with just logging (because otherwise compiler reports error).
Actually I need to call "get", because Option is this case is guaranteed to be non empty. But using Option.get is now prohibited. May be you can suggest some other better to handle this situation in Scala (transform components of a sequence, ignoring elements which Option fields are empty).
There was a problem hiding this comment.
ah I see, replaced .map with flatmap, and not checking errors at all (such checks should be done in other places)
| @@ -9,11 +9,11 @@ import scorex.core.network.{ConnectionDirection, PeerSpec} | |||
| * Information about peer to be stored in PeerDatabase | |||
| * | |||
| * @param peerSpec - general information about the peer | |||
| * @param lastSeen - timestamp when this peer was last seen in the network | |||
| * @param lastHandshake - timestamp when this peer was last seen in the network | |||
There was a problem hiding this comment.
timestamp when last handshake was done (last seen sounds ambiguous, i.e. can be considered as time when we got a message from a peer last time)
| log.info(s"Dropping connection with ${cp.peerInfo}, last seen ${delta / 1000.0} seconds ago") | ||
| cp.handlerRef ! CloseConnection | ||
| val lastSeen = cp.lastMessage | ||
| if (lastSeen != 0) { |
There was a problem hiding this comment.
Then if no messages ever received from a peer and connection is dead, the connection will never be removed. I suggested to set lastMessage = now then creating connection, and then the check is not needed.
| @@ -316,7 +312,7 @@ class NetworkController(settings: NetworkSettings, | |||
|
|
|||
| val handler = context.actorOf(handlerProps) // launch connection handler | |||
| context.watch(handler) | |||
| val connectedPeer = ConnectedPeer(connectionId, handler, None) | |||
| val connectedPeer = ConnectedPeer(connectionId, handler, 0, None) | |||
There was a problem hiding this comment.
Why None? Maybe possible to get peer info from PeerManager (ConfirmConnection handling)
There was a problem hiding this comment.
peerInfo will be set when handshake is established.
peerInfo.notEmpty is used now to filter connected peers.
I wonder if we need to distinguish connected and handshaked peers?
…rameter