Skip to content

Commit ba820b7

Browse files
authored
Merge pull request #99 from runcycles/test/v0.1.25.11-concurrent-idempotency-and-counters
test: v0.1.25.11 concurrent idempotency + counter-accuracy tests
2 parents 62f3475 + 2ec5bd8 commit ba820b7

8 files changed

Lines changed: 252 additions & 7 deletions

File tree

AUDIT.md

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Cycles Protocol v0.1.25 — Server Implementation Audit
22

3-
**Date:** 2026-04-14 (v0.1.25.10 — custom Micrometer counters for reserve/commit/release/extend/expired/events + overdraft, plus Redis-disconnect resilience test; dormant emitExpiredEvent key-prefix bug fixed as a side effect),
3+
**Date:** 2026-04-14 (v0.1.25.11 — concurrent retry-storm test for idempotency cache expiry + concurrent accuracy test for custom counters; closes two gaps flagged in the v0.1.25.10 review),
4+
2026-04-14 (v0.1.25.10 — custom Micrometer counters for reserve/commit/release/extend/expired/events + overdraft, plus Redis-disconnect resilience test; dormant emitExpiredEvent key-prefix bug fixed as a side effect),
45
2026-04-14 (v0.1.25.9 — second-wave test additions: overdraft property, expire.lua conformance, admin-release race, multi-scope attribution, idempotency-cache expiry, clock-skew, metrics correctness, audit-log completeness),
56
2026-04-14 (property-based concurrent budget-exhaustion test + jqwik-spring lifecycle and tries-override follow-up fixes; passing green on Docker Desktop),
67
2026-04-12 (spec endpoint-coverage report — parity with admin),
@@ -14,6 +15,39 @@
1415

1516
---
1617

18+
### 2026-04-14 — v0.1.25.11: concurrent idempotency + metrics tests
19+
20+
Closes two gaps flagged in the post-v0.1.25.10 review. Both are regression gates rather than live bug fixes — the existing code is correct because Redis Lua execution is single-threaded and Micrometer counters are lock-free — but without these tests a future refactor could silently violate those guarantees.
21+
22+
**New test 1: thundering-herd retry on expired idempotency cache** (`IdempotencyCacheExpiryIntegrationTest.ThunderingHerd`)
23+
24+
The v0.1.25.10 `IdempotencyCacheExpiryIntegrationTest` covered the sequential case: delete cache → retry → new reservation. The ops-realistic failure mode is different: N concurrent retries arriving at the server after cache expiry, all missing the idempotency cache, all racing into `reserve.lua`. New test fires 10 concurrent retries through the full HTTP path and asserts:
25+
26+
- Exactly one distinct reservation id is returned across all 10 retries (Redis's Lua serialisation makes the winner's cache write visible to the others before they execute).
27+
- No HTTP errors; all 10 return 200.
28+
- The Redis hash state for the winning id is consistent (exactly one reservation, correct idempotency key stored).
29+
- Metric tags reflect reality: exactly 1 × `reason=OK` (the winner that actually ran the reserve body) + 9 × `reason=IDEMPOTENT_REPLAY` (the rest that took the idempotent-replay short-circuit). A wrong-tag regression would surface here.
30+
31+
If a future change moves idempotency from `reserve.lua` into Java (e.g. a distributed lock), this test fails because Java-side races break the atomicity guarantee.
32+
33+
**New test 2: concurrent custom-counter accuracy** (`MetricsCorrectnessIntegrationTest.concurrentCustomCounterIsAccurate`)
34+
35+
Sibling of the existing `concurrentRequestCountIsAccurate` that tests Spring Boot's HTTP timer. The new test asserts on the domain counter `cycles.reservations.reserve` under the same 8-thread × 10-request load. Micrometer counters are lock-free atomic longs so this should be accurate, but we had no regression gate against a future refactor introducing locking or a shared-builder race (e.g. an aspect that builds tags from a mutable map).
36+
37+
**Verification:**
38+
- `mvn -B verify --file cycles-protocol-service/pom.xml`: 135 api + 320 data = 455 tests (2 new), 0 failures, JaCoCo coverage met, spec coverage 9/9.
39+
40+
**Wire format:** Unchanged. No production-code changes.
41+
42+
**Modified files:**
43+
- `cycles-protocol-service/pom.xml``<revision>``0.1.25.11`.
44+
- `docker-compose.prod.yml`, `docker-compose.full-stack.prod.yml` — bump `cycles-server` pin to `0.1.25.11` per the release workflow.
45+
- `cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/IdempotencyCacheExpiryIntegrationTest.java` — new `ThunderingHerd` nested class + MeterRegistry wiring.
46+
- `cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/MetricsCorrectnessIntegrationTest.java` — new `concurrentCustomCounterIsAccurate` test.
47+
- `AUDIT.md`, `CHANGELOG.md`, `README.md` — release notes + version bump.
48+
49+
---
50+
1751
### 2026-04-14 — v0.1.25.10: custom business metrics + resilience test
1852

