Add BAL-related metrics#9714
Conversation
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
| protocolSpec | ||
| .getStateRootCommitterFactory() | ||
| .forBlock(protocolContext, blockHeader, blockAccessList); | ||
| blockProcessingMetrics.wrapStateRootCommitter( |
There was a problem hiding this comment.
we are only adding BAL metrics. why not having that only for the bal commiter ?
There was a problem hiding this comment.
This metric is under BLOCK_PROCESSING category. This approach allows for measuring state root computation for any committer. What is the downside? The overhead of starting and stopping a timer is negligible.
There was a problem hiding this comment.
There was a problem hiding this comment.
it is more related to the fact that we are calling recordBlockAccessListMetrics in a generic part even if we can be in a fork without BAL. as we have specific path for it i will prefer to call it inside or maybe rename it to recordBlockMetrics and only add bal metrics inside the method when it's available as you are doing now
There was a problem hiding this comment.
I changed the method to take the BAL instead of an optional and moved it so that it's called only when BALs are active.
| } | ||
|
|
||
| private long encodedSizeBytes(final BlockAccessList bal) { | ||
| final Bytes encoded = RLP.encode(out -> BlockAccessListEncoder.encode(bal, out)); |
There was a problem hiding this comment.
I feel the metrics will impact performance if we need to encode bal. i will say maybe not adding this one for the moment or find another way
There was a problem hiding this comment.
I removed this metric. Maybe this can be addressed together with this #9674 (comment)
|
Some tests are failing |
matkt
added
block-access-lists
amsterdam
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
* Add BAL-related metrics Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com> * Remove metric measuring size of RLP-encoded BALs Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com> * Initialize metricsSystem to NoOpMetricsSystem if null Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com> * Record BAL metrics only if BALs are active Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com> * fix nullpointer issue Signed-off-by: Karim Taam <karim.t2am@gmail.com> --------- Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com> Signed-off-by: Karim Taam <karim.t2am@gmail.com> Co-authored-by: Karim Taam <karim.t2am@gmail.com>
3 participants
Adds some BAL-related metrics inspired by this spec draft.
The draft assumes structured OTEL span instrumentation across the block-processing path, which isn't currently present in Besu (beyond JSON-RPC tracing), so this PR starts with adding lightweight metrics as a first step.
Important part of the BAL-related metrics will be related to prefetching, which will be added in #9656.