Introduce operations rate limits - #1901
Open
IvanBorislavovDimitrov wants to merge 6 commits into
Open
Conversation
Add an application-level limiter that bounds the START of MTA operations along two independent keys (per CF user and per space) using two complementary mechanisms: - a token-bucket RATE limiter (bucket4j) whose state is persisted and synchronized in PostgreSQL via a SELECT FOR UPDATE proxy manager, so the limit holds across all service instances sharing the database; and - a concurrency cap on simultaneously non-final operations, counted over the shared operation table. The limiter is gated behind a feature flag (disabled by default) and all caps are configurable via environment variables. When triggered, the operation-start endpoint returns HTTP 429 with a Retry-After header and no process is started. Developed test-first: unit tests for config, key derivation, the limiter (mocked bucket + concurrency), and the 429 wiring; a Testcontainers integration test proves the cross-instance PostgreSQL synchronization.
Rows in operation_rate_limit_bucket were never removed: one row per space and per (space,user) key was inserted on first use and left forever, since the token-bucket store set no expiration and nothing swept the table. Set an expiration strategy so each row records when its bucket would be fully refilled (i.e. indistinguishable from a fresh bucket), and add a scheduled cleaner that deletes expired rows in batches. The cleaner runs on a single instance, only while rate limiting is enabled, and swallows/logs failures so it never disrupts the scheduler.
Replace the three inline exception-message string literals in the rate limiter with named constants in the web Messages class, matching the existing exception-message convention. No behavior change.
Increase the sweep batch to 1000 and the iteration cap to 10000 so a single cleanup run can clear far more expired rows, matching landscapes that accumulate many distinct rate-limit keys.
Relocate OperationRateLimitBucketCleaner into the process module's jobs package, alongside the existing clean-up jobs, and move its BucketStore / PostgresBucketStore collaborators into the process util package. The bucket4j dependency, the Postgres integration test, and the failsafe plugin move to the process module accordingly; the web limiter now uses the bucket store transitively. The three cleaner log messages move to the process Messages class. No behavior change.
Match the constructor-injection convention of the other @nAmed beans in the process util package.
IvanBorislavovDimitrov
changed the title
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
1 participant
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.
LMCROSSITXSADEPLOY-3360