Introduce operations rate limits by IvanBorislavovDimitrov · Pull Request #1901 · cloudfoundry/multiapps-controller

IvanBorislavovDimitrov · 2026-08-24T08:09:10Z

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.

@nAmed

Match the constructor-injection convention of the other @nAmed beans in the process util package.

OperationRateLimiter now emits an INFO log line on every rejection with user, spaceGuid, and reason in a fixed format that Dynatrace can parse as structured fields: Operation start rejected: user="<u>" spaceGuid="<s>" reason="<r>" This covers all three rejection paths: per-space active-op cap, per-user active-op cap, and token-bucket exhaustion. Also restores bucket4j + testcontainers dependencies in the web module pom that were dropped during a prior rebase.

Adds OperationRateLimitMetricsMBean / OperationRateLimitMetrics with two attributes: - RateLimitRejectionCount — all-time total since last restart - RateLimitRejectionCountInWindow — rolling window counter (resets on each Dynatrace poll cycle, consistent with UploadDurationMetrics) OperationRateLimiter calls metrics.recordRejection() from the shared rejectAndLog helper so every rejection — active-op cap, space bucket, or user bucket — is counted. Registered in JmxConfiguration under the object name org.cloudfoundry.multiapps.controller.web.monitoring:type=Metrics,name=OperationRateLimitMetricsMBean.

sonarqubecloud · 2026-09-11T11:02:02Z

Quality Gate passed

Issues
28 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

vkalapov · 2026-09-16T07:49:27Z