1953
Addresses the largest remaining gap flagged in the v0.1.25.9 retrospective: the service emitted no domain-level metrics, only Spring Boot's generic `http.server.requests` timer. Operators answering "how many denials in the last 5 minutes by reason and tenant" could only infer it from HTTP status codes. This release wires domain counters through a new shared component and extends the existing Micrometer integration test to cover them.

CHANGELOG.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,29 @@ changes to request/response bodies or Lua-script semantics would require a
1414
minor bump. "Internal signature changes" (e.g. Java method parameters) are
1515
called out but are not breaking to API clients.
1616

17+
## [0.1.25.11] — 2026-04-14
18+
19+
### Added
20+
21+
- Thundering-herd test for idempotency cache expiry. Asserts that N
22+
concurrent retries with the same idempotency key (arriving after the
23+
cache has expired) produce exactly one reservation, not N. Also
24+
verifies metric tags split correctly: 1 × `reason=OK` (the winner)
25+
+ (N-1) × `reason=IDEMPOTENT_REPLAY` (the replays).
26+
- Concurrent-accuracy test for the custom `cycles.reservations.reserve`
27+
counter under 8-thread × 10-request load. Counter count must match
28+
client-observed successes with zero lost increments.
29+
30+
### Wire format
31+
32+
Unchanged. Test-only release. No production-code changes.
33+
34+
### Notes for upgraders
35+
36+
No action required. These tests are regression gates — if you're not
37+
refactoring the reservation path or the metrics component, nothing
38+
changes for you.
39+
1740
## [0.1.25.10] — 2026-04-14
1841

1942
### Added
@@ -179,6 +202,7 @@ Unchanged. Test-only release.
179202

180203
v0.1.x and earlier versions predating this changelog: see `AUDIT.md`.
181204

205+
[0.1.25.11]: https://github.com/runcycles/cycles-server/compare/v0.1.25.10...v0.1.25.11
182206
[0.1.25.10]: https://github.com/runcycles/cycles-server/compare/v0.1.25.9...v0.1.25.10
183207
[0.1.25.9]: https://github.com/runcycles/cycles-server/compare/v0.1.25.8...v0.1.25.9
184208
[0.1.25.8]: https://github.com/runcycles/cycles-server/compare/v0.1.25.7...v0.1.25.8

README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ mvn clean install
117117
./build-all.sh
118118
```
119119

120-
The fat JAR is produced at `cycles-protocol-service-api/target/cycles-protocol-service-api-<version>.jar` (where `<version>` is the `revision` property in `cycles-protocol-service/pom.xml` — e.g. `0.1.25.10`).
120+
The fat JAR is produced at `cycles-protocol-service-api/target/cycles-protocol-service-api-<version>.jar` (where `<version>` is the `revision` property in `cycles-protocol-service/pom.xml` — e.g. `0.1.25.11`).
121121

122122
## Docker Deployment
123123

@@ -136,7 +136,7 @@ Pre-built images are published to GitHub Container Registry on each release:
136136

137137
```
138138
ghcr.io/runcycles/cycles-server:latest
139-
ghcr.io/runcycles/cycles-server:<version> # e.g. 0.1.25.10
139+
ghcr.io/runcycles/cycles-server:<version> # e.g. 0.1.25.11
140140
```
141141

142142
## Testing
@@ -235,7 +235,7 @@ GET /actuator/prometheus
235235

236236
Exposes JVM, HTTP, and Spring Boot metrics in Prometheus format. Both endpoints are unauthenticated. Configure your Prometheus scrape target to `http://<host>:7878/actuator/prometheus`.
237237

