-
Notifications
You must be signed in to change notification settings - Fork 44
Sync Committee: Batch Metrics + Lag Tracker #698
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
5a4d78d to
813819b
Compare
5a4d78d to
aedadff
Compare
aedadff to
769f8cc
Compare
akokoshn
left a comment
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.
May be keep aggregator in core directory and move lag_tracker to the metrics
Changed block-related metrics published by `Sync Committee` to match the new batching model: * Replaced `totalMainBlocksFetched` metric with `totalBatchesCreated` (counter); * Fixed the way `batchSizeBlocks` metric is collected (moved it to `RecordBatchCreated` method); * Replaced `txPerSingleProposal` metric published by `Proposer` with `batchSizeTxs` updated by `Aggregator`; * Renaming: `total_l1_tx_sent` -> `total_l1_state_updates`; `block_total_time_ms` -> `batch_total_proof_time_ms`;
Implemented new background worker to track per-shard block fetching lag and publish corresponding metrics to `SigNoz` * Defined `LagTracker` - new `Sync Committee` worker, responsible for fetching lag tracking; * Extended `SyncCommitteeMetricsHandler` with new `blockFetchingLag` metric;
769f8cc to
ea3fed9
Compare
Package I think And I'd like to keep |
akokoshn
left a comment
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.
Another idea - rename fetching to aggregator, but up to you)
| ) | ||
|
|
||
| lagTracker := fetching.NewLagTracker( | ||
| client, blockStorage, metricsHandler, fetching.NewDefaultLagTrackerConfig(), logger, |
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.
nice to have pass LagTrackerConfig options from command line
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 think we can do it later if needed, for now let's keep configuration as compact as possible
Block Metrics Adjustments
Changed block-related metrics published by
Sync Committeeto match the new batching model:totalMainBlocksFetchedmetric withtotalBatchesCreated(counter);batchSizeBlocksmetric is collected (moved it toRecordBatchCreatedmethod);txPerSingleProposalmetric published byProposerwithbatchSizeTxsupdated byAggregator;total_l1_tx_sent->total_l1_state_updates;block_total_time_ms->batch_total_proof_time_ms;Block Fetching Lag Tracker
Implemented new background worker to track per-shard block fetching lag and publish corresponding metrics to
SigNoz. By default, lag metrics are collected every 5 minutesLagTracker- newSync Committeeworker, responsible for fetching lag tracking;SyncCommitteeMetricsHandlerwith newblockFetchingLagmetric;