-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add PeerDAS reconstruction metrics #173
base: das
Are you sure you want to change the base?
Add PeerDAS reconstruction metrics #173
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.
Looks good, just few nits
recoveryComplete(); | ||
} catch (final Throwable t) { | ||
LOG.error( | ||
"Failed to reconstruct data column sidecar {}", sidecar::toLogString, () -> t); |
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.
It would be not very correct. We already have this sidecar, we want to reconstruct others. Let's log something like
"Failed to reconstruct data column sidecars {}", sidecar.getSlotAndBlockRoot(), t);
} | ||
} | ||
} | ||
|
||
private void recoveryComplete() { | ||
recovered = true; | ||
totalDataAvailabilityReconstructedColumns.inc( |
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.
Move promisesByColIdx.values().stream().mapToInt(List::size).sum()
to variable and reuse, please
RecoveringSidecarRetrieverTest fails, need to implement stub histogram for tests |
PR Description
Implemented PeerDAS metrics:
Fixed Issue(s)
fixes #65
Documentation
doc-change-required
label to this PR if updates are required.Changelog