238-
#### Domain counters (v0.1.25.10+)
238+
#### Domain counters (v0.1.25.11+)
239239

240240
In addition to Spring Boot's auto-emitted `http_server_requests_seconds`, the service exposes seven domain-level counters under the `cycles_*` namespace (reserve / commit / release / extend / expired / events / overdraft). Operators can alert on denial rates, overdraft incidence, and per-tenant activity without reverse-engineering it from HTTP status codes.
241241

cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/IdempotencyCacheExpiryIntegrationTest.java

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,25 @@
11
package io.runcycles.protocol.api;
22

3+
import io.micrometer.core.instrument.Counter;
4+
import io.micrometer.core.instrument.MeterRegistry;
35
import org.junit.jupiter.api.DisplayName;
46
import org.junit.jupiter.api.Nested;
57
import org.junit.jupiter.api.Test;
8+
import org.springframework.beans.factory.annotation.Autowired;
69
import org.springframework.http.ResponseEntity;
710
import redis.clients.jedis.Jedis;
811

912
import java.util.HashMap;
13+
import java.util.HashSet;
1014
import java.util.Map;
15+
import java.util.Set;
1116
import java.util.UUID;
17+
import java.util.concurrent.ConcurrentLinkedQueue;
18+
import java.util.concurrent.CountDownLatch;
19+
import java.util.concurrent.ExecutorService;
20+
import java.util.concurrent.Executors;
21+
import java.util.concurrent.TimeUnit;
22+
import java.util.concurrent.atomic.AtomicInteger;
1223

1324
import static org.assertj.core.api.Assertions.assertThat;
1425