+    void testSpaceKeyIsStableAcrossRuns() {
+        long firstValue = OperationRateLimitKeys.spaceKey(SPACE_GUID);
+        long secondValue = OperationRateLimitKeys.spaceKey(SPACE_GUID);
+        assertTrue(firstValue == secondValue);


Why isn't assertEquals used here?

vkalapov · 2026-09-16T10:01:54Z

+    private void checkTokenBuckets(String user, String spaceGuid) {
+        consumeSpaceToken(spaceGuid, user);
+        consumeUserToken(spaceGuid, user);
+    }


What happens if the space token is consumed but after that the user bucket is exhausted? Wouldn't that lead to losing space tokens for other users in the space? Would checking the user first make more sense in that case?

vkalapov · 2026-09-16T10:18:43Z

+    private static final String SPACE_NAMESPACE_PREFIX = "space:";
+    private static final String USER_NAMESPACE_PREFIX = "user:";
+    private static final String SEGMENT_SEPARATOR = ":";
+    private static final HashFunction HASH_FUNCTION = Hashing.sha256();


We shouldn't use sha256 to be compliant according to internal cryptography requirements. com.google.common.hash.Hashing provides sha384 - https://guava.dev/releases/33.5.0-jre/api/docs/com/google/common/hash/Hashing.html#sha384() which is compliant.

vkalapov · 2026-09-16T10:25:52Z

+
+    static String hashUser(String user) {
+        try {
+            byte[] digest = MessageDigest.getInstance("SHA-256")


We should avoid using SHA-256 for internal cryptography requirements. Please use a compliant algorithm such as SHA-384.

vkalapov · 2026-09-16T10:31:55Z

+        int activeOperationsPerSpace = operationService.createQuery()
+                                                       .spaceId(spaceGuid)
+                                                       .inNonFinalState()
+                                                       .list()
+                                                       .size();
+        if (activeOperationsPerSpace >= applicationConfiguration.getMaxActiveOperationsPerSpace()) {
+            rejectAndLog(user, spaceGuid, Messages.TOO_MANY_ACTIVE_OPERATIONS_IN_SPACE, NO_RETRY_AFTER_SECONDS);
+        }
+        int activeOperationsPerUser = operationService.createQuery()
+                                                      .user(user)
+                                                      .spaceId(spaceGuid)
+                                                      .inNonFinalState()
+                                                      .list()
+                                                      .size();


The second query is a subset of the first - both filter by spaceId + inNonFinalState. Could the user count be derived from the already-fetched list to avoid the second DB query?

vkalapov · 2026-09-16T10:42:30Z

+    private void consumeSpaceToken(String spaceGuid, String user) {
+        BucketConfiguration configuration = buildBucketConfiguration(applicationConfiguration.getOperationRateLimitPerSpaceCapacity(),
+                                                                     applicationConfiguration.getOperationRateLimitPerSpaceRefillPerHour());
+        Bucket bucket = bucketStore.getBucket(OperationRateLimitKeys.spaceKey(spaceGuid), configuration);
+        consumeToken(bucket, user, spaceGuid);
+    }
+
+    private void consumeUserToken(String spaceGuid, String user) {
+        BucketConfiguration configuration = buildBucketConfiguration(applicationConfiguration.getOperationRateLimitPerUserCapacity(),
+                                                                     applicationConfiguration.getOperationRateLimitPerUserRefillPerHour());
+        Bucket bucket = bucketStore.getBucket(OperationRateLimitKeys.userKey(spaceGuid, user), configuration);
+        consumeToken(bucket, user, spaceGuid);
+    }


BucketConfiguration is built on every call to checkStartAllowed - once for space, once for user. Since ApplicationConfiguration values don't change at runtime, could these be built once and reused?

vkalapov · 2026-09-16T10:51:04Z

+        this.mBeanServer = mBeanServer;
+    }
+
+    @Scheduled(fixedRate = 1, timeUnit = TimeUnit.MINUTES)


Should we add the same instance guard - SELECTED_INSTANCE_FOR_CLEAN_UP from OperationRateLimitBucketCleaner in order to not run queries on every instance?

vkalapov · 2026-09-16T11:05:48Z

+    private Map<String, Long> countActiveOperationsByUser() {
+        Map<String, Long> counts = new HashMap<>();
+        operationService.createQuery()
+                        .inNonFinalState()
+                        .list()
+                        .forEach(op -> counts.merge(hashUser(op.getUser()), 1L, Long::sum));
+        return counts;
+    }
+
+    private Map<String, Long> countActiveOperationsBySpace() {
+        Map<String, Long> counts = new HashMap<>();
+        operationService.createQuery()
+                        .inNonFinalState()
+                        .list()
+                        .forEach(op -> counts.merge(op.getSpaceId(), 1L, Long::sum));
+        return counts;
+    }


Consider implementing a count query instead of list() to avoid loading all operations into memory just to count them.

karrgov · 2026-09-16T07:38:33Z


+    @Test
+    void testIsOperationRateLimitingEnabled() {
+        Mockito.when(environment.getBoolean(ApplicationConfiguration.CFG_OPERATION_RATE_LIMITING_ENABLED,


maybe remove the "Mockito." and make it a static import in order to be consistent with how it is in the OperationRateLimitBucketCleanerTest

karrgov · 2026-09-16T10:45:20Z

+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-failsafe-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>integration-test</goal>
+                            <goal>verify</goal>
+                        </goals>
+                    </execution>
+                </executions>
+                <configuration>
+                    <includes>
+                        <include>**/*IntegrationTest</include>
+                    </includes>
+                </configuration>
+            </plugin>


is that needed?

karrgov · 2026-09-16T12:04:57Z

    }

+    @Test
+    void testStartOperationWhenRateLimitAllowsStartsProcess() {


just to remove "Mockito." for when and verify and make it static

karrgov · 2026-09-16T12:07:58Z

+                <!-- https://mvnrepository.com/artifact/org.apache.maven.plugins/maven-failsafe-plugin -->
+                <plugin>
+                    <groupId>org.apache.maven.plugins</groupId>
+                    <artifactId>maven-failsafe-plugin</artifactId>
+                    <version>3.5.4</version>
+                </plugin>


probably related to the other thing with the integration tests, is it needed?

IvanBorislavovDimitrov added the DO NOT MERGE YET label Aug 24, 2026

IvanBorislavovDimitrov changed the title ~~Lmcrossitxsadeploy 3360 operation rate limits~~ Introduce operations rate limits Aug 24, 2026

IvanBorislavovDimitrov and others added 9 commits September 8, 2026 15:13

Extract operation rate-limit exception messages into constants

4441fb4

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.

Raise expired-bucket cleanup batch size and iteration cap

4543b7e

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.

Annotate PostgresBucketStore constructor with @Inject

aee7c1e

Match the constructor-injection convention of the other @nAmed beans in the process util package.

Finish implemenation

08b0c76

Yavor16 force-pushed the LMCROSSITXSADEPLOY-3360-operation-rate-limits branch from 47df5f3 to 08b0c76 Compare September 11, 2026 10:40

Remove unused dependnecies

138ade3

vkalapov requested changes Sep 16, 2026

View reviewed changes

karrgov reviewed Sep 16, 2026

View reviewed changes

Navigation Menu

Uh oh!

Introduce operations rate limits - #1901

Introduce operations rate limits#1901
IvanBorislavovDimitrov wants to merge 10 commits into
masterfrom
LMCROSSITXSADEPLOY-3360-operation-rate-limits

IvanBorislavovDimitrov commented Aug 24, 2026

Uh oh!

sonarqubecloud Bot commented Sep 11, 2026

Uh oh!

vkalapov Sep 16, 2026

Uh oh!

vkalapov Sep 16, 2026

Uh oh!

vkalapov Sep 16, 2026

Uh oh!

vkalapov Sep 16, 2026

Uh oh!

vkalapov Sep 16, 2026

Uh oh!

vkalapov Sep 16, 2026

Uh oh!

vkalapov Sep 16, 2026

Uh oh!

vkalapov Sep 16, 2026

Uh oh!

karrgov Sep 16, 2026

Uh oh!

karrgov Sep 16, 2026

Uh oh!

karrgov Sep 16, 2026

Uh oh!

karrgov Sep 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

IvanBorislavovDimitrov commented Aug 24, 2026

Uh oh!

sonarqubecloud Bot commented Sep 11, 2026

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants