Skip to content

fix(debug-trace-server): stop frontier witness misses from surfacing as timeouts - #179

Closed
flyq wants to merge 1 commit into
liquan/feat/request-accounting-and-error-reasonsfrom
liquan/fix/frontier-witness-never-times-out
Closed

fix(debug-trace-server): stop frontier witness misses from surfacing as timeouts#179
flyq wants to merge 1 commit into
liquan/feat/request-accounting-and-error-reasonsfrom
liquan/fix/frontier-witness-never-times-out

Conversation

@flyq

@flyq flyq commented Aug 9, 2026

Copy link
Copy Markdown
Member

Summary

Stacked on #178. Gives the generator a bounded exclusive grace for frontier blocks on the request path, and reports a still-missing frontier witness as -32002 not generated yet instead of a deadline.

Root cause

Two client-visible -32001s on ore, both frontier blocks whose witness was not generated yet:

Witness fetch deadline exceeded block_number=23461272
  source="witness_generator" old_block=false budget_ms=7999 elapsed_ms=8001

The budget bought roughly four provider rotations, all structurally useless: the fallback endpoints are fed by the same generation pipeline and cannot be ahead of the generator (chain_sync.rs:27), so each rotation spent a public-gateway round trip to learn what the generator had already said. Chain sync has had an exclusive generator grace since #164 (chain_sync.rs:119); the request path (data_provider.rs:1160) never got one — that call site is the only one in the tree that does not use the grace probe.

Fix

  • Frontier blocks (above the local DB tip, generator first, ≥2 providers) get an exclusive generator probe under a caller-side timeout before any rotation, clamped to half the remaining witness budget so an unavailable generator still leaves the fallback chain a real budget instead of a nearly-expired one.
  • One deliberate difference from chain sync's twin: that constant also serves as the trust horizon for a head observation that goes stale whenever the fetcher has work queued, which silently disables the routing. The request path's freshness signal is the local DB tip, read fresh from redb per fetch, so this grace carries one meaning only and cannot be switched off by a busy window.
  • DataProviderError::WitnessNotReady renders as -32002 witness for block N is not generated yet; retry shortly, so a client can tell a retryable chain-lag apart from a nonexistent block — today both leave as -32001 — and reason="deadline_witness" goes back to meaning only what it says. Parity trace_transaction degrades it to null like its siblings.
  • Outcome observable via debug_trace_frontier_grace_total{outcome="served"|"expired"}.

Testing

cargo test --workspace green; fmt / clippy --all-targets --all-features / cargo sort clean. The new variant is covered by error_reason_separates_deadlines_from_not_found (distinct code, distinct reason, retry-shaped message).

Notes

This converts the failure into a typed, actionable one; it does not make the request wait until it succeeds. Unbounded waiting needs the response-size cap first, or it reintroduces the OOM shape from the 2026-08-07 tko incident.

Worth adding in review: an integration test driving the grace-expiry fall-through. test_support::scripted_witness_rpc can withhold the witness for N attempts, which is the shape needed. This is the riskiest part of the change (production witness routing) and currently only has compile-time coverage.

…as timeouts

Two client-visible -32001s on ore, both frontier blocks whose witness was
not generated yet:

    Witness fetch deadline exceeded block_number=23461272
      source="witness_generator" old_block=false budget_ms=7999 elapsed_ms=8001

The 8s bought roughly four provider rotations, all structurally useless:
the fallback endpoints are fed by the same generation pipeline and cannot
be ahead of the generator, so each rotation spent a public-gateway round
trip to learn what the generator had already said. Chain sync has had an
exclusive generator grace since #164; the request path never got one.

Adds it, with one deliberate difference from chain sync's twin: that one
also serves as the trust horizon for a head observation that goes stale
whenever the fetcher has work queued, which silently disables the routing.
The request path's freshness signal is the local DB tip, read fresh from
redb on every fetch, so this grace carries one meaning only and cannot be
switched off by a busy window. The grace is clamped to half the remaining
witness budget so an unavailable generator still leaves the fallback chain
a real budget rather than a nearly-expired one.

A frontier witness that still never arrives is no longer reported as a
deadline. `DataProviderError::WitnessNotReady` renders as -32002 "witness
for block N is not generated yet; retry shortly", so a client can tell a
retryable chain-lag apart from a nonexistent block — today both leave as
-32001 — and `reason="deadline_witness"` goes back to meaning only what it
says. Parity trace_transaction degrades it to null like its siblings.

