-
Notifications
You must be signed in to change notification settings - Fork 114
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 stats expansion & refactor #844
Conversation
5a67b17
to
7b56dc7
Compare
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.
The functionality looks good, but I'd like some more code cleanup before we merge this (as described in the comments). Also, please ensure full test coverage of the tracker classes.
modAionImpl/src/org/aion/zero/impl/sync/statistics/ResponseStats.java
Outdated
Show resolved
Hide resolved
modAionImpl/src/org/aion/zero/impl/sync/statistics/ResponseStatsTracker.java
Outdated
Show resolved
Hide resolved
modAionImpl/src/org/aion/zero/impl/sync/statistics/ResponseStatsTracker.java
Outdated
Show resolved
Hide resolved
modAionImpl/src/org/aion/zero/impl/sync/statistics/ResponseStatsTracker.java
Outdated
Show resolved
Hide resolved
modAionImpl/src/org/aion/zero/impl/sync/statistics/TopSeedsStatsTracker.java
Outdated
Show resolved
Hide resolved
76f1008
to
fbb53d2
Compare
modAionImpl/src/org/aion/zero/impl/sync/handler/BlockPropagationHandler.java
Outdated
Show resolved
Hide resolved
modAionImpl/src/org/aion/zero/impl/sync/handler/BlockPropagationHandler.java
Outdated
Show resolved
Hide resolved
cf540bc
to
41d71d2
Compare
modAionImpl/src/org/aion/zero/impl/sync/statistics/TopSeedsStatsTracker.java
Outdated
Show resolved
Hide resolved
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.
LGTM
Notice
It is not allowed to submit your PR to the master branch directly, please submit your PR to the master-pre-merge branch.
Description
Please include a brief summary of the change that this pull request proposes. Include any relevant motivation and context. List any dependencies required for this change.
BLOCKS
,RECEIPTS
andTRIE_DATA
to the enumRequestType
ResponseStatsTracker
to avoid code repetitionStandaloneBlockchain
fromSyncStatsTest
Todo: Log statistics regarding the new types of messages from the corresponding handlers/tasks (After fast sync is merged)
Fixes Issue #837
Type of change
Insert x into the following checkboxes to confirm (eg. [x]):
Testing
Please describe the tests you used to validate this pull request. Provide any relevant details for test configurations as well as any instructions to reproduce these results.
Verification
Insert x into the following checkboxes to confirm (eg. [x]):