Quantize auction transport timeouts to stabilize Fastly backend names#865
Open
prk-Jr wants to merge 1 commit into
Open
Quantize auction transport timeouts to stabilize Fastly backend names#865prk-Jr wants to merge 1 commit into
prk-Jr wants to merge 1 commit into
Conversation
Fastly dynamic backend names embed the first-byte and between-bytes timeouts so a registration can never be silently reused with a different transport configuration. Deriving those timeouts from the remaining wall-clock auction budget minted a new backend name on nearly every request, defeating cross-request TCP/TLS connection reuse (Fastly pools connections per backend name) and accumulating registrations toward the per-service dynamic backend limit. Compute the effective transport timeout from the configured provider timeout verbatim when the budget allows, floor the budget-bound value to 250ms buckets otherwise, and pass sub-quantum remainders through exactly so publishers with sub-250ms configured budgets keep launching. The quantized value feeds both the backend name and the registered configuration, so they cannot diverge. Rounding down never extends a transport cap past the auction deadline, which the mediator and dispatched-collect paths rely on to bound the </body> hold. Also add a Fastly platform test pinning predict_name == ensure for the same spec, since the orchestrator maps responses back to providers by predicted backend name. Fixes #847
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Automated review:
Review Summary
Reviewed PR #865 against main, focusing on the timeout quantization logic in crates/trusted-server-core/src/auction/orchestrator.rs and the Fastly backend-name consistency test in crates/trusted-server-adapter-fastly/src/platform.rs. I did not find blocking correctness, security, data-loss, authorization, or severe compatibility issues.
Findings
P0 / Blockers
None.
P1 / High
None.
P2 / Medium
None.
P3 / Low
None.
CI / Existing Reviews
gh pr checks 865 reports all checks passing, including Rust, JS/TS, formatting, integration, browser, CodeQL, and cross-adapter parity jobs. Existing review/comment queries did not show prior review feedback to duplicate.
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
</body>hold.predict_name == ensureon the Fastly platform backend with a Viceroy test, since the orchestrator maps SSP responses back to providers by predicted backend name.Changes
crates/trusted-server-core/src/auction/orchestrator.rsTRANSPORT_TIMEOUT_QUANTUM_MS,quantize_transport_timeout_ms, andeffective_transport_timeout_ms; apply at the four provider-launch sites (parallel, dispatch, synchronous mediator, collect-path mediator); extend the test stub with an optional timeout recorder and add quantization, sub-quantum-passthrough, and mediator-budget testscrates/trusted-server-adapter-fastly/src/platform.rspredict_name_matches_ensured_backend_nametest pinning the predicted name to the registered name for an identical specCloses
Closes #847
Test plan
cargo test-fastly && cargo test-axumcargo clippy-fastly && cargo clippy-axumcargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute servecargo test-cloudflare && cargo test-spin,cargo clippy-cloudflare && cargo clippy-cloudflare-wasm && cargo clippy-spin-native && cargo clippy-spin-wasm, and the integration parity suite (cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity) all passChecklist
unwrap()in production code — useexpect("should ...")tracingmacros (notprintln!)