This converts the failure into a typed, actionable one; it does not make
the request wait indefinitely. Unbounded waiting needs the response-size
cap first, or it reintroduces the OOM shape.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mega-maxwell

mega-maxwell Bot commented Aug 9, 2026

Copy link
Copy Markdown

Claude review status

Living comment — rewritten in place. The review workflow keeps this single comment up to date instead of posting a new one each round, so it always describes the latest reviewed commit and the earlier text is intentionally gone. No reply is needed here; reply to a finding in its own review thread, and answer an open question in a reply on this PR. The next review round reconciles your answer.

🛠️ Review did not finish

Attempted head 58502033 · updated 2026-08-09T09:36:19+00:00

This round did not publish: MODEL_ACTION_FAILED in phase review_retry. Anything listed below is from the last round that did. Re-run the workflow or push a new commit to try again.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5850203345

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +1234 to +1235
if frontier {
return Err(DataProviderError::WitnessNotReady { block_number });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve real frontier witness deadlines

When frontier is true, this branch converts every Err from the full witness provider chain into WitnessNotReady, but get_witness_light_with_deadline_from also returns RpcDeadlineExceeded after repeated provider stalls, transport errors, or decode failures until the deadline expires. In a local-cache deployment with generator+fallback endpoints, a gateway outage or overload for an above-tip block will now be reported to clients as -32002 ... not generated yet and counted as witness_not_ready instead of preserving the real witness deadline signal, masking the incident the separate reason labels are meant to expose.

AGENTS.md reference: AGENTS.md:L127-L127

Useful? React with 👍 / 👎.

@flyq
flyq marked this pull request as draft August 9, 2026 09:48
@flyq

flyq commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

Converting to draft: the premise this PR is built on does not hold.

Log forensics on ore (2026-08-09, 3.01h) shows the request-path gateway hop is trimodal, not uniformly useless:

outcome count duration
fast not found 10,543 p50 150ms, p99 279ms, max 828ms
served the witness 13 1.2–3.4s
stalled until the budget expired 2 7.90 / 7.91s → client -32001

So "the fallbacks cannot be ahead of the generator" is false. The generator's miss is a file lookup (Neither ".../backup/witness/22903/….w" nor ".../witness/….w" exists) while the gateway reads the R2 bucket — different stores, and R2 can hold a frontier witness the local files do not. Nine of those 13 are confirmed by a fetch_witness_ms slow-stage warning, which only fires on the success path. Blocks 23461274 / 23461279 / 23461280, immediately adjacent to the two failures in the same storm, were all served by the gateway.

This PR's grace would therefore delay 13 successful fetches by up to the grace to rescue 2, pushing the ones near the budget edge into new failures.

What the evidence does support: attempt_timeout = min(deadline - now, per_attempt_timeout) with a witness budget of 8–12s and per_attempt_timeout defaulting to 20s means the per-attempt guard never binds on the witness path, so one stalled hop can legally consume the entire remaining budget and then return via record_deadline — no log, and rpc_client.rs:1069-1074 deliberately suppresses the per-provider outcome.

What it does not yet establish: whether those 7.9s were spent inside the gateway call or waiting on the witness semaphore. Both are silent and produce identical observations. Instrumentation first; the fix follows the data.

@flyq

flyq commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

Closing rather than iterating: the premise is inverted, so there is nothing here to salvage.

Reading the generator (mega-reth, bin/stateless/witness) settles it:

  • The generator's RPC server answers from two bare exists() syscalls with no cache in between (block_updates/file_ops.rs:466-478), so its "not found" is a live, truthful statement about FileType::Witness on disk.
  • The R2 uploader reads a different file typeFileType::Upload (upload.rs:319-320; the module doc says it "scans the local upload directory") — with its own write timing.

So for a frontier block, R2 can legitimately hold the witness while the RPC server's witness/ file does not exist yet. That is structural, not a race we can tune around, and it matches the observed 13 gateway successes (nine confirmed by a fetch_witness_ms slow-stage warning, which only fires on the success path).

This PR gave the generator an exclusive grace before rotating — i.e. it prioritises the source that is structurally later. Wrong direction.

Two follow-ups instead:

  1. The clean fix is on the generator side: have the RPC server fall back to the Upload file when the Witness file is missing. Same host, same data, already on disk — the miss window disappears and no DTS change is needed.
  2. What remains unproven on our side is whether the two client-visible timeouts were spent inside the gateway call or waiting on the witness semaphore. Both are silent and produce identical observations. Instrumentation-only PR follows; the fix waits for its data.

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.

1 participant