Skip to content

[api] rate-limit activations per client key, atomically with the cluster bucket - #1131

Draft
the-gigi-pplx wants to merge 3 commits into
mainfrom
gigi/pla-6979-per-client-activation-rate-atomic-with-cluster-bucket
Draft

the-gigi-pplx wants to merge 3 commits into
mainfrom
gigi/pla-6979-per-client-activation-rate-atomic-with-cluster-bucket

Conversation

@the-gigi-pplx

@the-gigi-pplx the-gigi-pplx commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Each client can now be given its own activation rate, so one client flooding creates and restores can't spend the whole cluster activation budget and get everyone else's requests rejected. It's off until configured.

An activation takes one token from the shared cluster bucket and, when its client has a per-client rate, one from that client's bucket. Both come from one Lua script: it refills both, and takes a token from each only when both hold one, otherwise from neither. Tokens aren't refundable, so consuming the buckets one after the other would charge one of them for an activation the other then refused. The client is the bounded key from the base PR: an allowlisted X-Client-ID, or unknown.

  • Keys. A client bucket lives at {space:cluster_admission:sandbox_activation}:client:<key>. The braces make the whole existing cluster key the hash tag, so in a clustered Redis both keys hash to one slot, and the cluster key keeps its name: a rolling deploy never splits the cluster budget across an old and a new key. With no per-client rate configured the script runs on the cluster key alone, as before. A key still expires once it could have refilled completely, but that TTL is now capped at 10^12 seconds, which EXPIRE accepts. At a refill rate small enough, burst / rate was past what EXPIRE accepts, and the script failed after its writes, so admission failed open. The cap is that large because an expired key comes back full: a shorter one would hand a slow bucket tokens its rate never refilled. It applies to the cluster bucket too.
  • Rejections. When the client's bucket is empty the 429 says so, names the key, and asks that client to slow down; for unknown it adds that activations without an allowlisted X-Client-ID, implicit wakes and recoveries included, share the rate. When only the cluster bucket is empty the existing server message is unchanged. When both are, the client's bucket is named. Both keep the jittered Retry-After, now based on whichever empty bucket refills last. cluster.sandbox_admission.rate.decision tags a per-client rejection bucket:client_activation.
  • Config. SPACE_CLUSTER_ACTIVATION_ADMISSION_CLIENT_REFILL_RPS and _CLIENT_BURST set one rate for every key, unknown included; unset means no per-client rate. unknown also takes native /v1 activations and implicit wakes from data-plane, ingress or control-plane access, which carry no client header, so the env rate caps all of that together; "unknown": null in the flag lifts it. The rate field of space-api-client-concurrency-quotas sets single keys: an entry replaces the env rate, null exempts the key, and a malformed entry leaves only its own key on the env rate. The field applies only while SPACE_CLUSTER_ACTIVATION_ADMISSION_ENABLED is on. A rate entry doesn't allowlist its key. Cold creates and restores share a client's bucket, and Redis errors fail open, as for the cluster bucket.
  • Warm pool. A warm-pool hit charges neither bucket, as it charges no cluster token today. The create's pool step and the charge that follows it move into controllers/sandbox/queries/pool_step.rs, so that order can be tested against the scripted pool substrate. Every create entry point runs on the real substrate, and adopting a slot calls into its pod.

Validation

At this head, on a Linux devbox:

  • cargo fmt --all -- --check and cargo clippy --workspace --all-targets --all-features -- -D warnings.
  • cargo nextest run --all-features against Postgres and Redis: pplx-space-api, 2096 passed; pplx-space-cluster-controller-server, pplx-space-metrics and pplx-space-feature-flags, 1245 passed.
  • On the previous base head, ci/scripts/run-rust-checks.sh tests-unprivileged: 10914 passed. tests-privileged: 61 passed and 6 failed. Each failure is in a crate this PR doesn't touch, and each fails on what the box lacks: fs-verity, a private cgroup namespace, and a safe temp-dir ancestry for the launcher.
  • Four mutations, each caught by the new tests:
    • Taking each bucket's token independently in the Lua fails an_empty_client_bucket_takes_no_cluster_token, an_empty_cluster_bucket_takes_no_client_token, the concurrent-consumer test and both admission-path rejection tests.
    • Charging admission before the pool step fails a_warm_hit_leaves_both_activation_buckets_untouched, which then sees a token gone from each bucket.
    • Charging admission before the cold shape resolves fails a_create_whose_cold_shape_fails_to_resolve_charges_no_bucket, which drives a pool config whose disk is over the rootfs cap.
    • Removing the TTL cap fails a_slow_bucket_outlives_its_refill_and_an_unbounded_one_is_capped with Redis's value is not an integer or out of range. The same test checks that a bucket refilling at 1e-8 tokens a second keeps its full refill time as its TTL.
  • every_client_bucket_shares_the_cluster_buckets_slot checks the hash slots with the Redis Cluster CRC16, pinned to the specification's check value.

