Improve wait time in retry logic for CC calls by Yavor16 · Pull Request #1878 · cloudfoundry/multiapps-controller

Yavor16 · 2026-07-15T11:45:36Z

Summary

Improves the retry/backoff behaviour of ResilientCloudOperationExecutor for Cloud Controller calls so the service degrades more gracefully when the CC is under rate limiting or is unstable.

Honour the Retry-After header on HTTP 429. CloudControllerResponseErrorHandler now parses the Retry-After header on 429 TOO_MANY_REQUESTS responses and propagates it via a new CloudOperationException.getRetryAfterSeconds(). When present, the executor waits for that duration, capped at 120s to avoid excessively long stalls; when the header is absent, it falls back to a 60s wait.
Treat 429 as retryable. TOO_MANY_REQUESTS is added to the default set of statuses that are retried.
Randomized delay for repeated failures. After the first failed retry of a non-429 error, the executor waits a randomized interval between 30s and 90s instead of the fixed 5s, spreading out load when the CC is seriously impacted.

The random delay supplier and sleeper are injectable (package-private) purely to make the timing behaviour deterministically testable.

Acceptance criteria

Failed CC calls returning 429 respect the Retry-After header (capped), and fall back sensibly when it is missing.
Repeated retries no longer hammer the CC with a fixed short delay; they back off with a randomized wait.

Tests

Added coverage across three test classes:

CloudOperationExceptionTest — retry-after storage, default null, four-arg constructor leaves it null.
CloudControllerResponseErrorHandlerTest — Retry-After parsing on 429 (present, absent, non-numeric), non-429 does not set it, body + header passthrough.
ResilientCloudOperationExecutorTest — retry-after honoured, cap boundary, fallback wait, fixed vs randomized delay routing, retry exhaustion rethrow, Runnable-overload routing.

Test plan

mvn -pl multiapps-controller-client -am test -Dtest=CloudOperationExceptionTest,CloudControllerResponseErrorHandlerTest,ResilientCloudOperationExecutorTest

Extend CloudOperationException with a nullable retryAfterSeconds field so that the error handler can propagate the Retry-After response header to the retry executor. CloudControllerResponseErrorHandler now reads the Retry-After header on 429 responses and passes the parsed value (or null when absent or unparseable) to the exception constructor. ResilientCloudOperationExecutor overrides execute(Supplier) to compute wait time per-attempt: 429 errors honour the header value capped at 120s (or fall back to 60s when the header is absent); subsequent non-429 retries (attempt >= 2) use a random delay in the range 30–90s to spread load. The LongConsumer sleeper field enables test injection without requiring static mocking. TOO_MANY_REQUESTS is added to DEFAULT_STATUSES_TO_IGNORE so the executor retries 429 responses automatically. JIRA:LMCROSSITXSADEPLOY-2653

CloudOperationExceptionTest: verify retryAfterSeconds is null via existing constructors and stored correctly via the new five-arg form. CloudControllerResponseErrorHandlerTest: fix the ClientHttpResponseMock getHeaders() stub (was UnsupportedOperationException) and add four cases covering 429 with/without/unparseable Retry-After and non-429 headers. ResilientCloudOperationExecutorTest: add five new tests covering the Retry-After cap, header-absent fallback, fixed delay for non-429 first retry, and deterministic random delay from the second retry onward. Uses withSleeper and withRandomDelaySupplier testability seams to avoid real sleeps and non-deterministic assertions. Also adds withRandomDelaySupplier package-private builder to ResilientCloudOperationExecutor to support deterministic test seeding. JIRA:LMCROSSITXSADEPLOY-2653

Cover Retry-After cap boundary, retry exhaustion, Runnable-overload routing, 429 body+header passthrough, and four-arg constructor null retry-after. JIRA:LMCROSSITXSADEPLOY-2653

Yavor16

Automated code review by MTA Quality Agent — 2 finding(s).

Yavor16 · 2026-07-15T11:53:10Z

MTA Quality Report — cloudfoundry/multiapps-controller PR #1878

Jira: LMCROSSITXSADEPLOY-2653 — Improve wait time in retry logic for CC calls
Backlog alignment: PASS

Implements Jira scope? yes — the diff delivers all three scoped items: honour Retry-After on 429 (capped at 120s with a 60s fallback), add TOO_MANY_REQUESTS to the retryable set, and apply a randomized 30–90s wait after repeated failures.
Changes outside Jira scope? no — changes are confined to the retry/backoff logic (ResilientCloudOperationExecutor, CloudOperationException, CloudControllerResponseErrorHandler, Messages) and their tests.

Code Review

2 finding(s): 2 posted inline.

See the inline review on the PR diff for posted findings.

Security

L1 — LOW — Input validation: negative Retry-After value (CloudControllerResponseErrorHandler.java, extractRetryAfterSeconds): Long.parseLong(headerValue) does not reject negatives. A CC response with Retry-After: -1 leads to Thread.sleep(-1000) throwing IllegalArgumentException. The CC is normally a trusted internal service, but defense-in-depth warrants validating parsed > 0 — consistent with the code-review fix.

