Skip to content

feat: improve sequencer metrics #461

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.

Sign up for GitHub

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

Open
georgehao wants to merge 9 commits into
base: main
Choose a base branch
from
Open

feat: improve sequencer metrics #461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3e772db
Improve sequencer metrics
georgehao Dec 18, 2025
f8a42f4
fix comments
georgehao Jan 12, 2026
e7f150d
Merge branch 'main' into feat/improve_sequencer_metrics
georgehao Jan 12, 2026
83517bc
udpate
georgehao Jan 12, 2026
d941f8b
fix lint
georgehao Jan 12, 2026
d0a867c
fix
georgehao Jan 12, 2026
cb6a18c
fix conflict
georgehao Jan 13, 2026
0d607ef
fix comments
georgehao Jan 20, 2026
0bbc7b9
fix lint
georgehao Jan 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions crates/chain-orchestrator/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,13 +292,17 @@ impl<
}
}
SequencerEvent::PayloadReady(payload_id) => {
if let Some(block) = self
let block = self
.sequencer
.as_mut()
.expect("sequencer must be present")
.finalize_payload_building(payload_id, &mut self.engine)
.await?
{
.await?;

self.metric_handler.finish_all_block_building_recording();
self.metric_handler.finish_block_building_interval_recording();

if let Some(block) = block {
let block_info: L2BlockInfoWithL1Messages = (&block).into();
self.database
.update_l1_messages_from_l2_blocks(vec![block_info.clone()])
Expand All @@ -307,7 +311,7 @@ impl<
.as_mut()
.expect("signer must be present")
.sign_block(block.clone())?;
self.metric_handler.finish_block_building_recording();
self.metric_handler.finish_no_empty_block_building_recording();
Copy link
Collaborator

@frisitano frisitano Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this function doesn't do anything because you already take the .block_building_start.take() in finish_all_block_building_recording.:

                let block = self
                    .sequencer
                    .as_mut()
                    .expect("sequencer must be present")
                    .finalize_payload_building(payload_id, &mut self.engine)
                    .await?

                self.metric_handler.finish_all_block_building_recording(); # block_building_start is taken here
                self.metric_handler.finish_block_building_interval_recording();

                if let Some(block) = block {
                    let block_info: L2BlockInfoWithL1Messages = (&block).into();
                    self.database
                        .update_l1_messages_from_l2_blocks(vec![block_info.clone()])
                        .await?;
                    self.signer
                        .as_mut()
                        .expect("signer must be present")
                        .sign_block(block.clone())?;
                     # this doesn't do anything because block_building_start is now None.
                    self.metric_handler.finish_no_empty_block_building_recording();
                    return Ok(Some(ChainOrchestratorEvent::BlockSequenced(block)));
                }

Copy link
Member Author

@georgehao georgehao Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rollup-node/crates/chain-orchestrator/src/metrics.rs

Lines 22 to 27 in 3e772db

pub(crate) fn start_block_building_recording(&mut self) {
if self.block_building_meter.block_building_start.is_some() {
tracing::warn!(target: "scroll::chain_orchestrator", "block building recording is already ongoing, overwriting");
}
self.block_building_meter.block_building_start = Some(Instant::now());
}

I think no issue here, because it assign value here, calculates twice I think it's ok

Copy link
Collaborator

@frisitano frisitano Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rollup-node/crates/chain-orchestrator/src/metrics.rs

Lines 41 to 53 in cb6a18c

pub(crate) fn finish_all_block_building_recording(&mut self) {
let duration = self
.block_building_meter
.block_building_start
.take()
.map(|block_building_start| block_building_start.elapsed());
if let Some(duration) = duration {
self.block_building_meter
.metric
.all_block_building_duration
.record(duration.as_secs_f64());
}
}

You take self.block_building_meter.block_building_start.take() here.

You then try and take it again below which will always be None.

rollup-node/crates/chain-orchestrator/src/metrics.rs

Lines 29 to 39 in cb6a18c

/// The duration of the current block building task if any.
pub(crate) fn finish_no_empty_block_building_recording(&mut self) {
let duration = self
.block_building_meter
.block_building_start
.take()
.map(|block_building_start| block_building_start.elapsed());
if let Some(duration) = duration {
self.block_building_meter.metric.block_building_duration.record(duration.as_secs_f64());
}
}

Copy link
Member Author

@georgehao georgehao Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's right

Copy link
Member Author

@georgehao georgehao Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

struct Test {
    pub block_building_start: Option<Instant>,
}

impl Test {
    fn finish_no_empty_block_building_recording(&mut self) {
        if let Some(block_build_start) = self.block_building_start {
            let duration = block_build_start.elapsed();
            println!("Time taken: {:?}", duration);
        }
    }

    fn finish_all_block_building_recording(&mut self) {
        if let Some(block_build_start) = self.block_building_start {
            let duration = block_build_start.elapsed();
            println!("Time taken(2): {:?}", duration);
        }
    }
}
#[tokio::main]
async fn main() {
    let mut t = Test {
        block_building_start: Option::from(Instant::now()),
    };

    time::sleep(Duration::from_secs(2)).await;
    t.finish_no_empty_block_building_recording();

    time::sleep(Duration::from_secs(2)).await;
    t.finish_all_block_building_recording();
}

This is ok

return Ok(Some(ChainOrchestratorEvent::BlockSequenced(block)));
}
}
Expand Down
44 changes: 37 additions & 7 deletions crates/chain-orchestrator/src/metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,44 @@ impl MetricsHandler {

/// Starts tracking a new block building task.
pub(crate) fn start_block_building_recording(&mut self) {
if self.block_building_meter.start.is_some() {
if self.block_building_meter.block_building_start.is_some() {
tracing::warn!(target: "scroll::chain_orchestrator", "block building recording is already ongoing, overwriting");
}
self.block_building_meter.start = Some(Instant::now());
self.block_building_meter.block_building_start = Some(Instant::now());
}

/// The duration of the current block building task if any.
pub(crate) fn finish_block_building_recording(&mut self) {
let duration = self.block_building_meter.start.take().map(|start| start.elapsed());
if let Some(duration) = duration {
pub(crate) fn finish_no_empty_block_building_recording(&self) {
if let Some(block_build_start) = self.block_building_meter.block_building_start {
let duration = block_build_start.elapsed();
self.block_building_meter.metric.block_building_duration.record(duration.as_secs_f64());
}
}

pub(crate) fn finish_all_block_building_recording(&self) {
if let Some(block_build_start) = self.block_building_meter.block_building_start {
let duration = block_build_start.elapsed();
self.block_building_meter
.metric
.all_block_building_duration
.record(duration.as_secs_f64());
}
}

pub(crate) fn finish_block_building_interval_recording(&mut self) {
let interval = self
.block_building_meter
.last_block_building_time
.take()
.map(|last_block_building_time| last_block_building_time.elapsed());
if let Some(interval) = interval {
self.block_building_meter
.metric
.consecutive_block_interval
.record(interval.as_secs_f64());
}
self.block_building_meter.last_block_building_time = Some(Instant::now());
}
}

impl Default for MetricsHandler {
Expand Down Expand Up @@ -104,13 +129,18 @@ pub(crate) struct ChainOrchestratorMetrics {
#[derive(Debug, Default)]
pub(crate) struct BlockBuildingMeter {
metric: BlockBuildingMetric,
start: Option<Instant>,
block_building_start: Option<Instant>,
last_block_building_time: Option<Instant>,
}

/// Block building related metric.
#[derive(Metrics, Clone)]
#[metrics(scope = "chain_orchestrator")]
pub(crate) struct BlockBuildingMetric {
/// The duration of the block building task.
/// The duration of the block building task without empty block
block_building_duration: Histogram,
/// The duration of the block building task for all blocks include empty block
all_block_building_duration: Histogram,
/// The duration of the block interval include empty block
consecutive_block_interval: Histogram,
}
Loading