Not validated: a clustered Redis, and a Kind or deployed smoke run.

Not covered

  • A malformed per-client rate entry logs a warning on every activation by that client, since the flag is read per activation, as the concurrency limiter reads it per request.
  • No typed space_smoke scenario drives a per-client 429 yet. Enabling and testing the rate in a deployment, including rendering SPACE_CLUSTER_ACTIVATION_ADMISSION_CLIENT_REFILL_RPS and _CLIENT_BURST in the deployment's chart, is follow-up work.
  • A per-client rejection is backpressure for dispatcher-run operations, as cluster shedding is: they're requeued without spending their retry budget. Native /v1 operations all draw from unknown, so with a rate set on unknown, one tenant's queued native work can hold other tenants' queued native work in requeue. Whether client-bucket rejections should get a bounded retry instead, or dispatcher work should skip the client bucket, is an open design question; until it's settled, leave unknown unrated with "unknown": null.
  • The design document describing cluster activation rate limiting is maintained outside this repository and is updated separately.

Severity and risk

Severity: without a per-client rate, one client can hold the shared activation budget at zero while it floods, so every other client's creates and restores get 429s.

Risk: the cluster bucket's script changes for every activation, including deployments that set no per-client rate. It keeps the same key, the same hash fields and the same one-bucket behavior, and old and new replicas can share a key during a rollout. The only change to an existing bucket's behavior is at a refill rate small enough to have failed the old script, which now keeps working. A per-client rejection maps to the same 429 and the same dispatcher backpressure as a cluster one. To roll back, unset the two env vars and remove the flag's rate field; reverting the PR restores the previous script.

Stack

Stacked on PR #1068, which carries the bounded client key into activation admission. Merge it first.

AI assistance: Claude Code drafted the code, the tests and this description.

@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6979-per-client-activation-rate-atomic-with-cluster-bucket branch 6 times, most recently from d107ddb to 6903254 Compare October 29, 2026 23:10
@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6979-per-client-activation-rate-atomic-with-cluster-bucket branch from 6903254 to ec4cd51 Compare October 29, 2026 23:34
@the-gigi-pplx
the-gigi-pplx marked this pull request as ready for review October 30, 2026 00:21
@the-gigi-pplx
the-gigi-pplx marked this pull request as draft October 30, 2026 06:59
@the-gigi-pplx

Copy link
Copy Markdown
Collaborator Author

Parking this as a draft. The design RFC in PR #1072 defers the per-client activation rate to its own RFC, to be built only if evidence shows a gap once the live-sandbox cap and an enforced concurrency limiter ship. The design in that RFC differs from this one: it holds tokens rather than refunding them, and it gives native traffic its own bucket.

@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6977-carry-activating-client-id-label-sandbox-pods branch 12 times, most recently from de0c507 to 85557fb Compare October 30, 2026 17:28
@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6977-carry-activating-client-id-label-sandbox-pods branch 2 times, most recently from b6aee34 to e7b713f Compare October 30, 2026 19:16
@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6977-carry-activating-client-id-label-sandbox-pods branch 3 times, most recently from ca4a494 to 73df601 Compare October 30, 2026 20:32
@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6977-carry-activating-client-id-label-sandbox-pods branch 4 times, most recently from 9db802c to 1a1939e Compare November 2, 2026 14:56
@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6977-carry-activating-client-id-label-sandbox-pods branch 2 times, most recently from e3167bc to fefa52e Compare November 2, 2026 21:14
@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6977-carry-activating-client-id-label-sandbox-pods branch 7 times, most recently from 8916ff2 to 66a5cde Compare November 5, 2026 15:09
@the-gigi-pplx
the-gigi-pplx force-pushed the gigi/pla-6977-carry-activating-client-id-label-sandbox-pods branch 6 times, most recently from 59fe600 to 1ad115a Compare November 12, 2026 21:57
Base automatically changed from gigi/pla-6977-carry-activating-client-id-label-sandbox-pods to main November 12, 2026 22:31
@coderabbitai

coderabbitai Bot commented Nov 12, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

This branch has not been deployed

No deployments
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.

1 participant