feat(personhog): e2e test harness for ci - #70797
Merged
Merged
Conversation
Pods that register with an explicit host:port name resolve as-is, letting multiple local leaders listen on distinct ports. Bare pod names keep resolving via DNS on the fleet-wide leader port, so k8s deployments are unaffected.
Revives the personhog-cannon draft (#55581) as a correctness gate for the leader path. The gate command spawns an isolated stack (replica, writer, N leaders, leader-mode router) against the docker-compose Kafka/etcd/Postgres, seeds persons in SQL, drives concurrent update traffic while journaling every acked write under a unique key, then asserts acked writes are visible in strong reads and land in Postgres at exactly the acked version. Standalone seed/cleanup/blast/consistency subcommands target a running stack. SQL seeding is the interim mechanism until the create RPC's future is settled; src/seed.rs is the swap seam.
Disruptions fire mid-traffic against the spawned stack: kill the busiest leader (SIGKILL, with optional etcd lease revoke for instant coordinator detection), graceful shutdown (SIGTERM + drain while traffic flows), and scale-up (spawn another leader). Killed and draining leaders are retired from liveness checks since their exit is the point. Also adds --cache-capacity to put the leader cache under eviction pressure; below the seeded person count this currently loses acked writes (the known cache-eviction hazard), so the gate goes red — which is the harness doing its job.
New gate disruptions: zombie (SIGSTOP the busiest leader and revoke its lease so ownership moves while the process still holds its old cache and producer, then SIGCONT it), same-identity leader crash-restart (re-register and converge on owned partitions), writer crash-restart (at-least-once redelivery under the version guard) and writer pause/resume (controlled lag injection), multi-router stacks with a coordinator kill (traffic targets the last router; the first — the election winner — dies and a later handoff must run under the new coordinator), and a best-effort mid-handoff kill of a handoff's target pod. Also asserts version monotonicity throughout: an ack or strong read observing a version below an earlier ack for the same person is a violation.
Boots Postgres/Kafka/etcd from compose, builds the personhog stack, and runs the cannon gate twice: a plain run and a chaos run (leader kill + scale-up mid-traffic). Asserts every acked write is visible in strong reads and lands in Postgres at the acked version. Not a required check while it soaks.
Three gaps from a final review pass: a stack that failed every write passed the gate vacuously (acked-implies-visible holds with zero acks), so zero successful writes now fails the run; an unreadable or missing person during strong-read verification was logged but not counted, so NotFound and persistent read errors (after one retry) are now violations; and Postgres verification hardcoded posthog_person, which false-fails against stacks whose writer targets another table — the new --pg-target-table flag threads through the spawned writer, verification, and cleanup (the dev stack uses personhog_person_tmp).
…nnon The gate reproduces two real leader-path bugs; document their signatures so red or noisy runs read as signal: eviction under writer lag (gate goes red) and the unordered lifecycle shutdown that black-holes a draining leader's partitions (elevated Failed count, gate stays green since nothing was acked). Also explain the two deliberate seeding asymmetries: no team seeding exists because the persons database has no team table or team_id foreign key, and cleanup deletes distinct-id rows nothing writes yet so RPC-based seeding can swap in without leaking.
The tool grew from a load generator into a full simulation harness — stack orchestration, chaos disruptions, and correctness gating — so the name should say so. Renames the crate, binary, CI references, and run-scoped identifiers (etcd prefix, topic and consumer-group names, log directory, property prefixes), keeping only the historical reference to the original cannon draft. Also hoists the remaining inline fully-qualified paths into top-of-file imports.
Nothing writes distinct-id rows today — traffic is id-keyed and SQL seeding creates bare person rows — and whether RPC-based seeding will write them is not yet decided, so cleaning them was speculative. Cleanup now matches what actually gets written; the distinct-id delete comes back with whatever mechanism first writes those rows.
A TTL-expiry kill (--kill-fast false) previously needed a 30s+ outage window mid-run because spawned leaders used the production lease default. --leader-lease-ttl (default still 30) shortens the lease, with the heartbeat interval scaled to a third of it, so the scenario fits a 20s run. Exercised for the first time at a 5s lease: the coordinator stays blind until expiry, the victim's partitions fail hard for the full window (332 unacked failures vs 67 with lease revoke), then reassignment recovers and every acked write verifies.
The e2e gate exercises the verifier on every run but can only reveal false positives — a verifier that misses violations looks identical to a healthy stack — so the decision tables get unit tests: property verification (present/mismatched/missing/non-object), journal version tracking and ack-regression flagging, strong-read version checks, the SQL-interpolation table-name guard, and chaos timeline ordering (including stop/resume pairing).
The chaos step only exercised kill + scale-up. Three composite runs (each rehearsed locally) extend coverage to every validated scenario: drain + zombie under writer lag, TTL-expiry kill with writer crash and same-identity restart, and coordinator failover with a mid-handoff target kill. Also runs the harness's own unit tests in the job, since the crate sits outside the main test matrix.
The consistency check existed only as a standalone subcommand, so the gate never asserted recency — blast verification reads once at the end and cannot see a staleness window that heals before then. The gate now runs read-your-write probers (--probers, default 2) alongside the blast traffic: each repeatedly writes a unique key and immediately strong-reads it back, through whatever chaos fires. Probe acks are journaled so end-of-run checks cover their durability too; failed reads are not recency violations (owners die mid-chaos), but a served read missing the freshest write is. The probers' first outing exposed nondeterminism in the coordinator kill: it revoked the registration lease but not the election lease, so when the victim actually held the election, no coordinator existed for the 15s election TTL and the scenario silently degenerated. Bring-up now waits for the first router to win the election before spawning the rest, and the kill revokes the election lease when the victim holds it, making failover immediate and the scenario deterministic — verified with three consecutive green runs.
A crashed coordinator blocks handoffs for the 10-20s election window (lease TTL plus campaign retry). Leader crashes are usually masked by their own 30s discovery lease, but drains during the window stall — compounding with the unordered-shutdown defect into a partition outage. The gate's coordinator-kill scenario deliberately revokes the election lease for determinism, so the slow-failover path is documented as untested until the shutdown ordering is fixed.
z0br0wn
changed the title
Contributor
|
Reviews (1): Last reviewed commit: "docs(personhog): document the coordinato..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Semgrep blocks current_exe (its output is attacker-influenceable). The default bin dir is now the workspace target directory for the build profile, derived from the crate location at compile time — the harness only ever runs sibling binaries from the same cargo build, and --bin-dir overrides for anything else.
An update response without a person body was counted as a successful write but never journaled, so strong-read and Postgres verification silently skipped that acked write — and the vacuous-pass guard still saw a success. Such acks now journal their keys (without touching the person's version high-water mark, since no version came back) and flag the broken response contract as a violation in the gate, the probers, and the standalone consistency check.
nickbest-ph
reviewed
Jul 14, 2026
Contributor
|
Great! Lgtm. This is gonna be awesome to have. Might be worth considering AGENT context and a skill somewhere that tells agents when and how to add scenarios here after it fixes protocol bugs/regressions in the service. |
nickbest-ph
approved these changes
Jul 14, 2026
The spawned leaders used the production default writer consumer group, so their committed-offset queries (warming ranges, dirty-index pruning) silently saw no writer progress; leaders now point at the harness writer's group. Blast write failures also log the full anyhow chain — the outer context alone hid the gRPC status that made an instant-failure burst diagnosable.
The gate job ran on every rust PR. A determinator package-rule now marks the harness affected whenever a personhog service it spawns changes — a runtime dependency the cargo graph cannot see — and the job condition checks the affected-crates output, so unrelated rust PRs skip the job entirely. Verified with affected-services directly: a personhog-leader change marks the harness, an unrelated crate does not.
Teaches agents when a personhog fix earns a harness scenario, how to add chaos events and invariants (flags, timeline, stack primitives, journal verifiers), the determinism bar for CI inclusion, and the red-before/green-after validation convention for fixes.
The gate journaled every ack and flagged any version arriving below the person's high-water mark. The leader serializes writes per person, but concurrent clients record their acks in response-arrival order, so a slow response for an earlier version lands after later ones and trips a phantom regression — a timing lottery that CI's slower runners lost. The journal now checks the invariant that holds regardless of ack ordering: each version of a person is assigned to at most one acked write. A duplicate still catches real double-assigns (a stale warm, stale fallback, or zombie leader serving two writes from the same base state), while out-of-order arrivals of distinct versions are benign. The high-water mark stays a running max, which the strong-read and Postgres version floors already relied on.
A write that produced its changelog record but lost the response (a drain killing the connection, a client timeout) is unacked to the harness yet still applied by the writer, so a quiesced row legitimately sits above the highest acked version when such a write was the person's last. Exact equality flagged that as a violation; only a row below the floor means an acked write never reached Postgres. Acked-data completeness is still asserted independently by the per-key property checks, and stale double-assigns by the duplicate-version check.
…ation A drain overlapping a pod death can wedge convergence for the drained pod's full lifecycle timeout: the re-drive rebalance still counts the draining pod as an assignment target, its handoffs stall with no DrainedAck (coordination gets a 5s grace while the pod stays registered for 30s), and rebalancing is globally deferred while any handoff is in flight. Verification previously raced that window and flagged the black-holed partitions as strong-read violations. The gate now waits for convergence — no handoffs in flight and every partition owned by a live process — before verifying, bounded at 90s so a never-converging protocol becomes a deterministic, labeled failure instead of a probabilistic one. An already-settled run waits zero time. The wedge is documented as known defect #4 in the README; the ordered-shutdown fix collapses the settle time to near-zero, at which point the deadline can tighten to re-gate on recovery speed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The personhog leader write path (router → leader → changelog → writer → Postgres) has per-crate unit and integration tests, but nothing exercises the full five-service stack under real traffic, and nothing at all exercises it under failure: leader crashes, graceful drains, zombie processes, writer lag, coordinator loss. The property the whole design hangs on is simple to state and was previously unverified end to end: every write acked by the leader path must be visible afterwards – in strong reads immediately, and in Postgres at exactly the acked version once the writer drains.
Changes
New
rust/personhog-test-harnesscrate (reviving the earlier personhog-cannon draft, #55581) plus a CI job that runs it. Thegatecommand spawns a fully isolated local stack against the docker-compose Postgres/Kafka/etcd, seeds persons, drives traffic, disturbs the stack mid-run, then verifies and tears down. Isolation is by port range, etcd prefix, and a per-run changelog topic, so it runs alongside a dev stack.flowchart LR H{{test harness}} -->|spawn + chaos| R H -->|seed persons SQL| PG H -->|blast writes + probe reads| R subgraph S[isolated stack] R[leader-mode routers] --> L[leaders] L --> K[(changelog topic)] K --> W[writer] E[(etcd)] --- R E --- L end W --> PG[(posthog_persons)] H -->|verify strong reads + rows/versions| PG classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000; classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000; class R,L,W phBlue; class H phYellow; class K,E,PG phGray;Three verification layers share one acked-write journal (every ack recorded under a globally unique property key):
The gate exits non-zero on any violation, and also on zero acked writes (a broken-but-reachable stack must not pass vacuously).
Chaos scenarios, all validated green locally:
--kill-after(+ fast lease revoke)--kill-fast false --leader-lease-ttl--shutdown-after--scale-up-after--zombie-after(SIGSTOP + lease revoke, later SIGCONT)--restart-after--writer-crash-after,--writer-pause-after--routers 2 --router-kill-after--kill-handoff-target--cache-capacityOne production change rides along: the router's leader address resolver now dials
host:portpod names as-is (bare names keep resolving via DNS on the fleet-wide leader port), which is what lets N local leaders listen on distinct ports. K8s behavior is unchanged.The
personhog-gateCI job boots only db/kafka/etcd from compose, migrates the persons DB with sqlx directly, and runs five gate configurations (plain + four chaos composites). It is a new job name, so it is not a required check while it soaks. On failure it uploads every service's log as an artifact.Note
The harness surfaced three real leader-path defects, documented with signatures and fix directions in the crate README: cache eviction under writer lag loses acked writes (gate goes red under
--cache-capacitypressure), the leader's unordered lifecycle shutdown black-holes a draining pod's partitions (~1%failed writes on drain runs, gate stays green since nothing acked is lost), and a crashed coordinator blocks all handoffs for the 10–20s election window. The eviction run is intentionally excluded from CI until fixed.
How did you test this code?
cargo clippy -D warnings,cargo fmt, and the affected-services integration test (which the new crate registers with) all pass.Docs update
The crate README documents usage, every chaos flag, and the known-defects catalog. Internal dev tooling – suggest the
skip-inkeep-docslabel.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built with Claude Code. Skills invoked along the way:
/writing-tests(gating which unit tests earn their place). Key decisions, in rough order: revived the personhog-cannon draft rather than starting fresh, but dropped itscomfy-table/owo-colorsdependencies (new to the workspace) for plain formatting.Chose the
host:portpod-name resolver tweak over adding an advertise-address field to the registration schema (smaller, backward compatible). Seeding is direct SQL for now because the create RPC's future is still being settled –src/seed.rsis the documented swap seam. The read-your-write probers were added late and immediately exposed that the coordinator-kill scenario was nondeterministic (initial election winner is a race), which led to the deterministic bring-up (first router must win the election) plus election-lease revocation on kill, and to documenting the coordinator failover window as a third known defect. A toxiproxy-based network-fault layer (asymmetric partitions, etcd watch latency) was scoped and deliberately deferred.Two notes on it: the "three defects" callout is written code-facing (mechanisms and fix directions from this repo only, no operational numbers beyond harness-scale measurements), per the public-repo rules. And per the template's draft-by-default rule, when you're ready I'd push and open it as a draft with you assigned as DRI.