-
Notifications
You must be signed in to change notification settings - Fork 371
replay: adding better metrics #7583
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
Conversation
Performance Measurements ⏳
|
src/disco/metrics/metrics.xml
Outdated
|
|
||
| <counter name="SchedFull" summary="Times where sched is full and a FEC set can't be processed" /> | ||
| <counter name="ReasmEmpty" summary="Times where reasm is empty and a FEC set can't be processed" /> | ||
| <counter name="LeaderBidWait" summary="Times where replay is blocked by the the PoH tile not sending an end of leader message" /> |
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 the
src/discof/replay/fd_replay_tile.c
Outdated
| if( FD_UNLIKELY( (fec = fd_reasm_peek( ctx->reasm ))==NULL ) ) { | ||
| ctx->metrics.reasm_empty++; | ||
| ctx->metrics.reasm_latest_slot = ULONG_MAX; | ||
| ctx->metrics.reasm_latest_fec_idx = ULONG_MAX; |
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.
We might want these to monotonically increase, rather than resetting to ULONG_MAX. The way metrics make their way into the database, we might miss some high watermark values if we reset? I don't think we lose any information if we keep the reasm_latest_* values untouched in this branch, since reasm_empty will go up and that'll indicate an empty reasm.
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.
yeah agreed
Performance Measurements ⏳
|
No description provided.