No CRITICAL / HIGH / MEDIUM findings. No hardcoded credentials, secrets, or internal URLs introduced; no new SQL, shell, deserialization, or SSRF surface; ThreadLocalRandom usage is appropriate (not security-sensitive).

SonarCloud

The "Build and analyze" check run for the head commit is currently in_progress. Quality gate status is UNKNOWN — SonarCloud has not yet completed analysis for this PR. No per-line annotations are available yet. Re-check Sonar status once the CI run completes.

Dependency CVEs

✅ No new CVEs — this PR does not modify any dependency files (pom.xml, build.gradle, lockfiles).

Negative or zero Retry-After values caused either an IllegalArgumentException in Thread.sleep (negative case) or a busy-wait tight loop against the CC (zero case). Fix both: - extractRetryAfterSeconds now returns null for values <= 0 - computeWaitMillis floors cappedSeconds at 1 via Math.max(1L, ...) JIRA:LMCROSSITXSADEPLOY-2653

Yavor16

Automated code review by MTA Quality Agent — 3 finding(s).

Yavor16 · 2026-07-15T12:03:56Z

MTA Quality Report — cloudfoundry/multiapps-controller PR #1878

Jira: LMCROSSITXSADEPLOY-2653 — Improve wait time in retry logic for CC calls
Backlog alignment: PASS

Implements Jira scope? yes — the diff delivers all three scoped items: honour Retry-After on 429 (capped at 120s with a 60s fallback), add TOO_MANY_REQUESTS to the retryable set, and apply a randomized 30–90s wait after repeated failures.
Changes outside Jira scope? no — changes are confined to the retry/backoff logic (ResilientCloudOperationExecutor, ResilientOperationExecutor, CloudOperationException, CloudControllerResponseErrorHandler, Messages) and their tests.

Code Review

3 finding(s): 3 posted inline.

See the inline review on the PR diff for posted findings.

Security

No issues found.

No CRITICAL / HIGH / MEDIUM findings. The Retry-After header is scoped to HTTP 429, parsed with Long.parseLong inside a try/catch, and guarded by parsed > 0; the server-supplied delay is bounded by Math.min(retryAfterSeconds, 120L) and floored at 1L, so a compromised Cloud Controller cannot force an arbitrarily long client-side sleep. All new log calls use SLF4J parameterized logging with long primitives (no log injection), and ThreadLocalRandom is used only for retry jitter (not security-sensitive).

SonarCloud

The "Build and analyze" check run for the head commit is currently in_progress. Quality gate status is UNKNOWN — SonarCloud has not yet completed analysis for this PR. No per-line annotations are available yet. Re-check Sonar status once the CI run completes.

Dependency CVEs

⏭️ CVE scan skipped on this review-fix re-run — dependencies were scanned on the initial quality run.

- Narrow catch in execute() from RuntimeException to CloudOperationException so non-retriable exceptions (NPE, ISE) are not silently swallowed - Promote loop counter i from int to long to match the protected long retryCount field and avoid integer overflow on large retry counts - Promote computeWaitMillis attemptIndex parameter from int to long to match the updated loop variable without a narrowing cast JIRA:LMCROSSITXSADEPLOY-2653

Yavor16

Automated code review by MTA Quality Agent — 1 finding.

Yavor16 · 2026-07-16T05:28:10Z

