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

Sync refactoring to improve performance #1000

Merged
merged 10 commits into from
Oct 3, 2019
Merged

Sync refactoring to improve performance #1000

merged 10 commits into from
Oct 3, 2019

Conversation

AlexandraRoatis
Copy link
Contributor

Description

Rewrote the sync algorithm to use a header request manager that keeps track of peer states and received headers. Simplifies sync state management and improves performance.

Type of change

Insert x into the following checkboxes to confirm (eg. [x]):

  • Bug fix.
  • New feature.
  • Enhancement.
  • Unit test.
  • Breaking change (a fix or feature that causes existing functionality to not work as expected).
  • Requires documentation update.

@AlexandraRoatis AlexandraRoatis added enhancement New feature or request unit tests labels Oct 2, 2019
@AlexandraRoatis AlexandraRoatis added this to the 0.4.2 milestone Oct 2, 2019
@AlexandraRoatis AlexandraRoatis self-assigned this Oct 2, 2019
// delay sending the request if we have multiple peers and the last sent message was to
// the current peer by placing the request in the pendingRequests that will be processed
// during the next iteration
if (mgr.getActiveNodes().size() > 1 && node.getIdHash() == lastNode.getIdHash()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

not a big concern, the size check can just use " ! isEmpty()"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, here the difference is between 1 or larger. If only 1 peer exists, we will not enforce the requirement of not sending another request to the same peer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah, okay.

@AlexandraRoatis AlexandraRoatis force-pushed the AKI-373 branch 3 times, most recently from c38ce32 to 77d9322 Compare October 3, 2019 00:10
 - algorithm to avoid sending consecutive requests to the same peer
 when having multiple peers;
 - merged TaskWrite into TaskSend.
 - Removed the storage of status blocks since it was heavy and only marginally useful.
 - unifies state tracking for making header requests;
 - managing received responses;
 - initial unit tests.
 - simplified import block functionality;
 - removed sync-related constants from p2p module.
@AlexandraRoatis AlexandraRoatis merged commit 4c4794d into master Oct 3, 2019
@AlexandraRoatis AlexandraRoatis deleted the AKI-373 branch October 3, 2019 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants