Skip to content

ethmonitor: detect stalled newHeads streams and fall back to polling - #226

Open
ScreamingHawk wants to merge 1 commit into
masterfrom
ethmonitor-stream-health-check
Open

ScreamingHawk wants to merge 1 commit into
masterfrom
ethmonitor-stream-health-check

Conversation

@ScreamingHawk

Copy link
Copy Markdown
Contributor

Summary

A newHeads subscription can silently serve stale heads: no error arrives on the subscription, while the node's HTTP api keeps advancing. This is what left the node-gateway's Base head 23 minutes behind during INC-280 while every endpoint health probe stayed green.

In streaming mode, the listener now tracks the timestamp of the last received head and drops the subscription, counting it toward the existing switch to polling, when it is older than StreamHealthCheckStaleness (default 60s). Seeded on connect so a stream that never delivers a head is caught too. NewMonitor rejects StreamHealthCheckStaleness <= StreamHealthCheckInterval.

Implements the streamHealthCheck TODO that was already in listenNewHead.

Test Plan

  • TestMonitorStreamHealthCheckFallsBackToPolling: stalled stream switches to polling, head keeps advancing, alert raised
  • TestMonitorStreamHealthCheckFreshStreamKeepsStreaming: control, no stall alerts on a live stream
  • TestMonitorStreamHealthCheckRecoversToStreaming: after the stall clears, polling retries streaming and catches back up
  • full ./ethmonitor/ suite passes

@ScreamingHawk
ScreamingHawk force-pushed the ethmonitor-stream-health-check branch from ddfd377 to f89c4b1 Compare November 5, 2026 21:19
@ScreamingHawk
ScreamingHawk marked this pull request as ready for review November 5, 2026 21:32
@ScreamingHawk
ScreamingHawk requested a review from a team November 5, 2026 21:32
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-05T21:33:48.580777Z f89c4b1 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@taylanpince

Copy link
Copy Markdown
Contributor

Findings from Ada's review of f89c4b1:

  • Arbitrum Nova would repeatedly reconnect on a healthy, idle chain. Nova produces blocks only when there is activity. In the sampled public RPC history, 18 of the last 59 block gaps exceeded 2 minutes; the longest was 3412s. The gateway head matched the public head, and Nova had streamingMode: true. With the 60s threshold and the timer seeded on connect, an idle Nova stream would be dropped and alert roughly every 70s. Three drops switch it to polling, with streaming retried after 4 minutes, so the cycle repeats while the chain is quiet.
  • The check is enabled by default. DefaultOptions sets StreamHealthCheckInterval: 10s and StreamHealthCheckStaleness: 60s (ethmonitor/ethmonitor.go:35-36). node-gateway starts from ethmonitor.DefaultOptions (netmon/monitor.go:119) and does not override these fields, so an ethkit upgrade enables it automatically. Callers constructing Options from scratch get it disabled. Other consumers such as relayer and indexer still need checking.
  • An existing counter-reset bug compounds this. streamingErrLastTime is set just before goto reconnect, so the elapsed-time check is effectively always zero. StreamingErrorResetInterval never resets the counter, and drops accumulate even when far apart.

Suggested fix: when the stream appears stale, fetch the node's latest block over HTTP and only drop the subscription if the node is ahead of the stream. This distinguishes an idle chain from a stalled subscription and follows the deleted TODO.

The incident attribution also needs revisiting. In the 19:30–21:10 UTC window on Nov 5, the reviewed node-gateway logs showed no Base (8453) stream errors; the only ethmonitor stream errors were context canceled during shutdown on other chains. The incident evidence points to the monitor failing to keep up, with prefetch resolving it. If fresh heads continue arriving while processing lags, this check would correctly remain quiet, but would not catch that failure. The monitor-head check in node-gateway#334 addresses that case.

Before node-gateway adopts the release containing this change, either add the HTTP-head comparison or explicitly disable the check with StreamHealthCheckInterval: 0 / configure suitable per-network staleness.

Validation reported by Ada: go test -race ./ethmonitor/ passed, including the five new StreamHealthCheck tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants