Critique a design doc from the team's backlog

The round. The interviewer shares a real (lightly edited) design doc from the team's backlog and asks: what do you think? There is no single answer. They watch how you read: whether you find what matters most (not the most comments), separate blocking issues from nits, propose concrete alternatives, and say what is missing before this could ship. It is also a collaboration signal: would this person make a design review better or just louder?

A good 45 minutes:

1. 5 min: restate the goal and constraints in your own words; confirm them.
2. 10 min: check the doc's numbers and its main flow end to end.
3. 15 min: risks, ranked, each with a proposed fix.
4. 10 min: what is missing (rollout, failure handling, observability, cost, security).
5. 5 min: your recommendation: approve, approve with changes, or rethink, and the two or three things that decide it.

The sample doc used here (our own example, typical of what is handed over):

Usage metering and rate limits for the public API. Goal: bill customers per token and enforce per-organization limits. Design: each API server, after a request completes, runs INCRBY usage:{org}:{minute} tokens on a shared Redis primary (with one replica) and checks the same key against the org's per-minute limit before starting a request. Keys expire after 2 hours. An hourly billing job reads every usage:* key from the last hour with SCAN, sums by org, and writes invoice lines to Postgres. Scale: 50k requests/s today. Rollout: ship behind a flag next sprint.

Before any critique, write down what the doc must achieve, because every risk is measured against it:

• Billing must be exact. Money: no lost usage, no double counting, auditable later.
• Rate limits may be approximate. Being 2% over a limit for a few seconds is fine; being slow is not (it sits on every request).
• Those two needs are coupled into one mechanism here. That observation drives most of the review.

Then trace one request: request arrives → limit check (Redis GET) → model runs (seconds) → INCRBY with the token count → hourly job sums → invoice line. Ask at each arrow: what if this step fails, runs twice, or runs late?

As proposed

{"type":"excalidraw","elements":[
 {"type":"rectangle","id":"api","x":0,"y":0,"width":180,"height":70,"label":{"text":"API servers"}},
 {"type":"ellipse","id":"redis","x":340,"y":-5,"width":200,"height":80,"label":{"text":"Redis primary"}},
 {"type":"rectangle","id":"job","x":700,"y":0,"width":180,"height":70,"label":{"text":"Hourly job"}},
 {"type":"ellipse","id":"pg","x":1040,"y":-5,"width":200,"height":80,"label":{"text":"Invoices\n(Postgres)"}},
 {"type":"arrow","id":"a1","x":180,"y":25,"width":160,"height":0,"start":{"id":"api"},"end":{"id":"redis"},"label":{"text":"INCRBY"}},
 {"type":"arrow","id":"a2","x":340,"y":55,"width":-160,"height":0,"start":{"id":"redis"},"end":{"id":"api"},"strokeStyle":"dashed"},
 {"type":"arrow","id":"a3","x":700,"y":35,"width":-160,"height":0,"start":{"id":"job"},"end":{"id":"redis"},"label":{"text":"SCAN"}},
 {"type":"arrow","id":"a4","x":880,"y":35,"width":160,"height":0,"start":{"id":"job"},"end":{"id":"pg"},"label":{"text":"sums"}},
 {"type":"text","id":"legend","x":0,"y":120,"text":"Dashed: the limit check reads the same key before each request.","fontSize":16}
]}

With the proposed changes

Split the two jobs. Billing comes from a durable, replayable record of every request; limits stay fast and approximate in memory and Redis.

{"type":"excalidraw","elements":[
 {"type":"text","id":"legend","x":0,"y":-235,"text":"Shaded: what the review adds","fontSize":16},
 {"type":"rectangle","id":"api","x":0,"y":0,"width":180,"height":70,"label":{"text":"API servers"}},
 {"type":"ellipse","id":"redis","x":340,"y":-175,"width":200,"height":80,"label":{"text":"Redis limits"}},
 {"type":"rectangle","id":"log","x":340,"y":0,"width":180,"height":70,"label":{"text":"Usage events\n(Kafka)"},"backgroundColor":"#d0ebff","fillStyle":"solid"},
 {"type":"rectangle","id":"agg","x":680,"y":0,"width":180,"height":70,"label":{"text":"Aggregator"},"backgroundColor":"#d0ebff","fillStyle":"solid"},
 {"type":"ellipse","id":"ledger","x":1020,"y":-5,"width":200,"height":80,"label":{"text":"Usage ledger"},"backgroundColor":"#d0ebff","fillStyle":"solid"},
 {"type":"rectangle","id":"recon","x":680,"y":170,"width":180,"height":70,"label":{"text":"Reconciliation"},"backgroundColor":"#d0ebff","fillStyle":"solid"},
 {"type":"arrow","id":"a1","x":90,"y":0,"width":250,"height":-135,"start":{"id":"api"},"end":{"id":"redis"},"label":{"text":"approx limits"}},
 {"type":"arrow","id":"a2","x":180,"y":35,"width":160,"height":0,"start":{"id":"api"},"end":{"id":"log"},"label":{"text":"event"}},
 {"type":"arrow","id":"a3","x":520,"y":35,"width":160,"height":0,"start":{"id":"log"},"end":{"id":"agg"}},
 {"type":"arrow","id":"a4","x":860,"y":35,"width":160,"height":0,"start":{"id":"agg"},"end":{"id":"ledger"},"label":{"text":"upsert"}},
 {"type":"arrow","id":"a5","x":860,"y":205,"width":240,"height":-130,"start":{"id":"recon"},"end":{"id":"ledger"},"strokeStyle":"dashed"}
]}

