Skip to content

ethstats: report processing time#33231

Closed
MariusVanDerWijden wants to merge 2 commits intoethereum:masterfrom
MariusVanDerWijden:block-stats
Closed

ethstats: report processing time#33231
MariusVanDerWijden wants to merge 2 commits intoethereum:masterfrom
MariusVanDerWijden:block-stats

Conversation

@MariusVanDerWijden
Copy link
Member

@MariusVanDerWijden MariusVanDerWijden commented Nov 19, 2025

Implements https://notes.ethereum.org/@savid/block-observability

Draft for now, looking for feedback on it

fjl
fjl reviewed Nov 26, 2025
View reviewed changes
core/events.go Outdated

type ChainHeadEvent struct {
Header *types.Header
Time uint64
Copy link
Contributor

@fjl fjl Nov 26, 2025

Choose a reason for hiding this comment

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

Should be renamed to ProcessingTime or something, and the type should be time.Duration.

@fjl
Copy link
Contributor

fjl commented Nov 26, 2025 •
edited
Loading

Overall, I like the idea, but it'd be good to implement OpenTelemetry tracing in the engine API as well.

@MariusVanDerWijden MariusVanDerWijden marked this pull request as ready for review December 11, 2025 19:03
@MariusVanDerWijden
Copy link
Member Author

MariusVanDerWijden commented Dec 11, 2025

cc @Savid

Savid
Savid reviewed Dec 11, 2025
View reviewed changes
core/blockchain.go
defer func() {
if lastCanon != nil && bc.CurrentBlock().Hash() == lastCanon.Hash() {
bc.chainHeadFeed.Send(ChainHeadEvent{Header: lastCanon.Header()})
bc.chainHeadFeed.Send(ChainHeadEvent{Header: lastCanon.Header(), ProcessingTime: mclock.Now().Sub(stats.startTime)})
Copy link
Member

@Savid Savid Dec 11, 2025

Choose a reason for hiding this comment

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

I tested locally the other day and actually needed to add it to this one SetCanonical (which already has a start time too)

I'm wondering if we actually make a new ethstats event focused around engine_newPayloadVX calls and do the timing there?

The issue with existing block ethstats event is chainHeadFeed can be fired from 5+ different locations which would all require timing (which i think won't make sense in some cases) and the ethstats report also sends current head block which doesn't have timing value.

This new event block_new_payload (?) would only fire when valid block comes over the engine api. thoughts?

Copy link
Member

@Savid Savid Dec 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

i guess it's also fine if we accept block event sometimes returns no processingTime and just handle that on the ethstats server side

Copy link
Member

@Savid Savid Dec 12, 2025

Choose a reason for hiding this comment

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

PoC #33395

Copy link
Member Author

@MariusVanDerWijden MariusVanDerWijden Dec 16, 2025

Choose a reason for hiding this comment

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

yep, like yours better.

@Savid Savid mentioned this pull request Dec 12, 2025
Merged
@MariusVanDerWijden
Copy link
Member Author

MariusVanDerWijden commented Dec 16, 2025

closed in favor of https://github.com/ethereum/go-ethereum/pull/33395/changes

MariusVanDerWijden added a commit that referenced this pull request Jan 5, 2026
@Savid @MariusVanDerWijden
ethstats: report newPayload processing time to stats server (#33395)
a8a4804 
Add NewPayloadEvent to track engine API newPayload block processing
times and report them to ethstats. This enables monitoring of block
processing performance.

https://notes.ethereum.org/@savid/block-observability

related: #33231

---------

Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de>
weiihann added a commit to weiihann/go-ethereum that referenced this pull request Jan 8, 2026
@weiihann
Squashed commit of the following:
d57f256 
commit 64d22fd
Author: LittleBingoo <zpksdhr@gmail.com>
Date:   Thu Jan 8 11:49:13 2026 +0800

    internal/flags: update copyright year to 2026 (ethereum#33550)

commit 9623dcb
Author: rjl493456442 <garyrong0905@gmail.com>
Date:   Thu Jan 8 11:48:45 2026 +0800

    core/state: add cache statistics of contract code reader (ethereum#33532)

commit 01b39c9
Author: Ng Wei Han <47109095+weiihann@users.noreply.github.com>
Date:   Thu Jan 8 11:07:19 2026 +0800

    core/state, core/tracing: new state update hook (ethereum#33490)

    ### Description
    Add a new `OnStateUpdate` hook which gets invoked after state is
    committed.

    ### Rationale
    For our particular use case, we need to obtain the state size metrics at
    every single block when fuly syncing from genesis. With the current
    state sizer, whenever the node is stopped, the background process must
    be freshly initialized. During this re-initialization, it can skip some
    blocks while the node continues executing blocks, causing gaps in the
    recorded metrics.

    Using this state update hook allows us to customize our own data
    persistence logic, and we would never skip blocks upon node restart.

    ---------

    Co-authored-by: Gary Rong <garyrong0905@gmail.com>

commit 957a360
Author: cui <cuiweixie@gmail.com>
Date:   Wed Jan 7 10:02:27 2026 +0800

    core/vm: avoid escape to heap (ethereum#33537)

commit 7100084
Author: Csaba Kiraly <cskiraly@users.noreply.github.com>
Date:   Wed Jan 7 02:52:50 2026 +0100

    eth: txs fetch/send log at trace level only (ethereum#33541)

    This logging was too intensive at debug level, it is better to have it
    at trace level only.

    Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>

commit eaaa5b7
Author: rjl493456442 <garyrong0905@gmail.com>
Date:   Tue Jan 6 15:09:15 2026 +0800

    core: re-organize the stats category (ethereum#33525)

    Check out https://hackmd.io/dg7rizTyTXuCf2LSa2LsyQ for more details

commit a8a4804
Author: Andrew Davis <1709934+Savid@users.noreply.github.com>
Date:   Tue Jan 6 03:49:30 2026 +1100

    ethstats: report newPayload processing time to stats server (ethereum#33395)

    Add NewPayloadEvent to track engine API newPayload block processing
    times and report them to ethstats. This enables monitoring of block
    processing performance.

    https://notes.ethereum.org/@savid/block-observability

    related: ethereum#33231

    ---------

    Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de>

commit de5ea2f
Author: Mask Weller <Wellermask@gmail.com>
Date:   Sun Jan 4 13:47:28 2026 +0700

    core/rawdb: add trienode freezer support to InspectFreezerTable (ethereum#33515)

    Adds missing trienode freezer case to InspectFreezerTable, making it
    consistent with InspectFreezer which already supports it.

    Co-authored-by: m6xwzzz <maskk.weller@gmail.com>

commit b635e06
Author: rjl493456442 <garyrong0905@gmail.com>
Date:   Thu Jan 1 02:52:25 2026 +0800

    eth/fetcher: improve the condition to stall peer in tx fetcher (ethereum#32725)

    Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
    Co-authored-by: Csaba Kiraly <csaba.kiraly@gmail.com>

commit 32fea00
Author: shhhh <g1siddharthr@gmail.com>
Date:   Wed Dec 31 11:32:44 2025 +0530

    core/blockchain.go: cleanup finalized block on rewind in setHeadBeyondRoot (ethereum#33486)

    Fix ethereum#33390

    `setHeadBeyondRoot` was failing to invalidate finalized blocks because
    it compared against the original head instead of the rewound root. This
    fix updates the comparison to use the post-rewind block number,
    preventing the node from reporting a finalized block that no longer
    exists. Also added relevant test cases for it.

commit b2843a1
Author: Marco Munizaga <marco@marcopolo.io>
Date:   Tue Dec 30 17:48:50 2025 -0800

    eth/catalyst: implement getBlobsV3 (ethereum#33404)

    This is used by cell-level dissemination (aka partial messages) to give
    the CL all blobs the EL knows about and let CL communicate efficiently
    about any other missing blobs. In other words, partial responses from
    the EL is useful now.

    See the related (closed) PR:
    ethereum/execution-apis#674 and the new PR:
    ethereum/execution-apis#719

commit 25439aa
Author: Bashmunta <georgebashmunta@gmail.com>
Date:   Wed Dec 31 03:40:43 2025 +0200

    core/state/snapshot: fix storageList memory accounting (ethereum#33505)

commit 52ae75a
Author: Rim Dinov <rdin35051@gmail.com>
Date:   Wed Dec 31 01:04:38 2025 +0500

    cmd/geth: remove deprecated vulnerability check command (ethereum#33498)

    This PR removes the version-check command and its associated logic as
    discussed in issue ethereum#31222.

    Removed versionCheckCommand from misccmd.go and main.go.

    Deleted version_check.go and its corresponding tests.

    Cleaned up testdata/vcheck directory (~800 lines of JSON/signatures
    removed).

    Verified build with make geth

commit d9aaab1
Author: Fibonacci747 <albertofibonacci12@gmail.com>
Date:   Tue Dec 30 18:27:11 2025 +0100

    beacon/light/sync: clear reqFinalityEpoch on server unregistration (ethereum#33483)

    HeadSync kept reqFinalityEpoch entries for servers after receiving
    EvUnregistered, while other per-server maps were cleared. This left
    stale request.Server keys reachable from HeadSync, which can lead to a
    slow memory leak in setups that dynamically register and unregister
    servers.

    The fix adds deletion of the reqFinalityEpoch entry in the
    EvUnregistered handler. This aligns HeadSync with the cleanup pattern
    used by other sync modules and keeps the finality request bookkeeping
    strictly limited to currently registered servers.

commit b3e7d9e
Author: rjl493456442 <garyrong0905@gmail.com>
Date:   Tue Dec 30 23:05:13 2025 +0800

    triedb/pathdb: optimize history indexing efficiency (ethereum#33303)

    This pull request optimizes history indexing by splitting a single large
    database
     batch into multiple smaller chunks.

    Originally, the indexer will resolve a batch of state histories and
    commit all
    corresponding index entries atomically together with the indexing
    marker.

    While indexing more state histories in a single batch improves
    efficiency, excessively
    large batches can cause significant memory issues.

    To mitigate this, the pull request splits the mega-batch into several
    smaller batches
    and flushes them independently during indexing. However, this introduces
    a potential
    inconsistency that some index entries may be flushed while the indexing
    marker is not,
    and an unclean shutdown may leave the database in a partially updated
    state.
    This can corrupt index data.

    To address this, head truncation is introduced. After a restart, any
    excessive index
    entries beyond the expected indexing marker are removed, ensuring the
    index remains
    consistent after an unclean shutdown.

commit b84097d
Author: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Date:   Tue Dec 30 14:43:45 2025 +0100

    .github/workflows: preventively close PRs that seem AI-generated (ethereum#33414)

    This is a new step in my crusade against the braindead fad of starting
    PR titles with a word that is completely redundant with github labels,
    thus wasting prime first-line real-estate for something that isn't
    necessary.

    I noticed that every single one of these PRs are low-quality AI-slop, so
    I think there is a strong case to be made for these PRs to be
    auto-closed. A message is added before closing the PR, redirecting to
    our contribution guidelines, so I expect quality first-time contributors
    to read them and reopen the PR. In the case of spam PRs, the author is
    unlikely to revisit a given PR, and so auto-closing might have a
    positive impact. That's an experiment worth trying, imo.

commit 3f641db
Author: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Date:   Tue Dec 30 13:44:04 2025 +0100

    trie, go.mod: remove all references to go-verkle and go-ipa (ethereum#33461)

    In order to reduce the amount of code that is embedded into the keeper
    binary, I am removing all the verkle code that uses go-verkle and
    go-ipa. This will be followed by further PRs that are more like stubs to
    replace code when the keeper build is detected.

    I'm keeping the binary tree of course. This means that you will still
    see `isVerkle` variables all over the codebase, but they will be renamed
    when code is touched (i.e. this is not an invitation for 30+ AI slop
    PRs).

    ---------

    Co-authored-by: Gary Rong <garyrong0905@gmail.com>

commit 57f8486
Author: Archkon <180910180+Archkon@users.noreply.github.com>
Date:   Mon Dec 29 20:57:29 2025 +0800

    params: fix wrong comment (ethereum#33503)

    It seems that the comment for CopyGas was wrongly associated to
    SloadGas.

commit b9702ed
Author: oooLowNeoNooo <ooolowneonooo@gmail.com>
Date:   Mon Dec 29 09:23:51 2025 +0100

    console/prompt: use PromptInput in PromptConfirm method (ethereum#33445)

commit 4531bfe
Author: rjl493456442 <garyrong0905@gmail.com>
Date:   Mon Dec 29 16:13:30 2025 +0800

    eth/downloader: fix stale beacon header deletion (ethereum#33481)

    In this PR, two things have been fixed:

    ---

    (a) truncate the stale beacon headers with latest snap block

    Originally, b.filled is used as the indicator for deleting stale beacon headers.
    This field is set only after synchronization has been scheduled, under the
    assumption that the skeleton chain is already linked to the local chain.

    However, the local chain can be mutated via `debug_setHead`, which may
    cause `b.filled` outdated. For instance, `b.filled` refers to the last head snap block
    in the last sync cycle while after `debug_setHead`, the head snap block has been
    rewounded to 1.

    As a result, Geth can enter an unintended loop: it repeatedly downloads
    the missing beacon headers for the skeleton chain and attempts to schedule the
    actual synchronization, but in the final step, all recently fetched headers are removed
    by `cleanStales` due to the stale `b.filled` value.

    This issue is addressed by always using the latest snap block as the indicator,
    without relying on any cached value. However, note that before the skeleton
    chain is linked to the local chain, the latest snap block will always be below
    skeleton.tail, and this condition should not be treated as an error.

    ---

    (b) merge the subchains once the skeleton chain links to local chain

    Once the skeleton chain links with local one, it will try to schedule the
    synchronization by fetching the missing blocks and import them then.
    It's possible the last subchain already overwrites the previous subchain and
    results in having two subchains leftover. As a result, an error log will printed
    https://github.com/ethereum/go-ethereum/blob/master/eth/downloader/skeleton.go#L1074
weiihann pushed a commit to weiihann/go-ethereum that referenced this pull request Jan 14, 2026
@Savid @MariusVanDerWijden @weiihann
ethstats: report newPayload processing time to stats server (ethereum…
3cc8230 
…#33395)

Add NewPayloadEvent to track engine API newPayload block processing
times and report them to ethstats. This enables monitoring of block
processing performance.

https://notes.ethereum.org/@savid/block-observability

related: ethereum#33231

---------

Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de>
weiihann pushed a commit to weiihann/go-ethereum that referenced this pull request Jan 16, 2026
@Savid @MariusVanDerWijden @weiihann
ethstats: report newPayload processing time to stats server (ethereum…
8f98c91 
…#33395)

Add NewPayloadEvent to track engine API newPayload block processing
times and report them to ethstats. This enables monitoring of block
processing performance.

https://notes.ethereum.org/@savid/block-observability

related: ethereum#33231

---------

Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fjl fjl fjl left review comments

@rjl493456442 rjl493456442 Awaiting requested review from rjl493456442 rjl493456442 is a code owner

+1 more reviewer

@Savid Savid Savid left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MariusVanDerWijden @fjl @Savid