test: prove downstream H2 concurrent-stream multiplexing - #77
test: prove downstream H2 concurrent-stream multiplexing#77seonghobae wants to merge 21 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-current writer review on 04b1329ae9eba5b048e18a29dfb5b178bc253b0c: verified the non-force parent restack and the subsequent fixture-only repair. The PR-visible delta remains one real-wire H2 multiplexing test. Its origin helper no longer assumes a complete HTTP/1 request arrives in one TcpStream::read; it reads until the header terminator under the existing 5 s deadline and fails closed above 64 KiB. This prevents TCP segmentation from manufacturing a false multiplexing failure without weakening the core oracle: streams 1 and 3 must still be issued on one verified h2 connection before either response, require two live H1 origin connections within 3 s, return distinct bodies, avoid RST_STREAM and terminate independently. No production Rust/Admin Config/TLS policy/routing/auth or external authority changed. I found no further writer-safe defect in this focused current delta. COMMENT only; exact CI/Supply Chain/capacity are nonterminal and this is not independent APPROVED or promotion credit.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-current writer review on 32cd55d5a838f38db4dcd0e1bc6e7da02aed41db: the H2 multiplexing oracle remains unchanged from the repaired child exact 04b1329...; the only new tree movement is adoption of parent #76's complete-origin-header fixture repair by an ordinary two-parent merge. One verified h2 connection must still carry streams 1 and 3 concurrently, with a second live H1 origin connection required inside 3 s before either response is released, no RST_STREAM, distinct bodies and independent stream termination. No production code or H2 acceptance semantics changed and I found no further writer-safe defect in this current delta. COMMENT only; exact-current CI/Supply Chain/capacity are nonterminal and this is not independent APPROVED or promotion credit.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-current writer review on c8a637e0c521edfbfabcc8ad13a394dca4eef9d0: parent a5dc77... is now in ancestry through ordinary merge 926c72902355d98034bfeab48bed54471ad23038; 6ffefe472ca5da31e4152b18f32c6fadda23c5ff restores the parent-fixed TLS profile fixture in the resolved tree, and current c8a637... applies the same absolute remaining-budget deadline to this PR's H2 multiplexing origin reader. The two-stream concurrency oracle, 3 s second-origin admission bound, frame assertions and production Rust are unchanged. COMMENT technical evidence only; new exact CI/Supply Chain/capacity are nonterminal and this is not independent APPROVED, merge, release or cutover credit.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-current writer review on a4791b4db22f9748c77562a7519e8fabe2b9f1ab: ordinary two-parent merge adopts #76 exact a0bf55151255beea04fb305e721b5629b7c79b2e and therefore the parent TRACEABILITY repair, while preserving the H2 multiplexing acceptance tree at c8a637.... The single verified-H2/two-live-stream oracle, 3 s second-origin admission bound, absolute five-second origin-header budget, 64 KiB cap and production Rust remain unchanged. COMMENT technical evidence only; fresh exact CI/Supply Chain/capacity are queued and no predecessor GREEN or independent approval transfers.
seonghobae
left a comment
There was a problem hiding this comment.
Current-head technical review after parent repair/restack. The previous #77 CI failure was inherited from #76's unformatted structural TLS contract; its H2 multiplexing file was not the failing path. This exact inherits the repaired TLS source and rustfmt-clean contract from current #76, preserves the H2 concurrency oracle unchanged, and was non-force restacked onto df7262d.... Exact parent→child comparison now contains only tests/downstream_tls_h2_multiplexing_wire.rs, which is the intended ownership boundary. Fresh exact-head gates must pass before Ready or merge; this COMMENT is not independent approval.
seonghobae
left a comment
There was a problem hiding this comment.
Current-head follow-up after the #75/#76 rustfmt repair stack. This child inherits the formatter-clean TLS source, then non-force restacks onto #76 exact 273832d... via two-parent merge bee37f6.... Exact parent→child comparison is ahead-only and still contains only tests/downstream_tls_h2_multiplexing_wire.rs; no H2 oracle delta was introduced by the repair. Fresh exact gates must pass before Ready or merge; this COMMENT is not independent approval.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-current restack review on 400cf5d67bd39e03d930472f8a8a31a73f9ca354. Prior exact bee37f61... failed CI at the inherited #76 formatting gate while Supply Chain and bounded-origin capacity were GREEN. The branch first inherited the exact Rust 1.98.0 import-layout repair as ordinary forward commit 3c4153cd806488d7ae3838938d5f4207ba9220b8, then non-force restacked onto current #76 bc34e6302efd53682b8a207ee73cac3600fe86b5 through a two-parent merge. Parent→child compare is ahead-only with merge base exactly bc34e630... and contains only tests/downstream_tls_h2_multiplexing_wire.rs, so the H2 multiplexing ownership boundary is preserved. Fresh exact CI/Supply Chain/capacity are queued. COMMENT technical evidence only; this is not independent APPROVED and Draft/merge/release/cutover credit remains withheld until exact-current gates terminate GREEN.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-current restack review on 75ced2c6e51afa917db00be70faf94081a1e0e3f. This is an ordinary two-parent merge from prior child 400cf5d... and repaired #76 parent a1a1a5dc...; no force push or destructive rebase was used. Exact parent→child compare is ahead-only with merge base exactly a1a1a5dc... and contains only tests/downstream_tls_h2_multiplexing_wire.rs. The child therefore inherits the parent's coverage-boundary repair without changing its H2 concurrent-stream oracle: one verified H2 connection sends streams 1 and 3 before either response is read, while the H1 origin requires a second distinct origin connection before releasing responses. No H2 traffic threshold, TLS policy, supplier source, authority boundary or test semantics changed in this restack. Fresh exact CI/Supply Chain/capacity are required before Ready; this COMMENT is technical evidence only, not independent approval.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-current restack review on fc203de8b56fff34d8070618a50198186fecd07f. Ordinary forward a3afce1a25839ad722715df71ec5157c81df13df inherited only #76's TLS coverage-evidence repair, then this exact head was created as a normal two-parent merge with current parent #76 7a5f5e04016e93a7fd515517051430d853dd3b45; the branch ref was fast-forwarded without force. Fresh parent→child comparison is ahead-only with merge base exactly 7a5f5e... and only tests/downstream_tls_h2_multiplexing_wire.rs as the effective child delta. The H2 concurrent-stream traffic oracle, timings and authority boundaries are unchanged. Fresh exact CI/Supply Chain/capacity are nonterminal, so predecessor evidence does not transfer and Draft remains appropriate. COMMENT technical evidence only, not independent APPROVED.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-current fixture review on fb677251c4f255d1b0b78a94b53433f003cf13db. Fresh review found a valid acceptance-integrity defect in predecessor fc203de8...: reserve_distinct_loopback_addresses() returned only SocketAddrs, so both reservation listeners were dropped long before config completion and gateway spawn. That reopened a material test race where another process could claim either traffic or metrics port and make the H2 oracle fail for an unrelated bind reason. Current repair returns live TcpListener reservations, derives both addresses while they are held, writes the config, then consumes/drops both reservations inside spawn_gateway() immediately before launching the compiled gateway. This follows the repository's established held-listener fixture pattern and changes no H2 frame/origin/concurrency/timing semantics. Fresh parent→child comparison remains ahead-only with merge base exactly current #76 7a5f5e... and only tests/downstream_tls_h2_multiplexing_wire.rs as effective child delta. New exact CI 34558703085, Supply Chain 34558703059, and capacity 34558703060 are pending/queued, so predecessor evidence does not transfer and Draft remains appropriate. COMMENT technical evidence only, not independent APPROVED.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-current non-force restack review on 0d583374607f32876fa17ef0501a8a548fb074f1. Parent #76 moved only by ADR-evidence commit to 2c4433b99c539a43bdecc74c7d167b2446dd4fdc. Child first inherited that exact ADR as ordinary forward 8bab2babe8d39786455ad1fee165cbd14cd430d7, then 0d583374... was created as a normal two-parent merge with current #76; both branch updates were fast-forward/non-force. Fresh compare is ahead-only, merge base exactly 2c4433..., and effective child delta remains only tests/downstream_tls_h2_multiplexing_wire.rs. The listener-reservation TOCTOU repair from fb677251... remains intact; H2 frames, concurrent-origin oracle and timing thresholds are unchanged. Fresh exact CI 34558985477, Supply Chain 34558985542, and capacity 34558985485 are nonterminal, so no predecessor evidence transfers and Draft remains appropriate. COMMENT technical evidence only, not independent APPROVED.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-current terminal review on 0d583374607f32876fa17ef0501a8a548fb074f1. Fresh execution is terminal GREEN across CI 34558985477, Supply Chain 34558985542, and PgErd bounded-origin capacity 34558985485. Re-read of the sole effective child path confirms the fixture still proves two live streams on one verified TLS/H2 connection before either origin response, with distinct H1 origin connections, no stream reset, distinct bodies, independent END_STREAM, a 3 s serialization bound, bounded complete-header reads, and held listener reservations through config construction to remove unrelated bind-race REDs. No unresolved review threads are present and no additional writer-safe source/test defect was found on this exact head. COMMENT technical evidence only; this is not independent APPROVED governance and does not authorize protected merge, release or cutover.
|
@coderabbitai approve Current exact |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Local commit identity and ancestry =="
git rev-parse HEAD
git show -s --format='commit=%H%nparents=%P%nsubject=%s' HEAD
git merge-base HEAD 2c4433b99c539a43bdecc74c7d167b2446dd4fdc
git diff --name-status 2c4433b99c539a43bdecc74c7d167b2446dd4fdc...HEAD
git diff --check 2c4433b99c539a43bdecc74c7d167b2446dd4fdc...HEAD
echo "== Changed test file structure and content =="
ast-grep outline tests/downstream_tls_h2_multiplexing_wire.rs --items all
cat -n tests/downstream_tls_h2_multiplexing_wire.rs
echo "== PR state, head, reviews, and checks =="
gh pr view 77 --repo ContextualWisdomLab/pingora-gateway \
--json number,state,isDraft,headRefOid,baseRefOid,mergeStateStatus,reviewDecision,reviews,comments,statusCheckRollup \
--jq '{
number, state, isDraft, headRefOid, baseRefOid, mergeStateStatus, reviewDecision,
reviews: [.reviews[] | {author: .author.login, state, commit: .commit.oid}],
comments: [.comments[] | {author: .author.login, body}],
checks: [.statusCheckRollup[] | {name, status, conclusion}]
}'Length of output: 26879 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/pingora-gateway"
sha="0d583374607f32876fa17ef0501a8a548fb074f1"
echo "== Check suites and runs for the exact head =="
gh api "repos/${repo}/commits/${sha}/check-suites?per_page=100" --paginate \
--jq '.check_suites[] | {
app: .app.slug,
status,
conclusion,
head_sha,
workflow_name: .workflow_run.name,
workflow_id: .workflow_run.id,
url: .html_url
}'
echo "== Unresolved review threads for PR 77 =="
gh api graphql -F owner="ContextualWisdomLab" -F name="pingora-gateway" -F number=77 -f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
isOutdated
path
line
comments(first: 1) {
nodes { author { login } body }
}
}
}
}
}
}' --jq '
.data.repository.pullRequest.reviewThreads.nodes
| map(select(.isResolved == false))
'Length of output: 3322
I reviewed only the delta from The GitHub Actions suites for this head are successful. Some third-party check suites remain queued and can affect merge eligibility independently. ✅ Action performedComments resolved. Approval is disabled; enable |
Refs #51. Ready-for-review writer-safe child of #76. Parent is exact
2c4433b99c539a43bdecc74c7d167b2446dd4fdc; current child exact is0d583374607f32876fa17ef0501a8a548fb074f1. No force push or destructive rebase has been used.This PR owns executable real-wire HTTP/2 multiplexing evidence only. One certificate-verified
h2downstream connection sends stream 1 (/) and stream 3 (/index.html) before either response is read. The H1 origin withholds response until a second distinct origin connection appears within 3 s, so a serialized gateway cannot manufacture GREEN; both streams must remain live, reach the expected paths, avoid RST_STREAM, return distinct bodies and terminate independently.Fresh review of predecessor
fc203de8...found a fixture-only TOCTOU: traffic/metrics reservation listeners were dropped long before config completion/process spawn, allowing unrelated port reuse to fail the H2 acceptance. Ordinary forwardfb677251c4f255d1b0b78a94b53433f003cf13dbrepairs that by holding bothTcpListenerreservations through config construction and consuming/dropping them insidespawn_gateway()immediately before process launch. H2 frame/origin/concurrency/timing semantics did not change.Parent #76 then moved from source/test repair
7a5f5e...to ADR-only exact2c4433.... This child inherited the current ADR in ordinary forward8bab2babe8d39786455ad1fee165cbd14cd430d7, then current0d583374...was created as a normal two-parent merge with #762c4433...; both ref movements were fast-forward/non-force. Fresh parent→child comparison is ahead-only, merge base exactly2c4433..., and effective child delta remains onlytests/downstream_tls_h2_multiplexing_wire.rs.Exact-current CI
34558985477, Supply Chain34558985542, and PgErd bounded-origin capacity34558985485are all terminal GREEN. No inline review threads are open. Exact-current terminal technical COMMENT5174930925re-read the sole effective child path after execution and found no additional writer-safe source/test defect; it is technical evidence only, not independentAPPROVEDgovernance credit.The branch-local multiplexing implementation/execution gate is therefore closed for independent review and this PR is Ready rather than Draft. This evidence does not close H2 stream cancellation/reset, GOAWAY/drain, decoded header/body admission, connection/stream flow control/backpressure, origin failure/recovery, forwarding trust, handshake/reuse performance, H3/QUIC, supplier #901 Cookie coalescing or release-qualified #936/#976-equivalent zero-length body disposition.
The active organization ruleset still requires one approving review, resolved review threads and central required workflows before default-branch integration. No bypass, protected merge, immutable release, canary/shadow, rollback, cutover or legacy-removal credit is claimed.