Repository navigation
[api] rate-limit activations per client key, atomically with the cluster bucket - #1131
Draft
the-gigi-pplx wants to merge 3 commits into
Draft
the-gigi-pplx wants to merge 3 commits into
the-gigi-pplx wants to merge 3 commits into
Conversation
the-gigi-pplx
force-pushed
the
gigi/pla-6979-per-client-activation-rate-atomic-with-cluster-bucket
branch
6 times, most recently
from
October 29, 2026 23:10
d107ddb to
6903254
Compare
the-gigi-pplx
force-pushed
the
gigi/pla-6979-per-client-activation-rate-atomic-with-cluster-bucket
branch
from
October 29, 2026 23:34
6903254 to
ec4cd51
Compare
the-gigi-pplx
marked this pull request as ready for review
October 30, 2026 00:21
kumarpallav-pplx
requested review from
kumarpallav-pplx and
nikhilbirmiwal-pplx
October 30, 2026 00:21
the-gigi-pplx
marked this pull request as draft
October 30, 2026 06:59
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
force-pushed
the
gigi/pla-6977-carry-activating-client-id-label-sandbox-pods
branch
12 times, most recently
from
October 30, 2026 17:28
de0c507 to
85557fb
Compare
the-gigi-pplx
force-pushed
the
gigi/pla-6977-carry-activating-client-id-label-sandbox-pods
branch
2 times, most recently
from
October 30, 2026 19:16
b6aee34 to
e7b713f
Compare
the-gigi-pplx
force-pushed
the
gigi/pla-6977-carry-activating-client-id-label-sandbox-pods
branch
3 times, most recently
from
October 30, 2026 20:32
ca4a494 to
73df601
Compare
the-gigi-pplx
force-pushed
the
gigi/pla-6977-carry-activating-client-id-label-sandbox-pods
branch
4 times, most recently
from
November 2, 2026 14:56
9db802c to
1a1939e
Compare
This was referenced Nov 2, 2026
the-gigi-pplx
force-pushed
the
gigi/pla-6977-carry-activating-client-id-label-sandbox-pods
branch
2 times, most recently
from
November 2, 2026 21:14
e3167bc to
fefa52e
Compare
the-gigi-pplx
force-pushed
the
gigi/pla-6977-carry-activating-client-id-label-sandbox-pods
branch
7 times, most recently
from
November 5, 2026 15:09
8916ff2 to
66a5cde
Compare
the-gigi-pplx
force-pushed
the
gigi/pla-6977-carry-activating-client-id-label-sandbox-pods
branch
6 times, most recently
from
November 12, 2026 21:57
59fe600 to
1ad115a
Compare
Base automatically changed from
gigi/pla-6977-carry-activating-client-id-label-sandbox-pods
to
main
November 12, 2026 22:31
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
|
This branch has not been deployed
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
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, orunknown.{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, whichEXPIREaccepts. At a refill rate small enough,burst / ratewas past whatEXPIREaccepts, 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.429says so, names the key, and asks that client to slow down; forunknownit adds that activations without an allowlistedX-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 jitteredRetry-After, now based on whichever empty bucket refills last.cluster.sandbox_admission.rate.decisiontags a per-client rejectionbucket:client_activation.SPACE_CLUSTER_ACTIVATION_ADMISSION_CLIENT_REFILL_RPSand_CLIENT_BURSTset one rate for every key,unknownincluded; unset means no per-client rate.unknownalso takes native/v1activations 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": nullin the flag lifts it. Theratefield ofspace-api-client-concurrency-quotassets single keys: an entry replaces the env rate,nullexempts the key, and a malformed entry leaves only its own key on the env rate. The field applies only whileSPACE_CLUSTER_ACTIVATION_ADMISSION_ENABLEDis on. Arateentry doesn't allowlist its key. Cold creates and restores share a client's bucket, and Redis errors fail open, as for the cluster bucket.controllers/sandbox/queries/pool_step.rs, so that order can be tested against the scripted pool substrate. Everycreateentry 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 -- --checkandcargo clippy --workspace --all-targets --all-features -- -D warnings.cargo nextest run --all-featuresagainst Postgres and Redis:pplx-space-api, 2096 passed;pplx-space-cluster-controller-server,pplx-space-metricsandpplx-space-feature-flags, 1245 passed.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.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.a_warm_hit_leaves_both_activation_buckets_untouched, which then sees a token gone from each bucket.a_create_whose_cold_shape_fails_to_resolve_charges_no_bucket, which drives a pool config whose disk is over the rootfs cap.a_slow_bucket_outlives_its_refill_and_an_unbounded_one_is_cappedwith Redis'svalue 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_slotchecks 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
rateentry logs a warning on every activation by that client, since the flag is read per activation, as the concurrency limiter reads it per request.space_smokescenario drives a per-client429yet. Enabling and testing the rate in a deployment, including renderingSPACE_CLUSTER_ACTIVATION_ADMISSION_CLIENT_REFILL_RPSand_CLIENT_BURSTin the deployment's chart, is follow-up work./v1operations all draw fromunknown, so with a rate set onunknown, 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, leaveunknownunrated with"unknown": null.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
429and the same dispatcher backpressure as a cluster one. To roll back, unset the two env vars and remove the flag'sratefield; 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.