-
Notifications
You must be signed in to change notification settings - Fork 315
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
Improve logging during sync #9244
base: master
Are you sure you want to change the base?
Improve logging during sync #9244
Conversation
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, with a question on the return of SyncManager::getSyncStatus
peerSync.getStartingSlot(), | ||
new SyncingTarget( | ||
new SlotAndBlockRoot(bestPeerStatus.getHeadSlot(), bestPeerStatus.getHeadRoot()), | ||
1)); |
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 const 1 here might be a bit misleading if it comes out in one of the logs, should we print the total amount of peers instead even though they might not all have the same target?
let's put this LOG back to Lines 71 to 72 in 3d0dd71
|
f6dc3d0
to
e32b5ff
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.
I added an experimental change to the PR, adding more info taken from the BatchSync.
It is not good in the current form but we can play with it until we find the final form.
8be0071
to
a79fcf0
Compare
a053ade
to
24eb783
Compare
PR Description
Fixed Issue(s)
fixes #9220
Documentation
doc-change-required
label to this PR if updates are required.Changelog