electric-sql / electric-sql/electric
EtsInspector: prevent mailbox overload and avoid serving orphaned replies
- Dominant language
- TypeScript
- Stars
- 10.4k
- Forks
- 375
- Avg merge
- 3d 1h
- Merged PRs (30d)
- 18
Description
## Background
`Electric.Postgres.Inspector.EtsInspector` is the single GenServer that fronts Postgres relation/oid/column/feature lookups for every shape HTTP request that needs to construct a `%Shape{}` (i.e. every `validate_request` on the serve-shape path). On the warm-cache path it's pure ETS in the calling process, but the cold-cache and degraded-DB paths funnel every concurrent request through one mailbox.
This is a follow-up to the thundering-herd mitigation work tracked under #4266 — specifically the cheap admission control (#4292 / #4291 / PR #4359), which bounds how many shape requests can simultaneously sit in validate-and-load. That work makes the `:initial` bucket the right tier-0 protection for ShapeCache, but every admitted request still passes through EtsInspector during `validate_request`. Sizing `:initial` is only meaningful if the inspector's own behaviour under burst + DB-degraded conditions is bounded; this issue addresses that.
It rolls up two related weaknesses. They are partially independent; together they convert the inspector from "single serial work queue" to "single source of truth with bounded blast radius."
## Goals
**(a) Avoid mailbox overload on EtsInspector.** Under (1) a cold-start burst for a never-before-seen root table, or (2) PG pool exhaustion / DB unavailability, every concurrent shape request that misses ETS enqueues a `GenServer.call` and serializes through the inspector. Worst case: each handler reattempts `Postgrex.transaction` because failed lookups are not cached — meaning N concurrent requests for the same failing key produce N serial DB attempts. With `Postgrex`'s default 15s timeout, this stretches into minutes of work.
**(b) Stop spending work on orphaned waiters.** Read paths use `GenServer.call(_, _, :infinity)`. Upstream the request can have already timed out — HTTP request timeout, Plug timeout, client disconnect — but the inspector has no way to know that and continues to dequeue messages, run DB transactions, and produce replies that go nowhere. Under load this is pure deadweight, and it directly worsens (a) because every dead message still takes a turn ahead of live waiters.
## Concrete mitigations to consider
1. **Cache negative results with a short TTL.**
`fill_cache/2` only writes ETS on the `{:ok, {rel, cols}}` branch. `:table_not_found` and `{:error, :connection_not_available}` produce no ETS state, so the next request re-misses, re-queues, re-attempts. Even a 1–5s cache for these terminal results lets the mailbox drain instead of refilling at the same rate.
2. **Tighten the per-call DB timeout.**
`fetch_from_db/2` uses `Postgrex.transaction` with no explicit `timeout:`, inheriting the 15s default. Under PG pool exhaustion this is the wrong knob — a shape request that has likely already missed its HTTP budget is keeping the inspector busy. An explicit timeout closer to a few seconds would bound the per-failure cost.
3. **Explicit in-flight coalescing.**
Today, dedup of concurrent requests for the same key happens *only* by virtue of message N+1 finding the ETS row populated after message N's `fill_cache`. They still each take a turn in the mailbox. A small in-flight map keyed by `{:relation_to_oid, rel}` / `{:oid_info, oid}` / `:supported_features` would let the GenServer accept the second-Nth request, stash its `from`, return `{:noreply, …}`, and broadcast the result to all parked waiters when the single DB call completes. Mailbox depth becomes O(unique keys in flight) instead of O(requests in flight).
4. **Replace `GenServer.call(:infinity)` with a sleep loop in the request process.**
The inspector should not be asked to babysit orphan detection on every message — that's just more work multiplied by the same mailbox we're trying to keep small. Instead, the request process polls ETS for the key with a short sleep between attempts, and the inspector is only contacted once per unique missing key to *trigger* a fill (cast or coalesced call per (3)). If the request's own deadline (HTTP / admission budget) expires while polling, it gives up locally — the inspector never learns about the abandoned waiter because it was never sent a message for it. This naturally satisfies (b) without per-message checks, and it keeps the mailbox population proportional to fresh keys rather than to in-flight requests.
Contributor guide
Assessment
This issue has not been assessed yet.