@@ -34,6 +45,18 @@
3445
@DisplayName("Idempotency cache expiry")
3546
class IdempotencyCacheExpiryIntegrationTest extends BaseIntegrationTest {
3647

48+
@Autowired
49+
private MeterRegistry meterRegistry;
50+
51+
/** Sum of counts across every counter whose tags include the given filters. */
52+
private double counterCount(String name, String... kvs) {
53+
var search = meterRegistry.find(name);
54+
for (int i = 0; i + 1 < kvs.length; i += 2) {
55+
search = search.tag(kvs[i], kvs[i + 1]);
56+
}
57+
return search.counters().stream().mapToDouble(Counter::count).sum();
58+
}
59+
3760
@Nested
3861
@DisplayName("(A) reserve-cache expiry")
3962
class ReserveCacheExpiry {
@@ -128,4 +151,119 @@ void retryAfterCommittedIdempotencyKeyScrubbedReturnsFinalized() {
128151
assertThat(retry.getBody().get("error")).isEqualTo("RESERVATION_FINALIZED");
129152
}
130153
}
154+
155+
@Nested
156+
@DisplayName("(C) thundering-herd retry")
157+
class ThunderingHerd {
158+
159+
/**
160+
* The ops-realistic pattern when an idempotency cache has expired: a client
161+
* that originally timed out fires N concurrent retries with the same key.
162+
* All N arrive at the server within microseconds of each other, all N see
163+
* the idempotency cache gone, all N race through reserve.lua.
164+
*
165+
* Correctness depends on Redis's single-threaded Lua execution serialising
166+
* the calls: the first one to grab the script slot creates the reservation
167+
* and writes the idempotency cache; the rest see the cache and return the
168+
* same id. Without this test, that guarantee is a documented assumption
169+
* only — a future refactor (idempotency logic moved out of Lua into Java,
170+
* for instance) could silently violate it.
171+
*/
172+
@Test
173+
@DisplayName("N concurrent retries after cache expiry produce exactly one reservation")
174+
void concurrentRetriesAfterExpiryProduceOneReservation() throws Exception {
175+
String idempotencyKey = UUID.randomUUID().toString();
176+
int concurrency = 10;
177+
178+
// Prime the cache with a first reserve, then nuke it to simulate post-TTL.
179+
Map<String, Object> body = reservationBody(TENANT_A, 1_000);
180+
body.put("idempotency_key", idempotencyKey);
181+
ResponseEntity<Map> first = post("/v1/reservations", API_KEY_SECRET_A, body);
182+
assertThat(first.getStatusCode().value()).isEqualTo(200);
183+
String firstId = (String) first.getBody().get("reservation_id");
184+
185+
try (Jedis jedis = jedisPool.getResource()) {
186+
String idemKey = "idem:" + TENANT_A + ":reserve:" + idempotencyKey;
187+
jedis.del(idemKey);
188+
jedis.del(idemKey + ":hash");
189+
}
190+
191+
double okBefore = counterCount("cycles.reservations.reserve",
192+
"tenant", TENANT_A, "decision", "ALLOW", "reason", "OK");
193+
double replayBefore = counterCount("cycles.reservations.reserve",
194+
"tenant", TENANT_A, "decision", "ALLOW", "reason", "IDEMPOTENT_REPLAY");
195+
196+
// Fire N retries through the server simultaneously via a CountDownLatch.
197+
ExecutorService exec = Executors.newFixedThreadPool(concurrency);
198+
CountDownLatch start = new CountDownLatch(1);
199+
ConcurrentLinkedQueue<String> returnedIds = new ConcurrentLinkedQueue<>();
200+
ConcurrentLinkedQueue<Integer> statuses = new ConcurrentLinkedQueue<>();
201+
AtomicInteger errors = new AtomicInteger();
202+
CountDownLatch done = new CountDownLatch(concurrency);
203+
204+
for (int i = 0; i < concurrency; i++) {
205+
exec.submit(() -> {
206+
try {
207+
start.await();
208+
ResponseEntity<Map> resp = post("/v1/reservations", API_KEY_SECRET_A, body);
209+
statuses.add(resp.getStatusCode().value());
210+
if (resp.getStatusCode().is2xxSuccessful()) {
211+
String id = (String) resp.getBody().get("reservation_id");
212+
if (id != null) returnedIds.add(id);
213+
}
214+
} catch (Exception e) {
215+
errors.incrementAndGet();
216+
} finally {
217+
done.countDown();
218+
}
219+
});
220+
}
221+
start.countDown();
222+
assertThat(done.await(30, TimeUnit.SECONDS)).isTrue();
223+
exec.shutdown();
224+
225+
// I1: exactly one distinct reservation id returned across all N retries.
226+
// The first call re-creates a NEW reservation (cache was cleared); subsequent
227+
// retries land on the newly-written idempotency cache and replay it.
228+
Set<String> distinct = new HashSet<>(returnedIds);
229+
assertThat(distinct)
230+
.as("N=%d retries produced multiple reservations: %s", concurrency, distinct)
231+
.hasSize(1);
232+
String winningId = distinct.iterator().next();
233+
assertThat(winningId)
234+
.as("new reservation id should differ from pre-expiry id")
235+
.isNotEqualTo(firstId);
236+
237+
// I2: every request got a clean 200.
238+
assertThat(new HashSet<>(statuses)).containsExactly(200);
239+
assertThat(errors.get()).as("no HTTP errors expected").isZero();
240+
241+
// I3: exactly one Redis reservation hash exists for this id (no duplicates).
242+
try (Jedis jedis = jedisPool.getResource()) {
243+
Map<String, String> hash = jedis.hgetAll("reservation:res_" + winningId);
244+
assertThat(hash).isNotEmpty();
245+
assertThat(hash.get("idempotency_key")).isEqualTo(idempotencyKey);
246+
}
247+
248+
// I4: metric tags reflect reality. Exactly one reserve.lua call produced a
249+
// real reservation (reason=OK); the remaining N-1 took the idempotent-replay
250+
// branch (reason=IDEMPOTENT_REPLAY). A wrong-tag regression (e.g. marking
251+
// all N as OK, or all N as REPLAY) would surface here.
252+
double okDelta = counterCount("cycles.reservations.reserve",
253+
"tenant", TENANT_A, "decision", "ALLOW", "reason", "OK") - okBefore;
254+
double replayDelta = counterCount("cycles.reservations.reserve",
255+
"tenant", TENANT_A, "decision", "ALLOW", "reason", "IDEMPOTENT_REPLAY") - replayBefore;
256+
257+
assertThat(okDelta + replayDelta)
258+
.as("total reserve counter increments must equal concurrency (%d); ok=%.0f replay=%.0f",
259+
concurrency, okDelta, replayDelta)
260+
.isEqualTo((double) concurrency);
261+
assertThat(okDelta)
262+
.as("exactly one retry re-created the reservation (OK); rest replayed")
263+
.isEqualTo(1.0);
264+
assertThat(replayDelta)
265+
.as("remaining %d retries must have tagged IDEMPOTENT_REPLAY", concurrency - 1)
266+
.isEqualTo((double) (concurrency - 1));
267+
}
268+
}
131269
}

cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/MetricsCorrectnessIntegrationTest.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,4 +329,53 @@ void concurrentRequestCountIsAccurate() throws Exception {
329329
.as("metric count should match observed successes (%d from threads)", ok.get())
330330
.isEqualTo(ok.get());
331331
}
332+
333+
@Test
334+
@DisplayName("custom cycles.reservations.reserve counter is accurate under concurrent load")
335+
void concurrentCustomCounterIsAccurate() throws Exception {
336+
// Sibling of concurrentRequestCountIsAccurate, but assert on the domain
337+
// counter instead of Spring Boot's HTTP timer. Micrometer counters use
338+
// AtomicLong underneath so this should be safe — the test guards against
339+
// future refactors that might introduce locking or shared-builder races
340+
// (e.g. an aspect that builds tags from a shared mutable map).
341+
try (Jedis jedis = jedisPool.getResource()) {
342+
seedBudget(jedis, TENANT_A, "TOKENS", 100_000_000);
343+
}
344+
345+
double before = counterCount("cycles.reservations.reserve",
346+
"tenant", TENANT_A, "decision", "ALLOW", "reason", "OK");
347+
348+
int threads = 8;
349+
int perThread = 10;
350+
CountDownLatch start = new CountDownLatch(1);
351+
CountDownLatch done = new CountDownLatch(threads);
352+
var exec = Executors.newFixedThreadPool(threads);
353+
AtomicInteger ok = new AtomicInteger();
354+
355+
for (int t = 0; t < threads; t++) {
356+
exec.submit(() -> {
357+
try {
358+
start.await();
359+
for (int i = 0; i < perThread; i++) {
360+
ResponseEntity<Map> resp = post("/v1/reservations", API_KEY_SECRET_A,
361+
reservationBody(TENANT_A, 100));
362+
if (resp.getStatusCode().value() == 200) ok.incrementAndGet();
363+
}
364+
} catch (Exception ignored) {
365+
} finally {
366+
done.countDown();
367+
}
368+
});
369+
}
370+
start.countDown();
371+
assertThat(done.await(30, TimeUnit.SECONDS)).isTrue();
372+
exec.shutdown();
373+
374+
double after = counterCount("cycles.reservations.reserve",
375+
"tenant", TENANT_A, "decision", "ALLOW", "reason", "OK");
376+
assertThat(after - before)
377+
.as("cycles.reservations.reserve counter should match observed successes (%d)",
378+
ok.get())
379+
.isEqualTo((double) ok.get());
380+
}
332381
}

cycles-protocol-service/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
<module>cycles-protocol-service-api</module>
1919
</modules>
2020
<properties>
21-
<revision>0.1.25.10</revision>
21+
<revision>0.1.25.11</revision>
2222
<java.version>21</java.version>
2323
<maven.compiler.source>21</maven.compiler.source>
2424
<maven.compiler.target>21</maven.compiler.target>

docker-compose.full-stack.prod.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ services:
1414
retries: 5
1515

1616
cycles-server:
17-
image: ghcr.io/runcycles/cycles-server:0.1.25.10
17+
image: ghcr.io/runcycles/cycles-server:0.1.25.11
1818
restart: unless-stopped
1919
ports:
2020
- "7878:7878"

docker-compose.prod.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ services:
1414
retries: 5
1515

1616
cycles-server:
17-
image: ghcr.io/runcycles/cycles-server:0.1.25.10
17+
image: ghcr.io/runcycles/cycles-server:0.1.25.11
1818
restart: unless-stopped
1919
ports:
2020
- "7878:7878"

0 commit comments

Comments
 (0)