Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(net) : optimize the isIdle method #5879

Closed
wants to merge 4 commits into from

Conversation

zeusoo001
Copy link
Contributor

@zeusoo001 zeusoo001 commented Jun 24, 2024

What does this PR do?
Optimize the isIdle method. refer to issue #5913

Why are these changes required?
During the block synchronization process, if the broadcast list has not been received, the synchronization may fail. The detailed process is as follows:

  1. After processing the chain inventory message, set fetchFlag to true.
  2. The scheduler will execute the startFetchSyncBlock method to fetch the block, and at this time, fetchFlag will be set to false.
etchExecutor.scheduleWithFixedDelay(() -> {
 try {
   if (fetchFlag) {
     fetchFlag = false;
     startFetchSyncBlock();
   }
 } catch (Exception e) {
   logger.error("Fetch sync block error", e);
 }
}, 10, 1, TimeUnit.SECONDS);
  1. Since advInvRequest is not empty at this time, peer.isIdle() returns false, so after this scheduling, the block is not obtained, but fetchFlag is set to false and cannot be set back, so the block cannot be obtained later.
private void startFetchSyncBlock() {
 HashMap<PeerConnection, List<BlockId>> send = new HashMap<>();
 tronNetDelegate.getActivePeer().stream()
     .filter(peer -> peer.isNeedSyncFromPeer() && peer.isIdle())
     .filter(peer -> peer.isFetchAble())
  1. Since the block is not obtained, the peer status cannot be updated, the status check will fail, and the connection will be disconnected.

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

@@ -167,7 +167,7 @@ public void setBlockBothHave(BlockId blockId) {
}

public boolean isIdle() {
return advInvRequest.isEmpty() && syncBlockRequested.isEmpty() && syncChainRequested == null;
Copy link
Contributor

Choose a reason for hiding this comment

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

What were the considerations of this previous judgment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This logic has been around for a long time, and this special scenario may not have been considered before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants