Skip to content

docs(todo): lock-free load-mode instrumentation profile - #63

Merged
ianp94 merged 2 commits into
mainfrom
docs/lockfree-loadmode-idea
Jul 21, 2026
Merged

docs(todo): lock-free load-mode instrumentation profile#63
ianp94 merged 2 commits into
mainfrom
docs/lockfree-loadmode-idea

Conversation

@ianp94

@ianp94 ianp94 commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Captures the idea you green-lit (2026-07-21): keep instrumentation in play while scaling throughput — closure's thesis applied to load testing.

The problem, proven by the benchmark: load mode drives an Injected target whose valve serializes requests (DD-005/DD-010, so per-request heap/thread deltas are attributable) — which caps concurrency at 1. The k6 10-VU run showing 256ms was 10× queueing behind the lock, not app throughput. Naively uninstrumenting to go fast discards the whole availability oracle → load mode becomes plain k6.

The fix — split instrumentation by what concurrency allows, in load mode only:

  • Drop the serialization lock → real concurrent throughput.
  • Keep concurrency-safe per-request signals (latency, 5xx/crash — each request self-times, no lock).
  • Replace per-request heap/thread deltas (need the lock) with periodic process-global sampling for drift over the soak — exactly what load mode wants, and exactly what the feat(runner): periodic CSV time-series sampler for benchmark capture #59 sampler already does.
  • Net: realistic throughput + latency/5xx findings + heap/thread drift; gives up only per-request heap attribution (explore mode's job).

Composes with clustered runners (#62) and the cost-pheromone corpus (#60) into a distributed cost-guided load engine. Cross-refs DD-005/DD-010, DD-026, the sampler. Design-note-first; likely its own DD. Docs-only.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LXSdVrJQy6DvELDbtptG5p

…-21)

Keeps closure's thesis — measurement boundary in play while throughput
scales — by splitting instrumentation by what concurrency allows in LOAD
mode: drop the DD-005/DD-010 serialization lock (real concurrency), keep
concurrency-safe per-request latency + 5xx/crash, and replace per-request
heap/thread deltas (which need the lock) with periodic global sampling for
drift (the #59 sampler already does this). Load mode gets realistic
throughput + latency/5xx + heap-thread drift, losing only per-request heap
attribution (explore mode's job). Proven-needed by the 2026-07-21 benchmark
(k6 10-VU 256ms was the valve serializing, not app throughput). Composes
with clustered runners + cost-pheromone corpus into a distributed
cost-guided load engine. Design-note first; likely its own DD.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LXSdVrJQy6DvELDbtptG5p
@ianp94

ianp94 commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

@claude please review this docs-only roadmap addition: check the technical claim is sound (latency/5xx are concurrency-safe without the lock; heap/thread deltas need serialization; drift-via-sampling is the load-relevant substitute) and that the DD-005/DD-010/DD-026/#59 cross-references are accurate.

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown

Claude finished @ianp94's task in 4m 5s —— View job


Review: lock-free load-mode instrumentation profile

Core thesis checks out. BasquinValve (tomcat-valve/src/main/java/com/basquin/valve/BasquinValve.java:38,49,84) does take a single global ReentrantLock around every request, so an Injected target really does cap concurrency at 1 — the queueing-not-throughput read of the 10-VU/256ms benchmark is plausible. DD-005 (docs/DESIGN-DECISIONS.md:108-127) and DD-010 (:130-157) say exactly what the entry claims: latency and the leak set are scoped per-IterationContext (confirmed — agent/IterationContext.java has no static/shared fields) and are concurrency-safe today, while heap/thread deltas are process-global and DD-010 explicitly says they "remain trustworthy only under serialized/single-flight execution." DD-026 (:899-905) and LOAD-MODE-DESIGN.md confirm load mode shares the same Injected/valve-instrumented target as explore mode. All four cross-references are accurate.

Two claims are overstated relative to what's actually built today, worth fixing before this becomes a DD:

  1. "Keep... 5xx/crash" implies it already exists — it doesn't. runner/coverage/LoadRun.java:121-141 (fire()) swallows every exception and never inspects the HTTP status code; the method's own doc comment says "a load run measures behavior, not crashes." So 5xx/crash detection in load mode is new work to add, not an existing signal to preserve. Minor, but the DD should scope it as "add," not "keep."

  2. The feat(runner): periodic CSV time-series sampler for benchmark capture #59 cross-reference points at the wrong existing mechanism for lock-free drift. The CSV time-series sampler (runner/util/StatusReporter.java sampleHeader/sampleLine/sampleLoop, tested in test/StatusSamplerTest.java) does sample on its own timer — but the heap columns it emits (lastHeapKb/maxHeapKb) are fed from ctx.heapDeltaBytes (agent/Agent.java:105-110,234-235), i.e. the exact per-iteration begin/end delta that DD-005/DD-010 say needs the lock to be trustworthy. Sampling that value less often doesn't make it safe under concurrency — it's still the same racy delta, just resampled at a coarser cadence.
    The mechanism that's actually already lock-free and process-global today is LoadRun's own heap read (runner/coverage/LoadRun.java:97-98,143-146): a direct Runtime.totalMemory()-freeMemory() snapshot, entirely decoupled from the Agent/valve. That's the real precedent for "periodic process-global sampling," not feat(runner): periodic CSV time-series sampler for benchmark capture #59 — though today it's a single start/end delta in the driver JVM (not periodic, not target-side), matching the "periodic drift sampling" and "target-side heap/thread" items TODO.md already lists as open follow-ups (TODO.md:426). So the right framing is: the idea is sound and the primitive (absolute Runtime reads) already exists and already avoids the lock — but building it into a periodic, target-side sampler is still greenfield, not something feat(runner): periodic CSV time-series sampler for benchmark capture #59 already delivers.

Net assessment: the split-by-concurrency-tolerance design is technically sound and well-grounded in the existing DDs — dropping the lock for real throughput while keeping context-scoped latency and moving heap/thread to periodic absolute sampling is the right shape. Just tighten the two references above (5xx/crash as new work; point the "already does this" claim at LoadRun's direct Runtime reads rather than the #59 CSV sampler, whose current heap columns are lock-dependent) when this graduates to a full DD.

No code changes made — this was a review-only request on a docs-only PR.

Two technical corrections the review caught: (1) 5xx/crash detection is
NEW work in load mode, not a 'keep' — LoadRun.fire() currently swallows
exceptions and ignores status codes. (2) The drift-sampling primitive is
absolute Runtime.totalMemory()-freeMemory() reads (which LoadRun already
does, lock-free/process-global), NOT the #59 sampler's heap columns —
those come from ctx.heapDeltaBytes, the same lock-dependent per-iteration
delta, so resampling it coarsely doesn't make it concurrency-safe. #59
contributes the timer/CSV cadence, not the heap value.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LXSdVrJQy6DvELDbtptG5p
@ianp94

ianp94 commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

Both corrections applied — good catches, they'd have mattered when this graduates to a DD. (1) 5xx/crash reframed as ADD (LoadRun.fire swallows exceptions today). (2) The drift primitive now correctly points at LoadRun's absolute Runtime.totalMemory()-freeMemory() read (lock-free/process-global) rather than the #59 sampler's heap columns — which, as you note, are the lock-dependent ctx.heapDeltaBytes delta just resampled coarsely. #59's contribution is reframed as the timer/CSV cadence, not the heap value; the periodic+target-side sampler is the greenfield work.

@ianp94
ianp94 merged commit 6605666 into main Jul 21, 2026
@ianp94
ianp94 deleted the docs/lockfree-loadmode-idea branch July 21, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant