perf(metrics): prevent multi-second engine stalls from scrape hooks#189
Merged
Conversation
The two custom hooks registered by `metrics_hooks` walk MDBX page metadata + the freelist DB and iterate every static-file jar header on every Prometheus scrape. On large databases this is expensive enough to stall the metrics endpoint and starve the runtime. Skip registering both hooks when `RETH_DISABLE_HEAVY_METRICS` is set in the environment; the rest of the registry (process, jemalloc, io, chain spec, version) is unaffected and the endpoint still responds normally. The env var is documented on the function so the escape hatch is discoverable without grepping the source.
The Prometheus metrics handler is fundamentally synchronous: it invokes every registered hook and then runs the prometheus exporter's `render()`, all on the tokio worker that accepted the HTTP request (or on the runtime worker driving the push-gateway loop). The default hooks are cheap (procfs, jemalloc atomic reads), but the two `report_metrics` hooks (DB stat walk, static-file jar enumeration) can take seconds on large archives. Even with those gated out (see preceding patch), `render()` itself is O(total time-series) and will grow over time. A multi-millisecond synchronous block on a runtime worker is not ideal and can become a real engine latency source if hook cost ever regresses. Move the synchronous work off the runtime worker: - Endpoint handler now offloads `handle_request` (which calls the hook + render or the pprof dump) to `spawn_blocking`. On join error, return a 500 instead of letting the connection task panic. - Push-gateway loop offloads the hook + render to `spawn_blocking`; on join error, log a warning and skip this tick rather than killing the loop. The HTTP put itself was already async so it stays inline. No behavioral change to what the endpoint or push-gateway returns; only the thread on which the rendering happens.
Pull Request ReviewThis PR optimizes the metrics pipeline by preventing expensive metric hooks from stalling Tokio runtime workers. It introduces an environment-variable gate ( Sensitive ContentNo sensitive content detected. Security IssuesNo serious security issues detected. Generated by Hashdit Bot. This tool can absolutely NOT replace manual audits. |
zhk101
changed the title
will-2012
approved these changes
May 15, 2026
constwz
approved these changes
May 15, 2026
zhk101
changed the title
zhk101
changed the title
joey0612
approved these changes
May 18, 2026
chee-chyuan
pushed a commit
that referenced
this pull request
May 21, 2026
…189) * metrics: gate heavy scrape hooks behind RETH_DISABLE_HEAVY_METRICS The two custom hooks registered by `metrics_hooks` walk MDBX page metadata + the freelist DB and iterate every static-file jar header on every Prometheus scrape. On large databases this is expensive enough to stall the metrics endpoint and starve the runtime. Skip registering both hooks when `RETH_DISABLE_HEAVY_METRICS` is set in the environment; the rest of the registry (process, jemalloc, io, chain spec, version) is unaffected and the endpoint still responds normally. The env var is documented on the function so the escape hatch is discoverable without grepping the source. * metrics: run scrape handler and push-gateway render on spawn_blocking The Prometheus metrics handler is fundamentally synchronous: it invokes every registered hook and then runs the prometheus exporter's `render()`, all on the tokio worker that accepted the HTTP request (or on the runtime worker driving the push-gateway loop). The default hooks are cheap (procfs, jemalloc atomic reads), but the two `report_metrics` hooks (DB stat walk, static-file jar enumeration) can take seconds on large archives. Even with those gated out (see preceding patch), `render()` itself is O(total time-series) and will grow over time. A multi-millisecond synchronous block on a runtime worker is not ideal and can become a real engine latency source if hook cost ever regresses. Move the synchronous work off the runtime worker: - Endpoint handler now offloads `handle_request` (which calls the hook + render or the pprof dump) to `spawn_blocking`. On join error, return a 500 instead of letting the connection task panic. - Push-gateway loop offloads the hook + render to `spawn_blocking`; on join error, log a warning and skip this tick rather than killing the loop. The HTTP put itself was already async so it stays inline. No behavioral change to what the endpoint or push-gateway returns; only the thread on which the rendering happens.
chee-chyuan
pushed a commit
that referenced
this pull request
May 26, 2026
…189) * metrics: gate heavy scrape hooks behind RETH_DISABLE_HEAVY_METRICS The two custom hooks registered by `metrics_hooks` walk MDBX page metadata + the freelist DB and iterate every static-file jar header on every Prometheus scrape. On large databases this is expensive enough to stall the metrics endpoint and starve the runtime. Skip registering both hooks when `RETH_DISABLE_HEAVY_METRICS` is set in the environment; the rest of the registry (process, jemalloc, io, chain spec, version) is unaffected and the endpoint still responds normally. The env var is documented on the function so the escape hatch is discoverable without grepping the source. * metrics: run scrape handler and push-gateway render on spawn_blocking The Prometheus metrics handler is fundamentally synchronous: it invokes every registered hook and then runs the prometheus exporter's `render()`, all on the tokio worker that accepted the HTTP request (or on the runtime worker driving the push-gateway loop). The default hooks are cheap (procfs, jemalloc atomic reads), but the two `report_metrics` hooks (DB stat walk, static-file jar enumeration) can take seconds on large archives. Even with those gated out (see preceding patch), `render()` itself is O(total time-series) and will grow over time. A multi-millisecond synchronous block on a runtime worker is not ideal and can become a real engine latency source if hook cost ever regresses. Move the synchronous work off the runtime worker: - Endpoint handler now offloads `handle_request` (which calls the hook + render or the pprof dump) to `spawn_blocking`. On join error, return a 500 instead of letting the connection task panic. - Push-gateway loop offloads the hook + render to `spawn_blocking`; on join error, log a warning and skip this tick rather than killing the loop. The HTTP put itself was already async so it stays inline. No behavioral change to what the endpoint or push-gateway returns; only the thread on which the rendering happens.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses bnb-chain/reth-bsc#353 (the engine-tree handler is slow stalls observed on BSC full nodes have their root cause here in the metrics scrape path).
Description
The Prometheus metrics endpoint and push-gateway loop currently run the full hook chain + exporter
render()synchronously on a tokio worker. On large databases the two heavy hooks (Database::report_metricsandStaticFileProvider::report_metrics) walk MDBX page metadata, the freelist DB, and every static-file jar header on every scrape, which can take seconds and stall the runtime. In practice this surfaces as multi-second engine-tree handler is slow events on a cadence matching the hook throttle, observed on a BSC full node (bnb-chain/reth-bsc#353).This PR makes two independent changes:
RETH_DISABLE_HEAVY_METRICSenv-var escape hatch so operators can drop those two hooks without rebuilding.render()+ hooks) tospawn_blockingfor both the HTTP endpoint and the push-gateway loop, so even when the hooks are enabled they no longer block the tokio worker that accepted the request.Rationale
The heavy hooks above already produce real multi-second engine stalls today; the env-var escape hatch makes those stop without losing the rest of the metrics. Beyond that, even after gating the two heavy hooks out,
render()itself is O(total time-series) and grows over the lifetime of the node. A multi-millisecond synchronous block on a tokio worker is enough to add tail latency to anything else scheduled on that worker (engine API, RPC, networking). Moving the rendering to the blocking pool makes the metrics path a non-actor for runtime jitter regardless of how the registry grows.Example
Opt out of heavy DB / static-file hooks at startup:
The metrics endpoint still serves the rest of the registry (process, jemalloc, io, chain spec, version). Default behavior (no env var set) is unchanged.
Changes
Notable changes:
crates/node/builder/src/launch/common.rs:metrics_hooksskips registering the DB and static-filereport_metricshooks whenRETH_DISABLE_HEAVY_METRICSis set. Doc comment on the function documents the escape hatch.crates/node/metrics/src/server.rs: HTTP request handler offloadshandle_request(hooks + render or pprof dump) ontospawn_blocking; returns 500 on join error.crates/node/metrics/src/server.rs: Push-gateway loop offloads the per-tick hooks + render ontospawn_blocking; on join error logs a warning and skips the tick rather than killing the loop. HTTP put remains async.Potential Impacts
spawn_blockinghop per request. Negligible compared to the synchronousrender()cost it removes.RETH_DISABLE_HEAVY_METRICS: new env var. Operators who haven't set it see identical behavior.Related: bnb-chain/reth-bsc#353