Cache endpoint list so ws_endpoints never blocks the GenServer - #29
Merged
Merged
Conversation
ChainList.filter_endpoints/2 used to spawn a Task.async_stream over the chainlist and run the full health probe inline. The probe can take 30+ seconds when providers are slow, and it ran in whatever process called RemoteChain.ws_endpoints/1 — which is the NodeProxy GenServer. While blocked, the GenServer could not process :new_block messages, so lastblocks/1 went stale, every live WSConn was evicted as data-stale, and the eviction triggered another slow ws_endpoints call. On us1 Base the GenServer sat inside filter_endpoints for ~40 hours with 137k messages queued. This lands the fix planned in docs/specs/plan-endpoint-list-cache.md: * Stale-while-revalidate cache in front of filter_endpoints/2, keyed by chain_id, 60 s TTL. Warm callers return from ETS in microseconds. * Cold callers get best_effort_urls/1 (deduped + known-broken providers stripped) synchronously and schedule the probe in a detached Task. * Per-chain in-flight flag coalesces concurrent refreshes so 20 cold callers produce one probe, not 20. * Per-chain generation counter prevents zombie workers from resurrecting a cache that was explicitly cleared (chainlist refresh, tests). * ChainList.put_chains/2 invalidates the endpoint cache so newly-listed providers are picked up on the next call. * The cold path and the warm probe both apply the pocket.network / curie.radiumblock.co filter via a single rejected_provider?/1 helper, and probe_pass/2 collapses filter+map into one Enum.flat_map. Tests cover the regression, the warm/stale/empty paths, the debouncing, the chainlist invalidation, and the cold-path rejection of the two known-broken providers.
Closing a TOCTOU window where a slow probe followed by a clear_chain_cache bump could let a stale write land. The gen check now guards both the start of the refresh (existing) and the Globals.put itself (new), so a worker outliving its cache cannot resurrect the cache even if the generation was bumped mid-probe. Adds a regression test that schedules a refresh, invalidates the endpoint cache mid-flight, and asserts the worker skipped its write.
Per project requirement, Debouncer.immediate/3 must be used to coalesce refreshes. The Debouncer spawns a closure that calls Task.start to do the actual probe work, so the Debouncer's cast handling completes quickly and the spawned Task does not contend with the GenServer mailbox. timeout: 0 makes the cooldown window effectively instant: the timer fires immediately, clearing the events entry, so the next sequential caller (after the cooldown) hits the empty-events branch and gets a fresh worker too. The outer closure snapshots the generation counter; the spawned Task re-checks it before doing any work, so a clear_chain_cache bump between scheduling and the Task running causes the Task to no-op (refresh_endpoint_cache/3 also re-checks right before the write).
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
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.
Root cause
ChainList.filter_endpoints/2spawned aTask.async_streamover the chainlist and ran the full health probe inline. The probe can take 30+ seconds when providers are slow, and it ran in whatever process calledRemoteChain.ws_endpoints/1— which is theNodeProxyGenServer. While blocked, the GenServer could not process:new_blockmessages, solastblocks/1went stale, every live WSConn was evicted as data-stale, and the eviction triggered another slowws_endpointscall. On us1 Base the GenServer sat insidefilter_endpointsfor ~40 hours with 137k messages queued. Full diagnosis:docs/specs/plan-endpoint-list-cache.md.Fix
Stale-while-revalidate cache in front of
filter_endpoints/2(60s TTL, keyed bychain_id). Warm callers return from ETS in microseconds; cold callers get a deduped best-effort list synchronously and schedule the probe in a detachedTask.Debouncer.immediate/3(withtimeout: 0) coalesces concurrent refreshes so 20 cold callers produce one probe. Per-chain generation counter prevents zombie workers from resurrecting a cache that was explicitly cleared. Chainlist refresh (put_chains/2) invalidates the cache so newly-listed providers are picked up on the next call.The cold path and the warm probe both apply the
pocket.network/curie.radiumblock.cofilter via a singlerejected_provider?/1helper;probe_pass/2collapsesfilter+mapinto oneEnum.flat_map.best_effort_urls/1is reused by both paths.Why
Debouncer.immediate/3The plan called for
Debouncer.immediate2/3. In practice the closure runs synchronously inside the Debouncer's cast handling, which serialized the actual probe behind any other cast the GenServer was processing — enough to break the 2-second timeout in the "cold-cache callers trigger exactly one probe per chain" test. The implementation now wrapsDebouncer.immediate/3aroundTask.start: the Debouncer's closure only spawns the detached Task, so the cast handling returns immediately and the probe runs without contending with the Debouncer's mailbox.timeout: 0makes the cooldown effectively instant: the timer fires immediately, clears the events entry, and the next sequential caller (after the timer) hits the empty-events branch and gets a fresh worker.TOCTOU fix
The first commit had a TOCTOU window: the generation check ran once at the top of the closure, so a slow probe followed by a
clear_chain_cachebump could let a stale write land. The second commit moved the generation check to right before theGlobals.putinrefresh_endpoint_cache/3, so a worker outliving its cache cannot resurrect the cache even if the generation was bumped mid-probe.Out of scope (per plan)
Bug B —
data_stale?/4preferringlastblocks[url]overWSConn.lastblock_at/1— is left as a follow-up. With the GenServer no longer blocked, the cache tracks real block arrivals and Bug B stops being reachable in practice.Tests
Debouncer.immediate/3)refresh_chains/1invalidates the endpoint cache for affected chainspocket.network/curie.radiumblock.coare dropped on the cold pathclear_chain_cachetime does not resurrect the cache (TOCTOU regression)mix test test/remote_chain/→ 105/105 pass.mix lint→ clean.