Skip to content

Commit 98ac66e

Browse files
change attempts 3->4
1 parent d922085 commit 98ac66e

4 files changed

Lines changed: 19 additions & 14 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
- Default retry attempts corrected from 3 to 4 (one initial + three retries) to
12+
match SDK requirements §9.3 ("max 3 retries, yielding 4 total attempts").
13+
1014
### Added
1115
- Project scaffold per ADRs 001–007: Gradle Kotlin DSL build, JDK 17 toolchain,
1216
`integrationTest` source set, Spotless + JaCoCo, Vanniktech Maven Publish.

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ The Java SDK must also satisfy the canonical, cross-language [SDK Requirements](
6767
- §6 sealed `MarketDataException` hierarchy with the 7 canonical subtypes and full support context (`requestId`, `requestUrl`, `statusCode`, `timestamp`, `exceptionType`) + `getSupportInfo()`.
6868
- §10 timeouts: `REQUEST_TIMEOUT = 99s` and `CONNECT_TIMEOUT = 2s` exposed as constants on `MarketDataClient`. Connect timeout is wired into the `HttpClient`; the per-request 99 s timeout is applied via `HttpRequest.Builder#timeout` in `HttpTransport.buildRequest`.
6969
- §12 concurrency: 50-permit `AsyncSemaphore` on `HttpTransport` with acquire/release wired around every dispatch. The custom semaphore replaces `java.util.concurrent.Semaphore` so `executeAsync` never parks the caller's thread on a full pool (ADR-007).
70-
- §9 retry/backoff: `RetryPolicy` (3 attempts, exponential 1s→30s) wired into `HttpTransport.executeAsync` via a per-attempt loop using `CompletableFuture.delayedExecutor` (no scheduled threads). Network errors and HTTP 501–599 retry; 500 and 4xx do not.
70+
- §9 retry/backoff: `RetryPolicy` (4 total attempts = 1 initial + 3 retries, exponential 1s→30s per §9.3) wired into `HttpTransport.executeAsync` via a per-attempt loop using `CompletableFuture.delayedExecutor` (no scheduled threads). Network errors and HTTP 501–599 retry; 500 and 4xx do not.
7171
- §15 packaging: SemVer, MIT `LICENSE`, `CHANGELOG.md` in Keep a Changelog format, version auto-detected via JAR manifest (`Implementation-Version`).
7272
- §16 security: tokens never logged verbatim (use `Tokens.redact`); TLS validated by default (`HttpClient` does not expose a skip-verify option).
7373
- ADR-002 CI: split into four workflows.

src/main/java/com/marketdata/sdk/RetryPolicy.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@
77
import java.time.Duration;
88

99
/**
10-
* Decides which failures get retried and how long to wait between attempts. SDK requirements §9
11-
* fixes the parameters: max 3 attempts total (one initial + two retries), exponential backoff
12-
* starting at 1s, capped at 30s. Network errors (only when wrapping an {@link IOException}-shaped
13-
* cause — see {@link #shouldRetry}) and HTTP 501–599 are retriable; 500 specifically is not, and
14-
* 4xx (including 401/429) surfaces immediately.
10+
* Decides which failures get retried and how long to wait between attempts. Per SDK requirements
11+
* §9.3: max 3 retries (yielding 4 total attempts) with exponential backoff {@code initial *
12+
* 2^retry} starting at 1s, capped at 30s. Network errors (only when wrapping an {@link
13+
* IOException}-shaped cause — see {@link #shouldRetry}) and HTTP 501–599 are retriable; 500
14+
* specifically is not, and 4xx (including 401/429) surfaces immediately.
1515
*
16-
* <p><strong>Worst-case wall-clock per {@code executeAsync} call (defaults):</strong> 3 attempts ×
17-
* 99s per-request timeout + 1s + 2s backoff ≈ 5 minutes. SDK requirements §10 only mandates the
18-
* per-request timeout, not an overall deadline, so this is compliant — but callers in
16+
* <p><strong>Worst-case wall-clock per {@code executeAsync} call (defaults):</strong> 4 attempts ×
17+
* 99s per-request timeout + 1s + 2s + 4s backoff ≈ 6.75 minutes. SDK requirements §10 only mandates
18+
* the per-request timeout, not an overall deadline, so this is compliant — but callers in
1919
* latency-sensitive contexts may want to wrap calls with their own {@code orTimeout} cap.
2020
*
2121
* <p>The constructor accepts custom values so tests can drive retries with sub-millisecond delays
@@ -36,9 +36,9 @@ final class RetryPolicy {
3636
this.maxBackoff = maxBackoff;
3737
}
3838

39-
/** Spec defaults: 3 attempts, 1s → 30s exponential. */
39+
/** Defaults: 4 attempts, 1s → 30s exponential. */
4040
static RetryPolicy defaults() {
41-
return new RetryPolicy(3, Duration.ofSeconds(1), Duration.ofSeconds(30));
41+
return new RetryPolicy(4, Duration.ofSeconds(1), Duration.ofSeconds(30));
4242
}
4343

4444
/**

src/test/java/com/marketdata/sdk/RetryPolicyTest.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,12 @@ void unknownThrowableIsNotRetriable() {
105105
void retriesStopAfterMaxAttempts() {
106106
NetworkError retriable =
107107
new NetworkError("net", ErrorContext.empty(), new java.io.IOException("transport down"));
108-
// Defaults: maxAttempts = 3only attempts 0 and 1 are eligible to be followed by a retry
109-
// (attempt 2 was the third try; no fourth attempt allowed).
108+
// Defaults: maxAttempts = 4 → attempts 0, 1, 2 are eligible to be followed by a retry
109+
// (attempt 3 was the fourth try; no fifth attempt allowed).
110110
assertThat(DEFAULTS.shouldRetry(retriable, 0)).isTrue();
111111
assertThat(DEFAULTS.shouldRetry(retriable, 1)).isTrue();
112-
assertThat(DEFAULTS.shouldRetry(retriable, 2)).isFalse();
112+
assertThat(DEFAULTS.shouldRetry(retriable, 2)).isTrue();
113+
assertThat(DEFAULTS.shouldRetry(retriable, 3)).isFalse();
113114
assertThat(DEFAULTS.shouldRetry(retriable, 99)).isFalse();
114115
}
115116

0 commit comments

Comments
 (0)