feat(net): disconnect from inactive nodes if necessary by 317787106 · Pull Request #5924

feat(net): disconnect from inactive nodes if necessary by 317787106 · Pull Request #5924 · tronprotocol/java-tron

Conversation

What does this PR do?
To maintain the stability of the network and prevent node isolation, the node needs to be disconnected from inactive nodes on a regular basis. Three strategies are adopted:

If the node is in a LAN and the number of peers is greater than or equal to minConnection, disconnect from the earliest inactive peer that has been inactive for more than the threshold time.
If the node is isolated, disconnect from one active peer if the active number is greater than or equal to minActiveConnection. At the same time, disconnect from some passive peers but ensure that the retention percentage of peers is greater than or equal to 80%.
Optimize the random disconnect policy and only disconnect from the peer that has remained inactive for too long.
Why are these changes required?

This PR has been tested by:

Unit Tests
Manual Testing

Follow up

Extra details

jiangyuanshu added 4 commits

July 22, 2024 15:39

		private static final int initialDelay = 300;
		private static final String esName = "resilience-service";
		private final ScheduledExecutorService executor = ExecutorServiceManager
		.newSingleThreadScheduledExecutor(esName);

Choose a reason for hiding this comment

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

I see that you have added 3 tasks to be scheduled and executed in the same executor, the scheduling between different tasks will be affected, right? Have you considered splitting the tasks into different executors?

Choose a reason for hiding this comment

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

Any task only consume several milliseconds at most. There is no need to split them in different executors.

		List<PeerConnection> peers = tronNetDelegate.getActivePeer().stream()
		.filter(peer -> !peer.isDisconnect())
		.filter(peer -> now - peer.getLastActiveTime() >= inactiveThreshold)
		.filter(peer -> !peer.getChannel().isTrustPeer())

Choose a reason for hiding this comment

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

Why change the original logic, not excluding active connections.

Choose a reason for hiding this comment

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

Original logic don't have the inactiveThreshold restrict, it has many conditates. The number of peers whose inactive time is more inactiveThreshold is very small, it has very little condidates, so there is no need to exclude active connections.

jiangyuanshu added 3 commits

July 25, 2024 16:07