Skip to content

Quantize auction transport timeouts to stabilize Fastly backend names#865

Open
prk-Jr wants to merge 7 commits into
mainfrom
quantize-auction-transport-timeouts
Open

Quantize auction transport timeouts to stabilize Fastly backend names#865
prk-Jr wants to merge 7 commits into
mainfrom
quantize-auction-transport-timeouts

Conversation

@prk-Jr

@prk-Jr prk-Jr commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fastly dynamic backend names deliberately embed the first-byte and between-bytes timeouts so a registration can never be silently reused with a different transport configuration. But 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 to bidder hosts (Fastly pools connections per backend name) and accumulating registrations toward the per-service dynamic backend limit.
  • Timeout canonicalization is now a platform capability: PlatformBackend::canonicalize_transport_timeout_ms defaults to the exact budget-bound value (remaining.min(configured)) because adapters that neither register nor name-key backends gain nothing from rounding and must not shorten bidder deadlines. Only the Fastly adapter overrides it with quantization.
  • The Fastly canonicalization has three regimes, all name-stable and globally bounded in cardinality: (1) the provider's configured timeout is returned verbatim when the budget allows — it is a constant, including sub-250ms configured values, which must not be rounded away or the provider could never launch; (2) budget-bound values below the 2,000ms ceiling floor to 250ms quanta, with sub-quantum remainders snapping down to the finite [200, 150, 100, 50] ladder (below the smallest rung the value canonicalizes to 0 and callers skip the launch as budget-exhausted); (3) at or above the ceiling, values floor to a fixed 8-rung coarse ladder (2,000 … 60,000ms, clamping above the top rung) so a large configured ceiling cannot mint hundreds of buckets. Rounding down never extends a transport cap past the remaining budget — the mediator and dispatched-collect </body> hold bounds rely on this, and the exhaustive enumeration tests assert it for every reachable input.
  • Backend naming is hardened so name equality implies spec equality: the name is backend_<readable>_<digest> where the digest is the first 128 bits of a SHA-256 over an unambiguous encoding of the complete spec (scheme, host, port, certificate setting, Host override, provider discriminator, and both transport timeouts). This makes NameInUse reuse provably safe and prevents first-registration-wins poisoning where a later request with a tighter timeout would inherit an earlier registration's value. The readable prefix is a lossy, bounded slug carried only for logs, and the whole name is bounded to Fastly's 255-char limit.
  • Each auction integration folds its integration id into the backend spec as a discriminator, so two providers targeting the same origin get distinct backend names on all four adapters and orchestrator response correlation by predicted name cannot cross providers.
  • Startup validation rejects a provider listed twice in [auction].providers and a mediator that is also listed as a provider (both would fire duplicate outbound requests per auction; a mediator's own demand already flows through its mediation response). The parallel and dispatch launch paths additionally skip a duplicate predicted backend name before sending, as defense in depth.
  • predict_name == ensure is pinned with a Viceroy test, since the orchestrator maps SSP responses back to providers by predicted backend name.

Changes

File Change
crates/trusted-server-adapter-fastly/src/backend.rs Digest-based backend naming: canonical spec string, 128-bit SHA-256 suffix (written into a pre-sized buffer), bounded human-readable prefix, Host-override/discriminator/cert markers, 255-char guard; predict_name convenience wrapper
crates/trusted-server-adapter-fastly/src/platform.rs Fastly override of canonicalize_transport_timeout_ms; quantum, sub-quantum ladder, and coarse ladder constants with rationale (including the accepted worst-case rounding haircut); exhaustive cardinality + budget-bound tests; predict_name == ensure and discriminator tests
crates/trusted-server-core/src/platform/traits.rs, types.rs, test_support.rs canonicalize_transport_timeout_ms trait method with exact-passthrough default; PlatformBackendSpec.discriminator field
crates/trusted-server-core/src/auction/orchestrator.rs Apply canonical timeout at all four provider-launch sites (parallel, dispatch, synchronous mediator, collect-path mediator); startup rejection of duplicate providers and provider/mediator overlap; pre-send duplicate-backend-name skip; deterministic timeout wiring tests via an injectable clock
crates/trusted-server-core/src/integrations/mod.rs Fold the integration id into the backend spec as the provider discriminator
crates/trusted-server-adapter-{axum,cloudflare,spin}/src/platform.rs Carry the discriminator through the backend spec; canonicalization keeps the trait default

Closes

Closes #847

Test plan

  • cargo test-fastly && cargo test-axum && cargo test-cloudflare && cargo test-spin
  • cargo clippy-fastly && cargo clippy-axum && cargo clippy-cloudflare && cargo clippy-cloudflare-wasm && cargo clippy-spin-native && cargo clippy-spin-wasm
  • cargo fmt --all -- --check
  • Integration parity suite: cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity
  • CLI tests: ./scripts/test-cli.sh
  • JS tests: cd crates/trusted-server-js/lib && npx vitest run
  • JS format: cd crates/trusted-server-js/lib && npm run format
  • Docs format: cd docs && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve

Checklist

  • Changes follow CLAUDE.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses log macros (not println!)
  • New code has tests
  • No secrets or credentials committed

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
@prk-Jr prk-Jr self-assigned this Jul 8, 2026
ChristianPavilonis

This comment was marked as low quality.

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Reviewed the timeout quantization change against the PR's actual main base. The arithmetic is internally consistent, but the current implementation has correctness and operational risks that should be addressed before merge. Detailed findings are attached inline.

Comment thread crates/trusted-server-core/src/auction/orchestrator.rs Outdated
Comment thread crates/trusted-server-core/src/auction/orchestrator.rs Outdated
Comment thread crates/trusted-server-core/src/auction/orchestrator.rs Outdated
Comment thread crates/trusted-server-core/src/auction/orchestrator.rs Outdated
Comment thread crates/trusted-server-core/src/auction/orchestrator.rs Outdated
Resolve the PR review by making transport-timeout canonicalization a
platform capability and hardening auction backend-name correlation.

- Move quantization behind PlatformBackend::canonicalize_transport_timeout_ms.
  Fastly floors budget-derived timeouts to a 250ms quantum with a bounded
  sub-quantum ladder [200,150,100,50]; other adapters use the exact remaining
  budget so bidder deadlines (Prebid tmax, APS timeout) are not shortened
  where no connection-pooling benefit exists.
- Bound sub-quantum backend-name cardinality: exact 1-249ms values no longer
  pass through, capping the budget-derived names a single origin can mint
  toward the per-service dynamic backend limit.
- Add a provider discriminator to PlatformBackendSpec, folded into every
  adapter's backend name, so two providers sharing one origin no longer
  collide on the response-correlation key. Reject a duplicate
  backend_to_provider insertion with an attributed launch failure instead of
  silently overwriting and misattributing a response.
- Make the orchestrator call-site tests deterministic: record predicted and
  registered transport timeouts separately and assert exact equality via a
  controllable platform backend, and enumerate the sub-quantum ladder to
  assert a bounded name cardinality.
- Correct the timeout-semantics comments that overstated absolute-deadline
  enforcement; the Fastly connect/first-byte/between-bytes timeouts bound
  connection, first-byte, and inactivity, not total response time. A true
  absolute deadline carried through the platform HTTP API remains follow-up
  work (#849).

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Reviewed current HEAD 5512d850 against main. The Fastly-only canonicalization and provider-discriminator wiring are directionally correct, but two high-risk backend identity/cardinality issues and two medium reliability/compatibility issues remain; details are inline.

Documentation

The PR description no longer describes HEAD: it still claims exact sub-quantum passthrough and a two-file change, while HEAD uses a [200, 150, 100, 50] ladder, skips values below 50 ms, adds a platform trait capability and discriminator, and changes 15 files. Please refresh the Summary, Changes table, and test plan.

Comment thread crates/trusted-server-adapter-fastly/src/backend.rs
Comment thread crates/trusted-server-adapter-fastly/src/platform.rs
Comment thread crates/trusted-server-adapter-fastly/src/backend.rs Outdated
Comment thread crates/trusted-server-core/src/auction/orchestrator.rs
prk-Jr added 3 commits July 16, 2026 17:55
Address the auction transport-timeout review findings:

- Derive dynamic backend names from a bounded readable prefix plus a
  SHA-256 digest of the complete backend spec, so distinct specs never
  alias to one name. Name equality now implies spec equality, making
  NameInUse reuse provably safe, and the name is bounded to Fastly's
  255-char limit regardless of host or discriminator length.
- Bound budget-derived transport-timeout cardinality with a globally
  finite ladder: keep the 250ms quantum through 2000ms, then snap larger
  budgets to a small fixed set of coarse rungs so a large configured
  ceiling can no longer mint hundreds of dynamic backends.
- Reject duplicate configured providers at startup, and check the
  predicted backend name before request_bids fires the outbound send,
  retaining the post-launch check as defense against a provider that
  resolves to an unexpected name.
…sport-timeouts

# Conflicts:
#	crates/trusted-server-adapter-fastly/src/backend.rs
#	crates/trusted-server-core/src/auction/orchestrator.rs
The backend name now carries a SHA-256 digest suffix, so the three
platform predict_name tests that pinned the exact/ending name string no
longer hold. Assert the readable body via starts_with/contains instead.
These were missed earlier because the local run filtered the platform
suite too narrowly; CI aborted on the first panic.

@aram356 aram356 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Reviewed HEAD 77f3521 against main. All nine findings from the two earlier review rounds are verified fixed on this HEAD: the digest-based backend naming makes name equality imply spec equality (and I confirmed the only Backend::builder call in the Fastly adapter flows through it), the discriminator keeps same-origin providers on distinct names on all four adapters, cardinality is globally bounded by the sub-quantum and coarse ladders, duplicate providers are rejected at startup with a pre-launch guard on both paths, canonicalization is Fastly-only via the platform trait, and the timeout wiring tests are deterministic. Remaining items are one documentation blocker and a handful of non-blocking suggestions, inline.

Blocking

🔧 wrench

  • Stale PR description: the description still describes the pre-review design — "exact passthrough for sub-quantum remainders so publishers with sub-250ms configured budgets keep launching" was removed in 5512d85 (budget-derived sub-quantum values now snap to the [200, 150, 100, 50] ladder; only configured sub-250ms constants pass through). It also predates most of what the PR now contains: digest-based backend naming, provider discriminators, the coarse ladder, duplicate-provider startup rejection, and the platform-capability split. The changes table still places the quantization helpers in orchestrator.rs, but they live in the Fastly adapter now. Please rewrite the Summary/Changes to describe HEAD.

Non-blocking

⛏ nitpick

  • ensure() doc drift: the doc comment still says the name is "derived from the scheme, host, port, certificate setting, first_byte_timeout, and between_bytes_timeout" — no digest, no discriminator, no Host override (crates/trusted-server-adapter-fastly/src/backend.rs:328-334). compute_name carries the accurate description; this paragraph should defer to it instead of restating a stale subset.

CI Status

  • fmt: PASS
  • clippy: PASS
  • rust tests (fastly/axum/cloudflare/spin + parity + CLI): PASS
  • js tests: PASS
  • integration/browser tests: PASS

Comment thread crates/trusted-server-adapter-fastly/src/platform.rs
Comment thread crates/trusted-server-adapter-fastly/src/platform.rs
Comment thread crates/trusted-server-core/src/auction/orchestrator.rs
Comment thread crates/trusted-server-adapter-fastly/src/backend.rs Outdated
- Reject a mediator that is also listed in [auction].providers at startup;
  the overlap would fire the same provider twice per auction and its demand
  already flows through the mediation response
- Assert in both cardinality enumeration tests that canonical values never
  exceed the remaining budget (the mediator </body> hold bound invariant)
- Document the accepted worst-case rounding haircut on the coarse ladder
- Write the spec digest hex into one pre-sized buffer instead of allocating
  per byte, and defer the ensure() doc to compute_name
@prk-Jr
prk-Jr requested a review from aram356 July 17, 2026 06:33
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.

Refactor Fastly backend name to exlcude extraneous identifiers

3 participants