feat(net): add logs and adjust disconnection strategy #5944
Conversation
| if (peers.size() > disconnectSize) { | ||
| peers = peers.subList(0, disconnectSize); | ||
| } | ||
| peers.forEach(peer -> disconnectFromPeer(peer, ReasonCode.BAD_PROTOCOL)); | ||
| logger.info("All peer Size:{}, plan size:{}, candidate size:{}, real size:{}", peerSize, |
There was a problem hiding this comment.
This log will refresh the screen.
There was a problem hiding this comment.
No, only isolated node will print this log but this condition is rare. Even in this case, its period is 30 seconds.
| // disconnect from the node that has keep inactive for more than inactiveThreshold | ||
| // and its lastActiveTime is smallest | ||
| int peerSize = tronNetDelegate.getActivePeer().size(); | ||
| if (peerSize >= CommonParameter.getInstance().getMinConnections()) { |
There was a problem hiding this comment.
Is this conditional statement redundant?
There was a problem hiding this comment.
It's not redundant, condition of lan is different: peerSize > CommonParameter.getInstance().getMinActiveConnections().
There was a problem hiding this comment.
Why is it '>', shouldn't lan nodes be '=' in most scenarios?
There was a problem hiding this comment.
It's ">=", not ">".
There was a problem hiding this comment.
I'm talking about why the comparison in the isLanNode method is '>'
There was a problem hiding this comment.
You are right, I will revise it.
There was a problem hiding this comment.
So let's go back to the original question. How do you think about it?
There was a problem hiding this comment.
Even it's a lan node, it will disconnect from some nodes only if peerSize >= CommonParameter.getInstance().getMinConnections(). Later condition is more strict. It's not redundant.
lvs007
changed the title
lvs007
changed the title
317787106
changed the title
317787106
changed the title
What does this PR do?
Why are these changes required?
This PR has been tested by:
Follow up
Extra details