feat: lock-free load-mode instrumentation profile (DD-029)

[basquin-bot]

Implements DD-029 (design #68) — closure's thesis for load: keep instrumentation in play while throughput scales. One PR, built as four task-by-task commits.

Problem (benchmark-proven): a mode: load campaign drives an Injected target whose valve serializes every request, capping concurrency at 1 (k6 10-VU = 256ms of queueing, not throughput).

What ships:

• agent.LoadMode — a flag the valve reads: explore = serialized (unchanged); load = lock-free. TTL auto-reverts so a dead driver can't strand a target. driftSnapshotCsv() = absolute Runtime heap + thread count (not the lock-dependent per-iteration delta).
• Valve two-state — intercepts /__basquin/* control requests (mode toggle / drift) on the app's own port (no new listener), then branches explore/load. Explore path untouched; verified namespace-free (0 servlet refs, DD-011).
• LoadRun — toggles the target lock-free for the run, polls /__basquin/drift for the app's real heap/thread drift (first→last), counts 5xx (fire() now returns status instead of swallowing — the docs(todo): lock-free load-mode instrumentation profile #63 gap), reverts on exit, adds serverErrors to status.load.
• e2e — asserts the two-state valve serves /__basquin/drift in-cluster (the mechanism; a hard throughput number would flake on kind's CPU).
• Docs — DD-029 → implemented; OPERATOR-USAGE load section + the /__basquin in-cluster-trust caveat; changelogs.

Resolved the design's open-Q #2 by serving control+drift via valve interception on the app port — no agent HTTP server, no agent sampler thread, no operator change (per the plan #69 note you approved).

Unit-tested (LoadModeTest, LoadModeControlTest, LoadRunDriftTest); full suite green. Foundational for the pheromone + clustered-runner roadmap. Deferred: multi-driver mode coordination, back-pressure.

🤖 authored by basquin-bot[bot] with Claude Code

[ianp94]

@claude please review this PR: the DD-029 lock-free load-mode implementation. Focus on: the valve two-state correctness (explore path truly unchanged; load passthrough re-raises the checked ServletException namespace-free; control requests never reach the app or the lock); LoadMode TTL/auto-revert thread-safety; LoadRun's mode toggle + target-side drift polling + 5xx counting; and whether the /__basquin control surface has any collision or trust hole beyond the documented in-cluster caveat.

[claude]

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

---

Review: DD-029 lock-free load-mode implementation

Todo list

• Gather context (diff, changed files, DD-029 design doc)
• Review valve two-state correctness (explore path unchanged, checked-exception passthrough, control interception)
• Review LoadMode TTL/auto-revert thread-safety
• Review LoadRun mode toggle, drift polling, 5xx counting
• Assess /__basquin control surface for collision/trust holes
• Post final review

CI is green (CI, arm64 smoke); Operator e2e (kind) is still in progress at review time.

---

Valve two-state (tomcat-valve/.../BasquinValve.java)

• Explore path is byte-for-byte unchanged. Diffed against origin/main: the only change is the two new blocks inserted above the existing ITERATION_LOCK.lock() section; nothing inside it moved. Confirmed.
• Control requests never reach the lock or the app. LoadModeControl.handle(...) is called unconditionally first, and any /__basquin/* path is handled (never returns null) so a malformed/unknown sub-path can't fall through to getNext().invoke(...). unknownControlPathIsHandledNotPassedThrough covers this.
• Namespace-free passthrough is correct. The load branch's catch narrows to IOException/RuntimeException/Error and falls back to sneakyThrow for the checked ServletException case, matching the existing explore-path pattern exactly — no javax.servlet/jakarta.servlet type is named. response.getWriter() in the control branch is also fine: called on the concrete Catalina Response type, so it doesn't pull a servlet interface into the bytecode signature either.

LoadMode TTL/auto-revert

• setLoad() writes enteredAt/expiresAt before the load volatile flag — so a racing reader can never observe load=true paired with a stale/uninitialized expiresAt. Ordering is correct.
• Auto-revert (isLoad()'s now < expiresAt check) is a pure function of volatile reads, no torn state possible. Unit tests (loadUntilTtlThenAutoReverts) exercise the boundary correctly.

LoadRun

• Mode toggle (setTargetMode), baseline drift capture (AtomicBoolean.compareAndSet guarding exactly one baseline poll across N worker threads), and drift delta (last - first, null-safe) all look correct and are covered by LoadRunDriftTest.
• 5xx counting: fire() now returns the status and serverError increments on code >= 500 — closes the docs(todo): lock-free load-mode instrumentation profile #63 gap as described. Transport failures (code == -1) are correctly excluded from the 5xx count (they're a different signal), though note they're also not surfaced anywhere in the output JSON — only affect throughput/latency counts implicitly. Not a blocker, just worth knowing if silent connection failures need their own visibility later.

Findings

1. /__basquin/mode has no method check and no TTL ceiling — a real trust hole beyond the documented caveat.
LoadModeControl.handle (agent/LoadModeControl.java:25-42) doesn't check the HTTP method, and ttl() (agent/LoadModeControl.java:44-47) parses ttlMs from the query string with no upper bound (and no rejection of negative/absurd values beyond a parse failure). Combined, any request that reaches the app's port — a plain GET /__basquin/mode?to=load&ttlMs=99999999999999, e.g. from a health-check bot, browser prefetch, crawler, or a neighboring pod in a shared cluster — can force the target permanently lock-free, silently disabling explore-mode's per-request invariant serialization for that target indefinitely. The OPERATOR-USAGE caveat documents "don't expose the port outside the cluster," but doesn't cover this in-cluster case: any caller that can already reach the app port (which is a much larger set than just the intended driver) has an unauthenticated, GET-triggerable, unbounded-duration mode switch. Recommend: cap ttlMs server-side (e.g. clamp to a sane max like 5-10 minutes) and require POST for the mode sub-route (checked in BasquinValve.invoke before delegating, or inside LoadModeControl.handle given a request-method parameter).

Fix this →

2. Minor: explore/load transition boundary. When LoadRun reverts the target to explore at run end, in-flight load-mode (unlocked) requests can still be executing concurrently with the first newly-serialized explore-mode requests, since nothing waits for the lock-free in-flight requests to drain before the lock resumes serializing new ones. This means the per-request heap/thread deltas for the first explore requests immediately after a load campaign could be polluted by a straggling load-mode request finishing at the same time. Self-heals after the stragglers drain, and is a narrow window, but worth a one-line callout in the design doc if it isn't already implicitly accepted.

3. Minor/non-blocking: the DD-011 "0 servlet refs" namespace-free property is verified manually per the PR description but has no automated regression check (e.g. a javap/bytecode-scan CI step) — a future change could reintroduce a servlet-type reference without anything catching it. Pre-existing gap, not introduced by this PR, just flagging since the PR explicitly leans on that guarantee.

Summary

The core two-state valve logic, TTL auto-revert, and drift/5xx plumbing are all correct and well-tested (LoadModeTest, LoadModeControlTest, LoadRunDriftTest). The one finding worth addressing before merge is #1 — the unauthenticated, method-agnostic, unbounded-TTL mode toggle is a materially bigger trust hole than "don't expose the port externally" covers, since it's exploitable by anything with in-cluster network reach to the app, not just the intended driver.
· branch feat/lockfree-loadmode