+        for (long i = 1; i < retryCount; i++) {
+            try {
+                return operation.get();
+            } catch (CloudOperationException e) {


Severity: CRITICAL · Confidence: 93

The overridden execute narrows the catch from the parent's RuntimeException to CloudOperationException. Any non-CloudOperationException RuntimeException thrown by the operation (e.g. a NullPointerException or Spring RestClientException that bypasses the error handler) now escapes immediately on the first attempt without retry, silently regressing the fault-tolerance guarantee that ResilientOperationExecutor provides. The parent's handle(RuntimeException) dispatch already exists in this class to handle the non-CloudOperationException path correctly — the catch clause just needs to be widened to match it.

Evidence: } catch (CloudOperationException e) {

Fix:

Suggested change

} catch (CloudOperationException e) {

} catch (RuntimeException e) {

Yavor16 · 2026-07-16T05:28:39Z

MTA Quality Report — cloudfoundry/multiapps-controller PR #1878

Jira: LMCROSSITXSADEPLOY-2653 — Improve wait time in retry logic for CC calls
(⚠️ Jira backlog item could not be fetched — the Jira MCP tool is unavailable in this environment. Alignment below is derived from the PR title/description against the diff.)
Backlog alignment: PASS

Implements Jira scope? yes — the diff delivers all three scoped items: honour Retry-After on 429 (capped at 120s with a 60s fallback), add TOO_MANY_REQUESTS to the retryable set, and apply a randomized 30–90s wait after repeated failures.
Changes outside Jira scope? no — changes are confined to the retry/backoff logic (ResilientCloudOperationExecutor, ResilientOperationExecutor, CloudOperationException, CloudControllerResponseErrorHandler, Messages) and their tests.

Code Review

1 finding(s): 1 posted inline.

See the inline review on the PR diff for posted findings.

Security

No issues found.

No CRITICAL / HIGH / MEDIUM findings. The three prior concerns are confirmed resolved: negative/zero Retry-After is rejected (parsed > 0), the server-supplied backoff is bounded to [1,120]s via Math.max(1L, Math.min(…,120L)) preventing DoS-via-sleep with no overflow risk, and the catch is narrowed to CloudOperationException. All new log calls use parameterized SLF4J with numeric primitives only, so no log-injection (CWE-117) or sensitive-data exposure is introduced.

SonarCloud

The "Build and analyze" check run for the head commit is currently in_progress. Quality gate status is UNKNOWN — SonarCloud has not yet completed analysis for this PR. No per-line annotations are available yet. Re-check Sonar status once the CI run completes.

Dependency CVEs

⏭️ CVE scan skipped on this review-fix re-run — dependencies were scanned on the initial quality run.

stiv03 · 2026-07-17T09:57:21Z

+        for (long i = 1; i < retryCount; i++) {
+            try {
+                return operation.get();
+            } catch (CloudOperationException e) {


Timeouts always surface as CloudOperationException? Maybe we can catch it more gracefully -RuntimeException?

stiv03 · 2026-07-17T10:00:23Z

    // INFO messages
    public static final String WAITING_MS_BEFORE_RETRYING_WITH_TIMEOUT_OF_MS = "Waiting: {} ms before retrying with timeout of: {} ms";
+    public static final String RATE_LIMITED_BY_CC_WAITING_S = "CC returned 429 with Retry-After: {} s. Waiting {} s (capped) before retrying.";
+    public static final String RATE_LIMITED_BY_CC_NO_HEADER_WAITING_S = "CC returned 429 without Retry-After header. Waiting {} ms before retrying.";


maybe to be RATE_LIMITED_BY_CC_NO_HEADER_WAITING_MS, cause its logging ms

stiv03 · 2026-07-17T10:12:47Z

+            return null;
+        }
+        String headerValue = response.getHeaders()
+                                     .getFirst(org.springframework.http.HttpHeaders.RETRY_AFTER);


import it above org.springframework.http.HttpHeaders and reference HttpHeaders.RETRY_AFTER

stiv03 · 2026-07-17T10:17:38Z

                    e);
    }

+    private long computeWaitMillis(RuntimeException e, long attemptIndex) {


Signature show support for Runtime exceptions, but the only caller method catches CloudOperationException

vkalapov

Check sonarcloud comments for unit tests.

sonarqubecloud · 2026-07-17T12:21:30Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

Yavor16 added 3 commits July 15, 2026 14:38

Add gap-case tests for Retry-After and retry executor behaviour

5ff032f

Cover Retry-After cap boundary, retry exhaustion, Runnable-overload routing, 429 body+header passthrough, and four-arg constructor null retry-after. JIRA:LMCROSSITXSADEPLOY-2653

Yavor16 commented Jul 15, 2026

View reviewed changes

Comment thread ...loudfoundry/multiapps/controller/client/facade/rest/CloudControllerResponseErrorHandler.java Outdated

Comment thread .../java/org/cloudfoundry/multiapps/controller/client/util/ResilientCloudOperationExecutor.java Outdated

Yavor16 commented Jul 15, 2026

View reviewed changes

Yavor16 commented Jul 16, 2026

View reviewed changes

Refactor

bd58831

stiv03 requested changes Jul 17, 2026

View reviewed changes

vkalapov requested changes Jul 17, 2026

View reviewed changes

Fix comments

92094b7

Yavor16 force-pushed the feature/LMCROSSITXSADEPLOY-2653 branch from 930e59c to 92094b7 Compare July 17, 2026 11:21

stiv03 previously approved these changes Jul 17, 2026

View reviewed changes

vkalapov reviewed Jul 17, 2026

View reviewed changes

Comment thread ...a/org/cloudfoundry/multiapps/controller/client/util/ResilientCloudOperationExecutorTest.java

Fix comment from Veli

dc32f28

Yavor16 dismissed stiv03’s stale review via dc32f28 July 17, 2026 12:13

	} catch (CloudOperationException e) {
	} catch (RuntimeException e) {

Uh oh!

Conversation

Yavor16 commented Jul 15, 2026

Summary

Acceptance criteria

Tests

Test plan

Uh oh!

Yavor16 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Yavor16 commented Jul 15, 2026

MTA Quality Report — cloudfoundry/multiapps-controller PR #1878

Code Review

Security

SonarCloud

Dependency CVEs

Uh oh!

Yavor16 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Yavor16 commented Jul 15, 2026

MTA Quality Report — cloudfoundry/multiapps-controller PR #1878

Code Review

Security

SonarCloud

Dependency CVEs

Uh oh!

Yavor16 left a comment

Choose a reason for hiding this comment

Uh oh!

Yavor16 Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

Yavor16 commented Jul 16, 2026

MTA Quality Report — cloudfoundry/multiapps-controller PR #1878

Code Review

Security

SonarCloud

Dependency CVEs

Uh oh!

stiv03 Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

stiv03 Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

stiv03 Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

stiv03 Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

vkalapov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jul 17, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants