feat(core): HTTP/2 Phase 0 — groundwork (limits, error model, decision log)
Establishes the package layout, limits/error model and decision-log convention that every later HTTP/2 phase depends on, per flash/docs/http2/IMPLEMENTATION-PLAN.md Phase 0. - dev.relism.flash.h2: package-info (architecture overview), Http2ErrorCode (the 14 RFC 9113 §7 codes with precomputed 4-byte wire encodings), Http2Exception (connection error -> GOAWAY) and Http2StreamException (stream error -> RST_STREAM), neither extending IOException, both with stack-trace capture disabled on the hot rejection path. - Http2Limits: every bound Phase 0 requires (concurrent streams, frame size, header list size, CONTINUATION/reset/settings/ping rate bounds, flow-control windows, HPACK table size/string length, assembly and idle timeouts), each documented with the attack or RFC clause it addresses. - dev.relism.flash.http.Http1Limits: the h1 bounds needed by EX-03 (strict Content-Length) and EX-08 (header count/size limits). - flash/docs/http2/DECISIONS.md seeded with DEC-01..DEC-11 (the ten decisions implied by the plan itself, plus DEC-11 recording that commits keep scope `core` rather than adding `h2` to AGENTS.md). - flash/docs/http2/IMPLEMENTATION-PLAN.md: added the Progress Ledger (tracks phase status across sessions) and checked off Phase 0's DoD. 19 new tests, full flash module suite green (226/226). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Zakaria El Orche
co-authored by
Claude Sonnet 5
Claude Sonnet 5
parent
8f1f30b973
commit
db6e4a4d0c
@@ -0,0 +1,282 @@
|
||||
# Flash HTTP/2 — Decision Log
|
||||
|
||||
This is the living record of every non-obvious choice made while implementing
|
||||
`flash/docs/http2/IMPLEMENTATION-PLAN.md`. It is not a changelog of what was built — the git
|
||||
history is that — it is a record of *why*, for choices that were not forced by the RFC and that
|
||||
a future reader would otherwise have to re-derive or, worse, silently re-litigate.
|
||||
|
||||
Every entry: **Context / Options / Decision / Consequence / Revisit when**.
|
||||
|
||||
Seeded at Phase 0 with `DEC-01`…`DEC-10` (the decisions already implied by the plan itself, per
|
||||
Appendix A). Every subsequent non-obvious choice appends a new entry with the next free number.
|
||||
Numbers are never reused, even if a decision is later reversed — the reversal gets its own entry
|
||||
that supersedes the earlier one and says so explicitly.
|
||||
|
||||
---
|
||||
|
||||
## DEC-01 — HTTP/2 lives in `flash` core, package `dev.relism.flash.h2`, not an extension
|
||||
|
||||
**Context.** Flash has an extension mechanism (`flash-ext-*` modules) for optional
|
||||
functionality. HTTP/2 could in principle be shipped as `flash-ext-h2`.
|
||||
|
||||
**Options.**
|
||||
1. Ship as an extension, loaded optionally.
|
||||
2. Ship in `flash` core, alongside HTTP/1.1.
|
||||
|
||||
**Decision.** Core (option 2).
|
||||
|
||||
**Consequence.** The protocol decision (h1 vs h2) is made once, immediately after
|
||||
ALPN/preface detection, inside the transport layer. `HttpServer` (and its Phase 2 replacement)
|
||||
is package-private to `flash` core; an extension cannot hook into ALPN negotiation or the
|
||||
accept loop without core exposing seams it does not otherwise need. HTTP/2 is a transport
|
||||
concern in the same sense HTTP/1.1 is — it cannot be optional in the way, say, an OpenAPI
|
||||
generator is.
|
||||
|
||||
**Revisit when.** Never, absent a restructuring of the extension mechanism itself to support
|
||||
transport-level extensions (not currently planned).
|
||||
|
||||
---
|
||||
|
||||
## DEC-02 — h1 and h2 are peers behind a `ConnectionProtocol` seam, never flags in shared code
|
||||
|
||||
**Context.** The obvious shortcut is `if (isHttp2) { ... } else { ... }` scattered through the
|
||||
existing HTTP/1.1 code paths.
|
||||
|
||||
**Options.**
|
||||
1. Flag-branch inside shared code.
|
||||
2. A `ConnectionProtocol` interface with two implementations (`Http1Connection`,
|
||||
`Http2Connection`), selected once per connection.
|
||||
|
||||
**Decision.** Option 2 (R1).
|
||||
|
||||
**Consequence.** Shared code (byte scanning, the writer discipline, `Request`/`Response`) is
|
||||
extracted upward into protocol-neutral components (`dev.relism.flash.bytes`,
|
||||
`ResponseSerializer`), never pushed sideways with a protocol flag. This is enforced by an
|
||||
architecture test (Phase 2) asserting `dev.relism.flash.http1` never references
|
||||
`dev.relism.flash.h2` and vice versa. The cost is more up-front extraction work in Phase 2 and
|
||||
Phase 6; the benefit is that h1 throughput cannot regress from an `if` that the JIT fails to
|
||||
eliminate, and that either implementation can be read in isolation.
|
||||
|
||||
**Revisit when.** Never — this is a structural invariant, not a tunable.
|
||||
|
||||
---
|
||||
|
||||
## DEC-03 — `ReentrantLock` everywhere, never `synchronized` around blocking I/O
|
||||
|
||||
**Context.** Java 21 (this project's baseline) has virtual threads (JEP 444) but not JEP 491
|
||||
(which removes `synchronized` carrier-pinning); JEP 491 lands in JDK 24. A virtual thread that
|
||||
blocks inside a `synchronized` block pins its carrier platform thread for the duration of the
|
||||
block, including any blocking I/O inside it.
|
||||
|
||||
**Options.**
|
||||
1. Keep `synchronized` where it already exists (`WebSocketSession`, `EX-01`) and accept the
|
||||
pinning risk.
|
||||
2. Replace every `synchronized` block that can block on I/O with `java.util.concurrent.locks
|
||||
.ReentrantLock`, which unmounts a blocked virtual thread instead of pinning its carrier.
|
||||
|
||||
**Decision.** Option 2, applied retroactively to the existing WebSocket code (Phase 2) and as a
|
||||
standing rule for every future connection-writer path, most importantly `Http2FrameWriter`
|
||||
(Phase 3).
|
||||
|
||||
**Consequence.** One virtual thread blocking on a slow write no longer starves the carrier pool
|
||||
for every other connection scheduled onto that carrier. The cost is that `ReentrantLock` is
|
||||
slightly more expensive than an uncontended `synchronized` monitor in the*platform-thread* case
|
||||
— irrelevant here, since every request-serving thread in this codebase is virtual.
|
||||
|
||||
**Revisit when.** The project's Java baseline moves to JDK 24+ and JEP 491 is confirmed to
|
||||
remove pinning for `synchronized`. Even then, `ReentrantLock`'s explicit `tryLock()` — which
|
||||
`synchronized` cannot offer — is load-bearing for Phase 3's writer design, so this decision
|
||||
would only partially reverse.
|
||||
|
||||
---
|
||||
|
||||
## DEC-04 — The HPACK **encoder** uses the static table only; no dynamic table
|
||||
|
||||
**Context.** RFC 7541's dynamic table is optional for an encoder (a decoder must always
|
||||
support the peer using one; nothing requires the encoder to use one itself). Using it on the
|
||||
encode side would save bytes on repeated headers (e.g. a constant `server` value) but requires
|
||||
mutable, connection-shared state: an insertion changes indices for every subsequent encode on
|
||||
that connection.
|
||||
|
||||
**Options.**
|
||||
1. Encoder uses the dynamic table, saving bytes on repeated custom headers.
|
||||
2. Encoder emits only Indexed (static) and Literal-Without-Indexing representations; no dynamic
|
||||
table, no mutable encoder state.
|
||||
|
||||
**Decision.** Option 2.
|
||||
|
||||
**Consequence.** The write path — already the project's largest architectural risk (Phase 3) —
|
||||
needs no shared-table lock and no invalidation protocol across concurrently-writing streams.
|
||||
The cost is a few extra bytes per response for headers that do not already have a static-table
|
||||
entry (i.e. everything except the ~30 header names RFC 7541 Appendix A knows about). The
|
||||
encoder still honours the peer's `SETTINGS_HEADER_TABLE_SIZE` by sending a Dynamic Table Size
|
||||
Update of 0 at the start of the first header block, declaring "I will never use this table" —
|
||||
a correctness detail, not optional politeness (Phase 9 task 1).
|
||||
|
||||
**Revisit when.** Benchmark evidence (Phase 17) shows the extra wire bytes materially hurt
|
||||
throughput or latency on a realistic workload — not before. A shared dynamic table is a
|
||||
non-trivial correctness surface (see `DEC-06`'s discussion of the analogous decode-side hazard)
|
||||
and should only be taken on with a measured reason.
|
||||
|
||||
---
|
||||
|
||||
## DEC-05 — Huffman-encode constants at boot; emit runtime values as raw literals
|
||||
|
||||
**Context.** HPACK lets the encoder Huffman-code any string at its option. Constants (status
|
||||
lines, `content-type` values) are a closed, known set and can be Huffman-encoded once, at class
|
||||
initialization, for free at runtime. Runtime-generated values (a dynamic `ETag`, a user-set
|
||||
custom header) would need to be Huffman-encoded on every response.
|
||||
|
||||
**Options.**
|
||||
1. Huffman-encode everything, including runtime values, on every write.
|
||||
2. Huffman-encode only boot-time constants; emit runtime values as raw (uncompressed) literals.
|
||||
|
||||
**Decision.** Option 2, with `FlashConfiguration.h2HuffmanDynamicValues` (default `false`) so
|
||||
option 1's cost/benefit can actually be measured on real traffic rather than argued about in
|
||||
the abstract.
|
||||
|
||||
**Consequence.** The response write path's critical section has no per-byte Huffman encode
|
||||
loop for the common case. The cost is a few extra bytes on the wire for runtime header values,
|
||||
which HPACK's other mechanisms (indexing on the receive side, if the receiver chooses to use
|
||||
its dynamic table) can still partially recover.
|
||||
|
||||
**Revisit when.** Phase 17 benchmarks the flag both ways on a representative response shape.
|
||||
|
||||
---
|
||||
|
||||
## DEC-06 — Decoded headers are copied into a **per-stream** arena, not referenced in the dynamic table
|
||||
|
||||
**Context.** A `ByteView` into the HPACK dynamic table's arena is valid only while its entry is
|
||||
still live. Under HTTP/1.1 this is trivially safe (one thread, one request at a time). Under
|
||||
HTTP/2, the demux thread can decode a second stream's HEADERS — evicting and overwriting
|
||||
dynamic-table arena bytes — while a handler on a different virtual thread is still reading a
|
||||
view produced by an earlier decode. This is a genuine, silent data race: it does not manifest
|
||||
in any test that decodes one block at a time, only under real multiplexed load.
|
||||
|
||||
**Options.**
|
||||
1. Reference dynamic-table entries directly from decoded `ByteView`s, and protect them with an
|
||||
epoch or reference-count scheme so an entry cannot be evicted while still referenced.
|
||||
2. Copy every decoded header (name and value) into an arena owned by the stream being
|
||||
assembled, at decode time. One `~30`-byte-average `memcpy` per header; correctness by
|
||||
construction, no cross-thread coordination.
|
||||
|
||||
**Decision.** Option 2.
|
||||
|
||||
**Consequence.** Header decode is not zero-copy relative to the dynamic table (R3's "honest
|
||||
naming" clause applies: HTTP/2 copies each novel header once per connection and references it
|
||||
by index thereafter — the per-stream arena copy is that one copy). In exchange, no handler can
|
||||
ever observe a torn or evicted header value, and the demux thread never needs to coordinate
|
||||
with a handler thread to decode the next block. Per-stream arenas are pooled (returned on
|
||||
stream close) so this is zero allocation at steady state despite the copy.
|
||||
|
||||
**Revisit when.** Profiling (Phase 17) shows the per-header copy is a measurable cost on a
|
||||
realistic HPACK-heavy workload. Even then, option 1's concurrent bookkeeping is a large
|
||||
correctness surface to take on to avoid a small `memcpy`, and should not be revisited casually.
|
||||
|
||||
---
|
||||
|
||||
## DEC-07 — `:authority` is exposed to user code as both `:authority` and `host`
|
||||
|
||||
**Context.** HTTP/2 requests carry authority information in the `:authority` pseudo-header
|
||||
(RFC 9113 §8.3.1), not a `Host` header — `host` may optionally also be present and, if so, must
|
||||
match `:authority`, but is not required. Existing Flash middleware (and most middleware in the
|
||||
wild) reads `Host` by convention, inherited from HTTP/1.1.
|
||||
|
||||
**Options.**
|
||||
1. Expose only `:authority`, under whatever name the h2 header map uses for pseudo-headers.
|
||||
Middleware written against `Host` silently breaks on h2.
|
||||
2. Expose `:authority`'s value under both keys: the literal `:authority` and `host`.
|
||||
|
||||
**Decision.** Option 2.
|
||||
|
||||
**Consequence.** A single small duplication (one extra index entry into the same per-stream
|
||||
arena bytes — no extra copy) buys behavioural parity for existing and future middleware that
|
||||
reads `Host`, without requiring every middleware author to special-case h2. Documented in
|
||||
`flash/docs/http2/STREAMS.md`.
|
||||
|
||||
**Revisit when.** Not planned to be revisited; this is a compatibility shim with negligible
|
||||
cost, not a design compromise under pressure.
|
||||
|
||||
---
|
||||
|
||||
## DEC-08 — Flash ships HTTP/2, not a gRPC codec
|
||||
|
||||
**Context.** gRPC is one of the strongest motivations for HTTP/2 support (Pathway's upstream
|
||||
use case), and it is tempting to let that motivation expand scope into shipping gRPC framing,
|
||||
proto codecs, or a service-definition layer.
|
||||
|
||||
**Options.**
|
||||
1. Ship a gRPC codec/framework alongside HTTP/2 transport support.
|
||||
2. Ship HTTP/2 transport only; validate gRPC compatibility with an interop test, not a feature.
|
||||
|
||||
**Decision.** Option 2.
|
||||
|
||||
**Consequence.** Phase 12's `GrpcInteropTest` proves that the protocol features gRPC actually
|
||||
needs — trailers, `content-type: application/grpc`, `te: trailers`, half-close, streaming — are
|
||||
present and correct, using a real gRPC client against a hand-written Flash handler that speaks
|
||||
the wire format directly. Flash does not gain a dependency on any gRPC/protobuf library, and
|
||||
users who want a gRPC service framework build it on top of Flash rather than being handed one.
|
||||
|
||||
**Revisit when.** Not planned to be revisited; this is a scope boundary, not a temporary
|
||||
limitation.
|
||||
|
||||
---
|
||||
|
||||
## DEC-09 — The chosen `Http2FrameWriter` design, with its benchmark numbers
|
||||
|
||||
**Status.** Not yet decided — this entry is a placeholder until Phase 3 runs its gate. Phase 3
|
||||
benchmarks three candidate writer designs ((a) plain `ReentrantLock.lock()` per frame,
|
||||
(b) `tryLock()` + intrusive MPSC, (c) a dedicated writer virtual thread fed by an MPSC queue)
|
||||
against the numeric gate criteria in the plan (0 B/op and <50 ns overhead at N=1; ≥60% of the
|
||||
N=1 per-thread aggregate throughput and <1 ms p999 at N=64; no carrier pinning). This entry is
|
||||
filled in with the winning design and the raw numbers when Phase 3 completes, or with the
|
||||
failure and the redesign taken if no candidate meets the gate.
|
||||
|
||||
**Revisit when.** N/A until Phase 3 lands.
|
||||
|
||||
---
|
||||
|
||||
## DEC-10 — `Upgrade: h2c` is deliberately **not** implemented
|
||||
|
||||
**Context.** RFC 7540 §3.2 (the original HTTP/2 RFC) defined an `Upgrade: h2c` mechanism to
|
||||
move a plaintext HTTP/1.1 connection to HTTP/2 mid-connection. RFC 9113 (which obsoletes
|
||||
RFC 7540) §3.1 removes this mechanism entirely from the current specification.
|
||||
|
||||
**Options.**
|
||||
1. Implement `Upgrade: h2c` for compatibility with any client that still relies on it.
|
||||
2. Do not implement it; support cleartext HTTP/2 only via prior knowledge (RFC 9113 §3.4).
|
||||
|
||||
**Decision.** Option 2.
|
||||
|
||||
**Consequence.** Every h2c client that matters for Flash's use case (gRPC, and every modern h2c
|
||||
implementation) uses prior knowledge, not the upgrade dance, so nothing is lost in practice.
|
||||
Recorded explicitly so a future contributor who notices `Upgrade: h2c` is unhandled does not
|
||||
assume it was an oversight and add it back.
|
||||
|
||||
**Revisit when.** A concrete client that requires `Upgrade: h2c` and cannot be changed is
|
||||
identified. Not anticipated.
|
||||
|
||||
---
|
||||
|
||||
## DEC-11 — Commit scope stays `core`; `h2` is not added to `AGENTS.md`'s allowed-scope list
|
||||
|
||||
**Context.** `AGENTS.md` (§Commit Messages) enumerates the allowed Conventional Commits scopes.
|
||||
`h2` is not among them. R9 leaves the choice open: either add `h2` as a new scope via a
|
||||
`docs:` commit, or use `core` and record the decision here.
|
||||
|
||||
**Options.**
|
||||
1. Add `h2` as a new allowed scope, so h2-specific commits are distinguishable in history from
|
||||
other core work at a glance.
|
||||
2. Use the existing `core` scope for all HTTP/2 work.
|
||||
|
||||
**Decision.** Option 2.
|
||||
|
||||
**Consequence.** All HTTP/2 commits use `feat(core): ...` / `fix(core): ...` /
|
||||
`refactor(core): ...`, consistent with the branch name (`feature/core/http2`) and with `DEC-01`
|
||||
(HTTP/2 is core, not a separate concern). A reader can still find every h2-related commit via
|
||||
the file paths touched (`dev.relism.flash.h2/**`, `flash/docs/http2/**`) or via the commit body,
|
||||
which is no worse than a scope label and avoids growing the scope list for what is, by `DEC-01`,
|
||||
not actually a separate module.
|
||||
|
||||
**Revisit when.** The `h2` package's commit volume makes `core` too coarse to navigate in
|
||||
`git log` — not expected before Phase 10 at the earliest, if ever.
|
||||
Reference in New Issue
Block a user