-
Notifications
You must be signed in to change notification settings - Fork 115
[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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Timeout for dropping dead connections
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why None? Maybe possible to get peer info from PeerManager (ConfirmConnection handling)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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