Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Solid structure and the transport-first framing matches the ticket intent (callback vs WebSocket, sizing, auth on long-lived connections, backpressure, operational pitfalls). The metric names are the blocking problem: three of the four cited instruments are wrong or do not exist, which makes the observability and capacity-planning guidance unusable as written.
Blocking:
apollo_router_session_count_activeis not a per-subscription counter. The real metric isapollo.router.session.count.active(in-flight GraphQL requests across all operation types, now deprecated in favor ofhttp.server.active_requests). For counting open subscriptions useapollo.router.opened.subscriptions.apollo_router_deduplicated_subscriptions_event_count_totaldoes not exist in the router instrument set. Deduplication is surfaced throughapollo.router.opened.subscriptions(counts distinct subscriptions, not per-client) and the termination metrics; there is no deduplicated-event-count instrument. Both the sizing formula input and the backpressure bullet need to be re-pointed at a real metric or the claim dropped.apollo_router_skipped_event_count_totalis mis-named; the instrument isapollo.router.skipped.event.count(no_totalsuffix).
Also fix:
- The capacity formula's
1.2 µsconstant and the "50k-100k idle / ~5k at 1 event/sec" figures are presented as fact but flagged only as heuristic. Either cite the internal benchmark source or label them clearly as illustrative; SAs will quote these to customers. - The "See also" config link uses the legacy path
/docs/router/configuration/subscription. Current canonical slug isgraphos/routing/operations/subscriptions/configuration. Verify all links resolve. - Author's own caveat stands: the source Google Doc's customer-specific recommendations are not folded in. AC requires confirming technical accuracy against the source before this is review-complete; it should not advance until that happens.
The transport/auth/pitfalls prose is accurate and worth keeping. Fix the metric names against the standard router instruments reference and this is close.
|
|
||
| - ~8–16 KB of heap (the WS frame buffer + the per-stream state machine). | ||
| - One file descriptor. | ||
| - One in-flight `apollo_router_session_count_active` slot. |
There was a problem hiding this comment.
apollo_router_session_count_active is not subscription-specific; it counts all in-flight GraphQL requests and is deprecated (use http.server.active_requests). For open-subscription count use apollo.router.opened.subscriptions.
There was a problem hiding this comment.
Verified at 4b8a95a: apollo_router_session_count_active has been removed. The open-subscription count is now tracked with apollo_router_opened_subscriptions (OTel apollo.router.opened.subscriptions). This thread can be resolved.
|
|
||
| A 4 vCPU / 8 GiB Router holds 50k–100k idle subscriptions comfortably, but only ~5k subscriptions that all receive an event every second. The bottleneck shifts from memory to CPU as event frequency rises. | ||
|
|
||
| Plan capacity with `apollo_router_deduplicated_subscriptions_event_count_total` per second (the rate at which subgraph events are fanning out) as the primary input. Per-instance: |
There was a problem hiding this comment.
apollo_router_deduplicated_subscriptions_event_count_total does not exist as a router instrument. The sizing formula needs a real input; apollo.router.opened.subscriptions (distinct open subscriptions) is the closest measure of fan-out scale.
There was a problem hiding this comment.
Verified at 4b8a95a: the invented apollo_router_deduplicated_subscriptions_event_count_total is gone. The sizing formula now takes apollo_router_opened_subscriptions (distinct active subscriptions) as its real input. This thread can be resolved.
|
|
||
| Subgraphs can produce events faster than clients can consume them. Router applies backpressure by: | ||
|
|
||
| - Dropping events when the per-client buffer is full (counted as `apollo_router_skipped_event_count_total`). |
There was a problem hiding this comment.
Metric name is apollo.router.skipped.event.count (no _total suffix).
There was a problem hiding this comment.
Verified at 4b8a95a: the OTel instrument name is now written correctly as apollo.router.skipped.event.count, without the _total suffix. This thread can be resolved.
| Subgraphs can produce events faster than clients can consume them. Router applies backpressure by: | ||
|
|
||
| - Dropping events when the per-client buffer is full (counted as `apollo_router_skipped_event_count_total`). | ||
| - Coalescing events with identical payloads (counted as `apollo_router_deduplicated_subscriptions_event_count_total`). |
There was a problem hiding this comment.
No such instrument as apollo_router_deduplicated_subscriptions_event_count_total. Deduplication is reflected in apollo.router.opened.subscriptions (counts distinct subscriptions, not per-client deliveries); rewrite or drop this bullet.
There was a problem hiding this comment.
Verified at 4b8a95a: the backpressure bullet no longer references the nonexistent apollo_router_deduplicated_subscriptions_event_count_total. It now describes deduplication through apollo_router_opened_subscriptions (post-dedup distinct subscriptions, not per-client deliveries). This thread can be resolved.
Address docs-reviewer feedback. The cited metrics were wrong or fictitious; the standard-instruments reference lists no `apollo.router.session.count.active` (deprecated; was a request gauge, not subscription-specific) nor any deduplicated-event counter. Rewrite sizing + backpressure guidance around the two metrics that actually exist: - `apollo.router.opened.subscriptions` (Prometheus: `apollo_router_opened_subscriptions`) — count of distinct open subscriptions, post-deduplication. Use this for capacity planning, not session-count. - `apollo.router.skipped.event.count` (`apollo_router_skipped_event_count_total`) — events dropped under backpressure. The deduplication call-out is reframed against the post-dedup behaviour of `opened.subscriptions`. Also repoint the subscriptions-configuration link to the current `/docs/graphos/routing/operations/subscriptions` IA. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Second-pass review. All blocking items from the prior review are resolved at head 4b8a95a:
- apollo_router_session_count_active removed; open-subscription count now uses apollo_router_opened_subscriptions (OTel apollo.router.opened.subscriptions).
- The nonexistent apollo_router_deduplicated_subscriptions_event_count_total is gone from both the sizing formula and the backpressure section; both now point at real instruments (apollo_router_opened_subscriptions and apollo_router_skipped_event_count).
- The OTel name apollo.router.skipped.event.count is now correct (no _total suffix).
The two flagged non-blocking items are also addressed: the 1.2us constant is explicitly labeled heuristic from internal benchmarks, and the See also config link uses the canonical graphos/routing/operations/subscriptions slug rather than the legacy path. The transport/auth/pitfalls prose remains accurate. The remaining Google Doc caveat is the author's own intentional pre-public-publish note and does not block merging into this repo. Approving.
Summary
Adds
docs/subscriptions-in-production.md. Covers: transport choice (WebSocket vs HTTP callback) as the primary infrastructure-shaping decision, Router sizing heuristics for WS subscriptions, subgraph transport selection, auth on long-lived connections, backpressure metrics, and the operational pitfalls (LB connection caps, rolling deploys breaking WS, subscription query fan-out amplifying cost).Important
The original ticket references a Google Doc draft as primary source material. This draft synthesizes public Apollo guidance and observed field patterns; the Google Doc's customer-specific recommendations should be folded in by a reviewer with access before upstreaming to the public docs site.
Tracks AS-302.
Note
Placement: apollographql/docs archived; apollographql/platform-docs requires SAML grant.
Test plan
🤖 Generated with Claude Code