fix(block-proxy): dedup deadline - #1608
Merged
Merged
Conversation
A dedup entry was removed only when its promise settled, so an upstream promise that never settled pinned its block height indefinitely. Every later request for that height attached to the dead promise instead of retrying, and because indexers resume from the same persisted height after a crash, one poisoned height stalled every consumer until the process was restarted by hand. - withDeadline() guarantees the stored promise settles within DEDUP_TTL_SECS (default 25, validated 5..30 against the 30s client abort), so the entry is always released and the next request leads a fresh attempt. - Entries carry createdAt and are evicted on read when stale, so a reader is never served an entry that outlived its TTL even if the deadline timer failed to fire. - The .finally delete checks entry identity, so a slow leader settling after eviction cannot delete its successor. - The MAX_DEDUP_SIZE bypass is bounded too, and counts as a leader so dedup.saves no longer reports bypassed fetches as deduplications. - UPSTREAM_TIMEOUT_SECS 30 -> 10. It previously equalled the client abort, so indexers gave up at the instant the proxy would have answered and the fallback chain was invisible to them. - Bound cache reads, cancel response bodies before throwing (undici holds the socket until the body is read or cancelled), and fail fast when no working upstream is configured. - Add /livez, which reports a stall when requests arrive and none complete, plus block_proxy_dedup_saves, block_proxy_dedup_deadlines and block_proxy_last_served_timestamp. - Remove the unused NEAR Lake upstream, including a path that reported a block as skipped when it was merely absent from the bucket.
indexer-base and indexer-receipts called startMetricsServer(3010) twice, from libs/prom.ts and again from libs/http.ts, so every boot logged EADDRINUSE. The first listen won, so metrics were still exported, but the server exported from libs/http.ts and passed to server.close() on shutdown was the failed one, and the port was never released. Export the server from libs/prom.ts as the other nine indexers already do, and drop the redundant libs/http.ts.
turbo had no test task, so the suites in the repo never ran anywhere. Add one, wire it into CI, and fix nb-neardata's test script, which declared vitest with no test files: it exited 1 in CI and hung in watch mode locally. packages/nb-lake had no src, no dependents, and had not been touched since January.
Four scattered guards, a worstCaseChainMs helper and two startup warnings collapsed into one block. The warnings are replaced by the hard guard that subsumes them, and the cache read timeout is now a single exported constant instead of the same number in two files.
A review proved several additions inert or harmful: - The config guard coupling DEDUP_TTL_SECS to the upstream chain was unsatisfiable: S3 on with a 15s upstream timeout required a TTL above 32s and at most 30s, so the proxy refused to boot on a legal config. The documented 5-30 range was also wrong for every configuration. - /livez could never fire. The deadline guarantees a written 502, so res 'finish' always fired and lastServedAt always tracked lastRequestAt. It reported healthy while every request failed. - The stale-entry eviction and its .finally identity check were unreachable: timers run before the poll phase, so a due deadline always fires before the next request handler. - block_proxy_dedup_saves is identically dedup_requests minus dedup_leaders, which stats.ts already derives. - The cache AbortSignal does not interrupt a blocking open and does not free the libuv worker, and a hard cap under load turns cache hits into misses. - discardBody on the 404 paths is a measured no-op; undici reclaims a small unread body in the same tick. Inline withDeadline at its one call site and reject before the side effects, which removes the try/catch, its module and its test file.
- A leader created while the dedup map was at capacity is never stored, but still carried the .finally cleanup, so on settling it deleted the key belonging to a later leader and singleflight collapsed for that height. Only the stored promise carries the cleanup now. - Rejecting before the deadline's side effects settled the promise but did not contain a throw, which escaped the timer callback as an uncaught exception and exits the process. Guard the side effects. Both now have tests that fail without the fix, along with tests for the capacity cap and the error payload that drives 502 logging.
Every filesystem call shares one libuv worker pool, four wide by default, and none can be cancelled. A volume that stops responding parks a worker per call, permanently. Reads, background writes and the eviction sweep all draw on that pool, so a hung mount drains it within seconds and stops all file and DNS work process-wide, taking the proxy down entirely rather than just the cache. Background writes are the heaviest draw at three calls per block, and nothing observed them. Count operations that overrun and, after three, stop touching the disk for the life of the process: the proxy keeps serving from upstream, slower but serving. One-way on purpose, since re-probing a hung mount costs another worker that never returns. Exposed as block_proxy_cache_disabled and block_proxy_cache_timeouts. Also guard the eviction sweep against overlapping itself, so a stalled sweep costs one worker rather than one per minute, and raise UV_THREADPOOL_SIZE so three tolerated stalls leave margin for the DNS lookups outbound fetches need.
Upstream is billed per query and the cache carries most of the load during catch-up, so leaving it off until the next deploy over-corrects for a transient stall. Suspension is now temporary: after a backoff starting at 60s and doubling to 15m, one read is let through to see whether the volume recovered, and a successful one resumes writes and eviction too. Reads carry the probe because they are the cheapest call. Each probe that stalls costs a worker for good, so the total is capped at four, after which only a restart brings the cache back. Suspension and resumption both log, the reason names the cost impact, and the state is exposed as block_proxy_cache_disabled and in the /stats cache.disabled field.
It cannot do the job it was written for, and it introduces a cost regression on healthy hardware. The pool is eight workers wide and a block costs four filesystem calls (one read, three for the write). At catch-up rates that is roughly a thousand calls a second, so a hung volume parks every worker in about eight milliseconds. The detector needs a five second timeout to fire. Even at tip rate the pool is gone in two seconds. A latency-based detector can never outrun pool drain; it always arrives after the process is already dead. Worse, the timeout measures queueing as well as service time. A thousand offered operations a second against eight workers at ten milliseconds each is past capacity, the queue grows without bound, and every operation eventually crosses five seconds. So on a healthy network volume during catch-up the cache disables itself permanently and turns cache hits into billed upstream queries. Recovery never worked either: in-flight operations at the moment of suspension all count against the probe budget, so a real hang grants zero probes and the backoff ladder is dead code. Bounding parked workers needs a limit on concurrent filesystem calls, not a timer. That is a different design and will come separately.
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
1 participant
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.
No description provided.