The review, as you would deliver it: ranked, each with why it matters and a fix.

Blocking 1: billing data lives only in Redis. Redis replication is asynchronous; a primary failure loses the last seconds of increments, and a failover during the hourly job loses or repeats a window. At 50k requests/s and, say, $0.002 per request on average, one lost minute is ~$6,000 of unbilled usage, and nobody can tell which customers it belonged to. Fix: emit a usage event per request (request_id, org, model, input_tokens, output_tokens, ts) to a durable log, and build billing from that. Redis stays for limits only.

Blocking 2: the hourly job races key expiry and minute boundaries. Keys expire after 2 hours; if the job is late by an hour (deploy, outage), it reads partial data and silently bills less. SCAN while writes continue can see a minute bucket that is still being incremented (a request finishing at 10:59:59.9 lands in 10:59 after the job read it). Fix: aggregate from the log by event time with a watermark (close an hour after a 10-minute grace period), make the aggregation idempotent (upsert keyed by org, hour, model), and keep the raw events 30+ days so any hour can be recomputed.

Blocking 3: no idempotency. API servers retry; the doc does not say whether a retried INCRBY after a timeout double counts (it does: the first may have succeeded). Fix: every usage event carries request_id; the aggregator dedupes on it (a 24-hour dedupe window keyed by request_id is a few GB at 50k/s × 86,400 × ~32 bytes ≈ 140 GB; use a windowed state store or dedupe per partition with the log keyed by request_id).

Major 4: limits check before the request but count after. A request reads the counter, then runs for 20 seconds, then increments. 500 concurrent long requests from one org all pass the check before any of them counts: limits can be overshot by the org's concurrency × average request size. Fix: reserve an estimate at admission (max_tokens or a running average) and reconcile on completion; or limit concurrent requests as well as tokens per minute.

Major 5: one Redis primary on the hot path of every request. 50k requests/s × 2 operations = 100k ops/s: fine today on one node, but at 3× growth it is at the edge, and the biggest org (often 20–30% of traffic) is a hot key on one shard however you cluster. A Redis blip now fails or slows every API request. Fix: local token buckets per server that sync with Redis every ~100 ms (approximate, which limits allow); fail open with conservative local limits if Redis is unreachable; shard keys by org.

Major 6: minute buckets use each server's clock. Skew of a few hundred milliseconds moves usage between minutes. Harmless for limits, but invoices and customer usage dashboards will disagree at the edges. Fix: bill by the event's server timestamp taken at one place (the gateway), and state the rule.

Missing before it can ship:

• Reconciliation: a daily job comparing ledger totals with gateway request logs per org; alert on any difference above 0.1%.
• Backfill and correction: how to re-bill an hour after a bug, and how customers see a correction.
• Observability: events produced versus aggregated (lag, drops), limit rejections per org, Redis latency.
• Rollout: shadow mode first (compute usage both ways for two weeks, compare per org), then switch invoices; the "flag next sprint" plan has no way to tell if the new numbers are right.
• Customer-facing behavior: what a limited request returns (429 with Retry-After and which limit), and a usage endpoint so customers can check their own bill.
• Security and privacy: usage events must not carry prompt content; who can read per-customer usage.

Questions to ask the author (they may know things the doc does not say): Is per-minute granularity required by pricing, or is per-hour enough? Are there contractual limits (enterprise commitments) that need hard enforcement? What is the current billing error budget?

• Log-based billing adds a component (Kafka plus a stream job) and minutes of delay to usage dashboards, in exchange for exactness, replay and audit. For money, that trade is clearly right.
• Approximate local limits can let an org exceed its limit by a few percent for a fraction of a second; strict global limits would put a cross-region round trip on every request.
• Reserve-then-reconcile limits are more accurate under long requests but can reject requests that would have fit; tune the reservation to the 90th percentile of actual usage.
• Scope: the review should not redesign everything. The proposal is a reasonable rate limiter; it becomes a problem only because billing was attached to it. Saying "keep this for limits, move billing" is a smaller, more persuasive change than a rewrite.

Recommendation as delivered: approve the rate-limiting half with changes 4 and 5; do not ship billing on Redis; build billing on usage events with idempotent aggregation, run it in shadow for two weeks with reconciliation, then switch.

Mid-level (L4)

• Restates the goal and finds several real problems (Redis durability, retries double counting).
• Suggests reasonable fixes for each.
• Communicates clearly and respectfully.

Senior (L5)

• Separates exact billing from approximate limiting and builds the critique around that split.
• Traces failure modes along the request path (failover, late jobs, retries, long requests, clock skew) with numbers on impact.
• Ranks issues as blocking, major and minor, and lists what is missing for launch (reconciliation, rollout, observability).

Staff (L6+)

• Proposes the smallest change that fixes the blocking issues and a migration path (shadow, compare, switch).
• Asks the questions that could change the conclusion and adapts to the answers.
• Gives a clear recommendation and shows how they would run the review so the author leaves with a better design and their ownership intact.