fix: bound adaptive capacity recovery probes - #24
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Address the qualification reset and pending-probe feedback-grace issues.
Review effort: Lite
Findings: None
What changed in this PR
Adds bounded adaptive recovery probes to restore learned capacity after receiver backpressure while preserving conservative and fixed-rate behavior.
Changes:
- Adds bounded probe admission, deadlines, feedback handling, and backoff.
- Adds deterministic safety tests and real QUIC recovery coverage.
- Documents probe limits and operational caveats.
| File | Summary |
|---|---|
internal/tunnel/pacing_recovery_integration_test.go |
Adds real QUIC recovery validation. |
internal/accel/pacer.go |
Integrates probe pacing, refill, and lifecycle handling; pending probes may be aborted prematurely. |
internal/accel/capacity_probe.go |
Implements recovery probes; RTT-reset observations can incorrectly count toward qualification. |
internal/accel/capacity_probe_test.go |
Tests recovery under persistent RTT penalties. |
internal/accel/capacity_probe_safety_test.go |
Tests probe bounds, cancellation, expiry, and congestion handling. |
docs/ACCELERATION.md |
Documents probe behavior and limitations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix a bounded adaptive-pacing recovery failure: after real receiver backpressure lowered the learned capacity, persistent RTT penalties could prevent higher delivery observations even after the receiver became fast again.
Validation
make checkandmake race: passed locally.go test -race ./internal/accel -run '^TestCapacityProbeSafety' -count=20: passed.f6a931bpacer, same real QUIC regression, payload and assertions. All 3.5 MiB completed within the same deadline, but target stayed at 178,824 B/s; both final recovery windows failed the intended progress/target assertions. No simulated RTT or packet counters.These are bounded synthetic and loopback recovery observations, not claims about Internet throughput, fairness, browser similarity or general superiority. Large bandwidth-delay products/custom bursts and delayed feedback can exceed this mechanism's limits. No full corpus, release, or historical test-server access.
Exact head
3e2cf90bca05dccda9ee53d5944305aa84951b35passed CI (35806573075, all 10 jobs), CodeQL (35806573022), and Linux netem (35806573085). Container integration, OCI index build and namespace/netem suite each completed successfully. No unresolved review threads at merge check. Post-merge main1740df22857cb97406e98e0740e46701f5f901d6also passed CI (35806830314, all 10 jobs including container integration and OCI build), CodeQL (35806830336), and Linux netem (35806830311, suite plus diagnostics upload). Main treef3437c5bd5969391496e3b26cd99c3953e0f07acequals the tested PR tree. Local main is synchronized and clean; no release created.