Skip to content

fix(bench): cancel and join owned benchmark transfers - #46

Merged
cppla merged 2 commits into
mainfrom
codex/benchmark-cancellation
Oct 2, 2026
Merged

cppla merged 2 commits into
mainfrom
codex/benchmark-cancellation

Conversation

@cppla

@cppla cppla commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Make benchmark shutdown close and join accepted transfers, including a socket waiting for admission, instead of waiting only for a stalled peer's default two-minute timeout.
  • Interrupt and join the client's owned benchmark connection on cancellation during request, payload, or acknowledgement I/O. Retain original failed-dial errors and cancellation causes, own failed/late returned connections, reject nil success, and skip already canceled dials.
  • Add real owned TCP and net.Pipe regressions; preserve protocol, default transport behavior, successful timing/statistics, and the individual transfer's shared tunnel dialer.
  • Strengthen the existing native QUIC admission/timeout fixture with authoritative server state and typed rejection reasons. No QUIC production code changed.
  • Document source-build availability; released v1.0.1 is unchanged.

Validation

  • Same-source negative benchmark baseline: all 12 cancellation leaves fail on the previous implementation; independent cleanup joins complete. Four separate original dial-ownership leaves also fail as expected.
  • Final combined candidate: Go 1.25.13 make check and make race pass.
  • Benchmark source unchanged from the first candidate: Go 1.25.13 complete package race x10, 270 group-inclusive RUN/PASS entries, zero failures/skips.
  • Final combined focused Go 1.27.1 race x5: 150 group-inclusive RUN/PASS entries across the benchmark package and QUIC admission control, zero failures/skips.
  • Final QUIC control Go 1.25.13 race x20: 60 group-inclusive RUN/PASS entries (20 tops / 40 leaves), zero failures/skips. A completed client TLS handshake before application Serve starts is observed with zero registered server connections and zero admission slots; the exact first peer is then observed admitted before starting the second dial.
  • The global rejection still has the original 250 ms post-handshake bound and must be remote 0x102 with the global-limit reason. The separate auth control retains the 500 ms budget / 2 s upper bound and checks no premature expiry plus remote 0x103.
  • One isolated capacity+1 negative mutation, tracked production unchanged: global control fails on the wrong per-source reason despite the same 0x102; auth control passes and independent cleanup joins complete. This proves the fixture does not let per-source rejection mask a broken global limit.
  • Final isolated cached-image Linux/arm64 container x5: 150 group-inclusive RUN/PASS entries across both selected packages, zero failures/skips. CGO disabled, not Linux race coverage; no external networking, image pull, privilege, or remote machine. Owned container removed; source/binary hashes unchanged.

Initial CI failure retained

Initial head 9e5da60d82b5bb416c606245b1a16b1c36a8c180, CI run 37070510464 attempt 1: 10 jobs passed, Go 1.25.13 job 111048589015 failed in the existing QUIC connection-limit test; benchmark package passed. The complete failure/shuffle log is retained. Client TLS completion was not a server-admission witness in that fixture. Follow-up commit adds that witness and separates the timer control without changing production or weakening the rejection policy. The exact cause of the original CI observation remains unproven. No blind workflow rerun was requested, and the initial failed head was not merged.

Boundaries

No protocol/default/dependency/credential/authentication-rule changes, release/tag, or remote deployment. Not performance or traffic-classification evidence. External listener closure while Serve is already blocked on pending admission is not claimed as promptly detected; parent-context cancellation is the tested shutdown path. No new Windows benchmark-runtime claim.

Base: 0a23ee20d28f642557e5dc7841cb842cd293a6d9. Final candidate: a3f06ab0f38fe49c42c7293a2a91f7c8ba0a01c9; tree: 53a389dbd1053fbf293b08f1000b5678f6cb692c. Six changed files, two commits.

Merge and main verification

Squash-merged as d78cbc599468c19ae848f44b3b2ea76f0efd76bd after the final-head CI (37071474072, all 11 jobs), CodeQL (37071474085), and netem (37071474120) succeeded. The official merge tree equals the frozen candidate tree above; its sole parent is the exact base. Local main is clean and matches origin/main; all frozen source checksums are unchanged.

Independent exact-merge main push / attempt 1 checks also completed successfully: CI 37071902553, all 11 jobs; CodeQL 37071903385; Linux netem 37071902493. This does not replace the retained original failure evidence. No release/tag or remote deployment was performed.

Copilot AI balanced review requested due to automatic review settings October 2, 2026 22:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The focused ownership changes have targeted regression coverage and no unresolved blocking findings.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes benchmark lifecycle handling so cancellation closes owned connections and joins transfers without closing the shared tunnel dialer. The published v1.0.1 binary is unchanged.

Changes:

  • Cancel and join server transfers, including connections awaiting admission.
  • Interrupt client I/O and handle failed, nil, or canceled dial results safely.
  • Add lifecycle regression tests and document source-build availability.
File Description
internal/​netbench/​netbench.go Adds cancellation-aware connection ownership and cleanup.
internal/​netbench/​listener_ownership_test.go Tests listener and worker cleanup after accept failures.
internal/​netbench/​dial_ownership_test.go Tests dial-result ownership and cancellation-close completion.
internal/​netbench/​cancellation_test.go Tests shutdown during stalled I/O and pending admission.
docs/​BENCHMARK.md Documents cancellation behavior and release boundaries.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@cppla
cppla merged commit d78cbc5 into main Oct 2, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants