diff --git a/TASK-BRIEF.md b/TASK-BRIEF.md new file mode 100644 index 0000000..c89f2fb --- /dev/null +++ b/TASK-BRIEF.md @@ -0,0 +1,249 @@ +# TASK BRIEF — ukmesh MQTT ingestion: performance investigation & optimization PR + +Session: `ukmesh-ingest-perf` · Host: projects-vps (`vps-0e251c4d`, 57.129.131.35) · Opened: 2026-10-11 by Hermes (infra lane) +Reviewer: Ben — he reviews the PR; nothing gets deployed. Operator (Hermes) collects your final report. + +**Before anything else: reason thoroughly and design fully before writing code.** Then execute autonomously end-to-end per this brief and the original task in Appendix A. Update §8 PROGRESS LOG at every phase boundary — the operator reads it for check-ins. + +--- + +## 0. Mission + +Investigate the MQTT ingestion pipeline of this repo at current HEAD, verify-or-refute six claimed findings with first-hand evidence (code + measurements), build a reproducible **synthetic** benchmark on an **isolated** stack, implement the **smallest maintainable** optimizations that measurably improve resource efficiency, and open ONE pull request to `gadgethd/ukmesh` from branch `perf/mqtt-ingest-20261011` — **left unmerged** — containing the full evidence package (8 required sections, see §7). + +Done means: (a) PR open with all eight required sections; (b) baseline-vs-optimized benchmark results with raw outputs preserved on disk; (c) all tests green, CI green on the PR; (d) `FINAL-REPORT.md` written; (e) final message = short summary + PR URL + headline numbers. + +**The live UKMesh production must remain completely untouched. This is pass/fail.** + +--- + +## 1. Ground rules — ABSOLUTE + +### Production is off-limits (read the whole list before starting) +- **Never** modify / restart / recreate / stop / `exec` / `logs` / `inspect` ANY production container. Production names: anything starting `meshcore-analytics-` or `meshcore-infra-` (incl. `meshcore-infra-timescaledb-1`, `meshcore-infra-mosquitto-1`, `meshcore-analytics-backend-1`), plus compose projects `meshcore-analytics`, `meshcore-infra`, `meshcore-beacon-ukmesh`, `meshcore-health-check`. Do not run ANY docker command against them. +- **Never** touch production databases (`meshcore`, `beacon_ukmesh`), their volumes, or the production Mosquitto config/logs. +- **Never** touch `~/ukmesh/meshcore-analytics` — the deployed checkout with uncommitted deploy lineage. Not yours. All repo work happens ONLY in **this worktree**: `/home/ben/ukmesh/.agent-worktrees/mqtt-ingest-perf-20261011`. +- **Never** touch `~/ukmesh/meshcore-infra/` (prod compose + `.env`), prod cron jobs, or any other directory under `~/ukmesh/` besides your worktree and `~/ukmesh-bench/`. +- **Never** restart ANY service on this host. If you notice prod trouble, note it in PROGRESS and do not fix it. +- **Never** run load tests / packet replays / benchmarks against any production endpoint. Synthetic traffic only, isolated stack only. +- **Never** subscribe to or publish to the production broker. The bench broker is yours and isolated. +- **No deleting other people's stuff:** no `docker system prune`, no `docker volume rm`, no `docker rmi` of images you didn't build for the bench. Cleanup only with your own bench compose project (`docker compose -p mqttbench … down --volumes` from your bench dir). No `pkill`/pattern-kills anywhere — track PIDs. +- Read-only production inspection that IS allowed (task explicitly permits already-available read-only metrics): `curl` against the read-only Prometheus/metrics endpoints if reachable (e.g. production backend `:9091/metrics`, VPS Prometheus `:9092`). No prod config changes to enable anything. + +### GitHub rules +- **Allowed:** `git push origin perf/mqtt-ingest-20261011`; open the PR to `main` via `/home/ben/bin/gh`; watch CI checks; edit the PR body. +- **Forbidden:** merging the PR; pushing any other branch; creating tags or releases; `workflow_dispatch`; editing `.github/workflows/**`; touching repo settings; printing secrets. +- **Deploy-gate (verified by operator 2026-10-11; re-confirm cheaply by reading the workflows at your branch):** `.github/workflows/release.yml` fires only on `release: [published]` + `workflow_dispatch` → a branch push / PR cannot trigger it; `ci.yml` runs tests on push + PR entirely inside GitHub runners (isolated smoke stack, torn down); the actual production deploy is a manual VPS procedure that you must not perform. If you find anything that contradicts this, **stop before pushing** and report. +- **Git identity:** the worktree is already configured (`gadgethd <111318106+gadgethd@users.noreply.github.com>`), a previously-used identity for this repo's agent branches. `gh` is at `/home/ben/bin/gh` (not on PATH — use absolute path or `export PATH="$HOME/bin:$PATH"`). Push auth is verified working. + +### Resource caps (shared host) +- Give every bench container cgroup limits so production keeps headroom (suggest: timescaledb 2 cpu / 2 GB; backend 3 cpu / 1.5 GB; mosquitto and publisher minimal). Cap the whole bench footprint ≈ ≤4 vCPU burst / ≤4 GB. +- Never bind a production port. Taken (verify with `ss -tln`): 53, 1883, 3000-3010, 3080, 3090, 4174, 5432, 6379, 8080-8099, 8923, 9001, 9092, 13000, 20241-20245, 32720, 35809, 36667, 37855+, 39999, 40504. Suggested bench ports: **25432** (pg), **21883** (mqtt tcp), **29001** (mqtt ws), **23000** (backend http), **29091** (metrics) — re-verify free first. +- It is currently ~01:00–03:00 local (BST, UTC+1) — the quiet window. Prefer heavy runs now; if you slip into the UK day, keep caps in place anyway. +- Never embed secrets anywhere; the bench broker gets its own throwaway credentials (see CI bootstrap pattern in §2). + +--- + +## 2. Environment (verified 2026-10-11 by operator) + +- **Worktree:** `/home/ben/ukmesh/.agent-worktrees/mqtt-ingest-perf-20261011` — branch `perf/mqtt-ingest-20261011` tracking `origin/main` @ `fa801dc` (clean; `HEAD == origin/main`). This is your cwd. +- **Bench dir** (your scratch; keep everything not meant for the repo here): `/home/ben/ukmesh-bench/mqtt-ingest-perf-20261011/` +- **Host:** 8 vCPU, ~23 GB RAM (~17 GB available), 61 GB free on `/`. Docker + Compose v5.4.0. Node v22.22.1 on the host. +- **Pinned digests to reuse (from `.github/workflows/ci.yml`):** timescaledb `sha256:22e8a5ae7aef121d1537afe946dd7cc5deeeb63ab36ce19849d671bd3b663509`, redis `sha256:8b81dd37ff027bec4e516d41acfbe9fe2460070dc6d4a4570a2ac5b9d59df065`, mosquitto `eclipse-mosquitto@sha256:9cfdd46ad59f3e3e5f592f6baf57ab23e1ad00605509d0f5c1e9b179c5314d87` (verify exact strings in ci.yml). +- **Proven recipes to adapt (do not reinvent):** + - `scripts/ci-smoke-repro.sh` — fresh-DB stack recipe (migrate with retry, CI-shaped boot, 150 s cap). + - CI "Workers and Compose" job in `.github/workflows/ci.yml` — mosquitto creds bootstrap (`scripts/bootstrap-mosquitto.sh` + `mosquitto_passwd` via the mosquitto image), ACL lines, `db-migrate` service, readiness/WS fanout checks (great template for your bench smoke test + WS-preservation check). + - `docker-compose.ci.yml` — CI overrides for the stack; `docker-compose.yml` has the service definitions (backend env at ~L60-125). +- **Bench stack shape (suggested):** compose project `mqttbench` (own network `mqttbench_default`, own volumes), services: `timescaledb` (pinned image, `shm_size: 512mb`+), `mosquitto` (pinned image, your own passwd/acl, listeners on bench ports), `db-migrate` (from your backend image), `backend` (built from THIS worktree's `Dockerfile.backend`, `SOURCE_REVISION=`, env pointing at bench DB/broker only), publisher (run from the backend image or a minimal node image — `mqtt` is already a backend dependency). +- **Test traffic shape:** publish to realistic topics `meshcore///packets|status|neighbors` (the isolated DB means you can use the full-fidelity non-test path; `meshcore-test/...` routes to network `'test'` scope and SKIPS some SQL sub-writes — prefer the full path for fidelity, note the difference). Craft packet payloads with valid `raw` hex so the decode path is exercised (borrow samples from `backend/src/mqtt/decodePacket.test.ts`, other tests, and CI E2E payloads; you may also generate via `@michaelhart/meshcore-decoder` if it supports it). Status envelopes must satisfy `statusEnvelopeTargetsObserver` (see `identityBinding.ts`) or they're rejected — read it before crafting status messages. +- **Operator-local reference files (for context, may be stale vs your HEAD):** none needed — read the code. + +--- + +## 3. Operator pre-verification (2026-10-11) — re-verify EVERY item yourself; cite `file:line` + measurements. Do not trust; confirm or refute. + +**F1 — MQTT.js serializes processing; the durable handler holds the flow. [CONFIRMED at source level by operator; re-confirm on the installed build]** +- `backend/src/mqtt/durableHandler.ts` (whole file): `handleMessage` override calls the MQTT.js callback **only after** `persist()` resolves; on failure it reports + closes the stream (`client.ts:311-319`, via `mqttClient.stream.destroy()`) so the broker redelivers. +- `backend/src/mqtt/client.ts:346` assigns it onto `client.handleMessage`. +- MQTT.js **5.16.0** (pinned in `backend/package-lock.json`): see installed `node_modules/mqtt` after `npm ci` (`backend`). In `src/lib/client.ts` `connect()` (~L988-1105): each TCP chunk is fully parsed into a `packets[]` queue, then `work()` handles **one** packet; the `done` handed to handlers (`nextTickWork`, L1083-1098) advances the queue only when called. For QoS1, `src/lib/handlers/publish.ts` (case 1, ~L280-315) sends PUBACK only inside `handleMessage`'s callback and passes that same `done` to the PUBACK write — so the pump advances only after the PUBACK is actually written. The library's own doc comment on `handleMessage`: *"Handle messages with backpressure support, one at a time. Override at will."* (client.ts:842-849). +- **Consequences to verify empirically:** ingestion is strictly one message at a time end-to-end; `enqueuePacket` is called once per pump step (only caller chain: `client.ts:755` → `db/index.ts:633,677` → `packetBatch.ts:147`), so the batch NEVER coalesces multiple packets; the 50 ms flush timer (`packetBatch.ts:15,123-132`) therefore adds pure latency to EVERY packet message — expect `meshcore_packet_batch_size` histogram ≈ all-1s on the baseline at every rate. Sustainable per-message rate ≈ 1/(50 ms + decode + write) ≈ ≤ ~20/s from the timer alone, regardless of DB capacity. If you observe larger batches, my model is wrong — re-investigate before designing anything. + +**F2 — batch utilization:** consequence of F1 + single producer. Confirm via the histogram (baseline burst scenario) and code. + +**F3 — single active flush:** `packetBatch.ts:539` (`if (activeFlush) return activeFlush`) + drain loop L541-550. Note what this actually means under F1 (pending[] only ever holds the current message) and what changes if/when multi-pending is possible. + +**F4 — dead config:** `MQTT_INGEST_CONCURRENCY` (docker-compose.yml:119, .env.example:39) and `MQTT_INGEST_QUEUE_MAX` (docker-compose.yml:121, .env.example:40) are **read by nothing** in `backend/src` (grep: only `MQTT_INGEST_ENABLED` is read, `index.ts:69-70`). Leftovers from the pre-durable design. Repair options: remove, or repurpose (document choice + compatibility). + +**F5 — pool utilization:** OLTP pool max = `DATABASE_POOL_MAX` (compose default 16; `platform/config/database.ts:29-35` default 8; analytics pool max 2, `db/index.ts:53-57`). Ingest commits use ONE connection at a time **by design** (serial pipeline). Measure actual concurrent connections per scenario (`pg_stat_activity`) and state plainly that spare connections cannot accelerate a serialized pipeline (they serve API/WS/stats in parallel). + +**F6 — SQL shape:** the per-batch statement is one big CTE with up to **8 write targets** — `packets`, `packet_paths`, `nodes` (observer upsert), `node_network_sightings`, `packet_daily_stats`, `packet_hourly_stats`, `observer_region_packet_sightings`, `observer_region_observer_sightings` — plus materialized CTEs incl. `FOR KEY SHARE` on `public_visibility_state` and a `privacy_rematerialization_queue` EXISTS check (`packetBatch.ts:243-481`). At 1-row batches the per-execution overhead matters: enable **pg_stat_statements** in the BENCH db (preload + `track_planning=on`-equivalent; set `pg_stat_statements.track_planning = on` if the background worker approach is awkward — a plain `ALTER SYSTEM`+restart of the bench pg is fine) and record `total_plan_time` vs `total_exec_time` per call for this statement. This measurement decides whether prepared statements / restructuring are worth it. + +**Existing metrics inventory (backend :9091 `/metrics`):** `meshcore_mqtt_messages_total`, `meshcore_mqtt_ingest_outcomes_total{outcome}`, `meshcore_mqtt_ingest_active`, `meshcore_packet_batch_flush_total{outcome}`, `meshcore_packet_batch_retries_total`, `meshcore_packet_batch_size` (histogram), `meshcore_packet_batch_duration_seconds` (histogram) — `backend/src/metrics.ts`. Use these in the bench; §6.6 adds phase-timing metrics. + +**Per-message serial path (profile it):** decode/parse (`decodePacketCompat`, `buildSummary`, channel keyStore) → advert crypto (`verifyAdvertSignature`) where applicable → dedup check → `insertPacket` → batch commit → post-commit `emit()` subscriber chain (`broadcastPacket` WS fanout, owner packet share, link queue — `index.ts:222-247`) → ack write. Everything in that chain delays the PUBACK. Quantify where wall time goes before optimizing anything. + +--- + +## 4. Design thinking — decided constraints + evidence-gated candidates + +**The hard constraint (state it in the PR):** QoS1 PUBACK must not be sent before its persistence commits — and in MQTT.js 5.x the pump's flow control is coupled to that same callback (see F1). So **cross-message batching/concurrency is impossible without either (a) breaking the library's callback contract, or (b) internals monkey-patching** (e.g. intercepting `_sendPacket` to defer PUBACK writes) — both fragile, both against the spirit of "design around the constraint rather than bypassing durability guarantees". **Do NOT ship any internals hack in this PR.** If you believe a *bounded, low-fragility* alternative exists after your own investigation, present it in `FINAL-REPORT.md` as a proposal with risks — do not ship it. The PR's job: make the necessarily-serial path as cheap and low-latency as possible, and prove the serialization claim with evidence so the reviewer understands why batching isn't the answer here. + +**Candidates (keep/drop by measurement — implement the smallest set with meaningful, demonstrated gains):** + +1. **Immediate flush on the serial path [prime candidate].** Under F1 a message can never coalesce with a later one, so the flat 50 ms wait is pure latency on every packet. Flush on next-tick when an item is enqueued and no flush is active (keep `MAX_BATCH_SIZE` + drain-loop semantics for any multi-pending path, keep retry logic untouched). Make timing behavior configurable with safe defaults; document. Prove no coalescing is lost via the `packetBatchSize` histogram (baseline vs optimized). Expected: p50 persistence latency collapse (≈50 ms → few ms) + higher sustainable rate; measure the real numbers, don't assert them. +2. **Plan-cost reduction [only if F6 shows planning dominates].** E.g. named prepared statements per batch length, validated against pooled connections; or statement restructuring. Measure before/after `total_plan_time`. +3. **Move best-effort post-commit work off the ack path [only if profile justifies].** `recordMultibyteEvidence` (`client.ts:795-807`) is awaited inline today, already best-effort (errors logged, never rethrow) and a known deadlock source; candidate for a bounded async queue (drop counters + graceful drain on shutdown). Keep semantics; drop if complexity outweighs measured gain. Also quantify the post-commit `emit()` chain — defer anything that provably doesn't need to precede the ack. +4. **Decode-result cache for repeated raws across observers [optional; only if multi-observer profile shows repeated decode cost].** Bounded LRU keyed by raw hex. Drop if marginal or risky. +5. **Dead-config cleanup [F4].** Remove or repurpose; note compatibility implications. +6. **Phase-timing metrics [required by the spec's "Add metrics where necessary"].** Time waiting for messages / processing / queueing / DB write / pool-acquire wait — bounded cardinality, wired into `metrics.ts`, safe defaults, no behavior change. +7. **NOT to be done:** no dependency upgrades; no forking/patching `node_modules` in shipped code; no unrelated refactors; no `.github/workflows` edits; no schema migrations unless unavoidable (they complicate rollback — if one is truly needed, say so loudly in the PR). + +--- + +## 5. Benchmark requirements (from the spec; methodology is yours to perfect) + +- **Baseline** = build from `origin/main` @ `fa801dc` (record image digest + sha). **Optimized** = build from your branch. Same harness, same stack config, **interleaved A/B/A/B**, ≥3 repetitions for headline numbers. Keep both images tagged until Ben has reviewed. +- **Scenario matrix (name them, run both sides):** (S1) low steady ≈2 msg/s ×≥3 min — prove no efficiency regression when idle-ish; (S2) sustained ≈25-50 msg/s ×≥5 min; (S3) burst (e.g. ≈300-500 msg/s for 10-20 s, then quiet, measure drain time); (S4) duplicates flood; (S5) multi-observer (same packets via 3+ observer keys); (S6) DB congestion (e.g. `docker pause` the bench timescaledb for a few seconds mid-run, or a competing heavy query — your choice, document); (S7) reconnect (restart bench broker connection / drop the backend's TCP, verify QoS1 redelivery); (S8) crash mid-burst (`docker kill` the bench backend; restart; verify zero loss of *sent* messages, dedup handles replays) + shutdown drain (`SIGTERM` → clean drain, no stuck `MQTT_DRAIN_TIMEOUT`); (S9) privacy transition sanity on the bench stack (visibility classification still fails closed; the intended flows from the privacy tests still hold). +- **Measure per scenario:** persisted msg/s; persistence latency avg/p50/p95/p99 (**define and document the method** — e.g. a bench-only instrumentation patch applied identically to BOTH builds writing per-message phase timestamps to JSONL; keep that patch as a file in the bench dir and do NOT ship it in the PR; post-hoc "visible in DB" polling is acceptable as a cross-check but state its granularity bias); CPU-time per 1,000 persisted messages (cgroup `cpu.stat` delta of the backend container ÷ persisted count; also report per-scenario CPU% and total); average batch size (histogram); SQL executions per 1,000 messages (pg_stat_statements delta); DB connection utilization (peak concurrent `pg_stat_activity`); peak memory (cgroup `memory.peak`); queue/lag behavior (broker queue depth if available + backlog drain time). +- **Raw outputs preserved:** one file per run under the bench dir (`results/--run.json|txt`); consolidated `bench.md` with method → raw results → timeline → threats-to-validity → exact repro commands. No invented numbers — if something couldn't be measured, say so. +- **Repro commands** must be exact and self-contained enough for the operator to re-run. + +--- + +## 6. Correctness & tests (must preserve — from the spec) + +Preserve: QoS1 durable-ack semantics (no ack before commit); correct redelivery/recovery after crash or DB outage; dedup + idempotency; privacy classification & fail-closed visibility; atomicity of packet + packet_path writes; correct node/observer/statistics updates; ordering where state depends on prior messages; bounded queues/backpressure/graceful shutdown; deadlock/hot-row protection; existing WebSocket + live-map behavior. Never trade correctness for benchmark numbers. + +Run (at minimum, and document results in the PR): +- `npm run typecheck`, `npm test`, `npm run test:ingest-integration`, `npm run build` (in `backend/`); `npm run test:analysis-integration` if anything it touches changes; contract checks only if routes/contracts change (prefer not to touch them). +- NEW tests for whatever you change (flush behavior incl. immediate-flush + drain + retry paths; the metrics additions get at least a no-crash/format check). +- The S4/S7/S8/S9 scenarios double as regression tests — record their outcomes as evidence. +- Watch the PR's CI (`/home/ben/bin/gh pr checks --watch` or poll) until green. + +--- + +## 7. Deliverables & checkpoints + +- Commits on `perf/mqtt-ingest-20261011`, conventional style (`perf(mqtt): …`, `test(mqtt): …`, `docs: …`), small and coherent. Do NOT commit `TASK-BRIEF.md` or bench outputs; do not `git add -A` blindly. +- Push the branch, open the PR to `main` (title suggestion: `perf(mqtt): durable ingest — verified serialization analysis + measured efficiency improvements`). PR body must contain the **8 required sections** from the spec (§Appendix A): root-cause analysis w/ code refs → implementation & why safe → before/after benchmarks (CPU efficiency, throughput, latency, batching, memory) → tests executed + results → reliability/privacy/concurrency risks considered (including the pump-constraint analysis) → config changes + compatibility → rollback procedure → explicit production-untouched confirmation. Include raw evidence paths + repro commands. **Do not merge.** +- `FINAL-REPORT.md` in the bench dir: what you did, measured results, surprises, an honest mistake inventory, open questions, proposed future work (incl. the concurrency-constraint discussion). +- Keep the session available for follow-up questions (Ben may ask the agent directly later); evidence stays on disk. + +--- + +## 8. PROGRESS LOG (append entries; newest last — keep ≤10 lines each) + +**Format:** `### — — ` then bullets. Also add an `OPEN QUESTIONS` section at the bottom if any; continue past any non-blocking question with a safe default. + +- [x] Phase 0 — verify semantics + F1-F6 against HEAD (source + minimal experiments) +- [ ] Phase 1 — bench harness built + smoke-proven (incl. WS fanout check) +- [ ] Phase 2 — baseline runs + raw results +- [ ] Phase 3 — implementation + tests +- [ ] Phase 4 — optimized runs + comparison +- [ ] Phase 5 — chaos/reliability scenarios +- [ ] Phase 6 — PR pushed + CI green +- [ ] Phase 7 — FINAL-REPORT + final message + +### 2026-10-11 01:59 BST — Phase 0 — source findings verified; benchmark measurements remain in Phase 2 +- `npm ci` succeeded; installed `mqtt@5.16.0`; npm reported three dependency audit advisories. +- MQTT.js pump and QoS1 handler source confirm one-at-a-time callback flow; durable-handler tests pass 3/3. +- Verified enqueue/flush path, fixed 50 ms timer, single active flush, unused concurrency/queue settings, pool defaults, and eight SQL write targets. +- Valid decoder fixture experiment produced `metadataValid=true` and canonical packet IDs; histogram, pool, and SQL plan measurements remain for baseline runs. +- Referenced `scripts/ci-smoke-repro.sh` is absent at HEAD; adapt current `.github/workflows/ci.yml` smoke recipe. + +--- + +## Appendix A — Original task text (authoritative; Ben's words) + +> # UKMesh — MQTT Ingestion Performance Investigation & Optimization PR +> +> ## Objective +> Investigate the MQTT ingestion pipeline in `https://github.com/gadgethd/ukmesh`, identify opportunities to improve CPU efficiency, batching, throughput and multicore utilization, and implement justified improvements in a dedicated GitHub pull request. +> +> **ABSOLUTE RULE: DO NOT CHANGE THE LIVE SYSTEM.** This is a code investigation, development, testing and PR task ONLY. The production UKMesh deployment must remain completely untouched. +> +> ## Production safety constraints +> - Do not restart, stop, redeploy, reconfigure or otherwise modify any production service. +> - Do not execute database migrations, SQL writes, configuration changes or maintenance operations against production. +> - Do not modify the running MQTT broker, Redis instance, PostgreSQL/TimescaleDB instance or Docker containers. +> - Do not run benchmarks, packet replays or load tests against production. +> - Do not merge the PR, push directly to the default branch or activate deployment workflows. +> - Work exclusively in an isolated development branch/worktree. Use synthetic fixtures and disposable test infrastructure. +> - Read-only production metrics may be inspected if already available, but must not require changing production configuration. +> - Inspect GitHub Actions before pushing the branch to ensure the PR cannot accidentally trigger production deployment. If this cannot be guaranteed, stop before pushing and report the blocker. +> - Production behavior and data must remain unchanged until I explicitly approve a deployment in a separate task. +> +> ## Investigation +> Review the current implementation, particularly: `backend/src/mqtt/client.ts`, `backend/src/mqtt/durableHandler.ts`, `backend/src/db/packetBatch.ts`, `backend/src/db/index.ts`, `backend/src/platform/config/database.ts`, relevant metrics, tests and Docker Compose configuration. +> Previous inspection suggested the following potential issues. **Verify each finding against current HEAD rather than assuming it is correct.** +> 1. MQTT.js may be processing incoming messages sequentially because `handleMessage` waits for durable persistence before completing its callback. +> 2. Packet batching currently uses a maximum batch size of 50 and a 50ms flush interval. Determine whether sequential message processing prevents batches from filling efficiently. +> 3. `packetBatch.ts` appears to permit only one active database batch flush at a time. +> 4. `MQTT_INGEST_CONCURRENCY=8` appears in Compose configuration but may not actually control ingestion concurrency. +> 5. PostgreSQL has multiple available connections, but the ingestion design may not be utilizing them effectively. +> 6. Complex packet insertion SQL performs multiple upserts and aggregate updates, potentially creating CPU overhead or contention. +> Determine whether these are genuine bottlenecks and whether improving them would reduce total CPU consumption, not merely spread the same work across more cores. +> +> ## Performance goals +> The VPS has 8 vCPU and 24GB RAM. The primary objective is **resource efficiency**, allowing more unrelated services to run alongside UKMesh. Optimize for: +> - Lower CPU time consumed per successfully persisted MQTT message. +> - Higher sustainable ingestion throughput. +> - Better batch utilization and fewer unnecessary SQL executions. +> - Lower processing latency. +> - Bounded memory consumption and predictable backpressure. +> - Useful multicore utilization when workloads justify it. +> - No unnecessary increase in idle resource consumption. +> +> Investigate controlled concurrency, potentially starting with 2–4 concurrent operations, but do not assume additional parallelism automatically improves efficiency. +> Consider alternative approaches such as more effective batching, reduced write amplification, more efficient SQL and separating computationally expensive processing from the ingestion critical path. +> Do not introduce unnecessary services or complexity. +> +> ## Critical correctness requirements +> MQTT reliability and privacy take priority over performance. +> Preserve QoS 1 durable acknowledgement semantics: a message must not be acknowledged before its required persistence work is committed successfully. +> Do not achieve concurrency by acknowledging MQTT messages early and relying on a volatile in-memory queue. +> Investigate MQTT.js internal message handling before proposing modifications. If its callback model prevents safe concurrent processing, design around that constraint rather than bypassing durability guarantees. +> Maintain: +> - Correct redelivery and recovery after crashes or database outages. +> - Existing packet deduplication and idempotency. +> - Privacy classification and fail-closed visibility rules. +> - Atomicity of related packet and packet-path database writes. +> - Correct node, observer and statistics updates. +> - Appropriate ordering where state changes depend on prior messages. +> - Bounded queues, backpressure and graceful shutdown. +> - Protection against database deadlocks and hot-row contention. +> - Existing WebSocket and live-map behavior. +> Never sacrifice correctness to improve a benchmark. +> +> ## Testing and benchmarking +> Establish a reproducible baseline using synthetic MQTT traffic and an isolated PostgreSQL/TimescaleDB instance. +> Measure: Messages processed per second. Average and p95/p99 persistence latency. CPU time per 1,000 successfully persisted messages. Average packet batch size. Number of SQL operations per 1,000 messages. Database connection utilization. Peak memory usage. Behavior during bursts, reconnects and database failures. +> Test multiple traffic patterns, including low-volume traffic, ordinary sustained traffic, burst ingestion, duplicate messages, multiple observers and periods of database congestion. +> Verify that the new design does not increase resource consumption unnecessarily under low traffic. +> Include tests for packet loss, duplicates, partial failures, shutdown during in-flight processing, retry behavior and privacy transitions. +> Run relevant existing tests and regression checks. Run new benchmarks against both baseline and optimized implementations under equivalent conditions. +> Never use live MQTT traffic or production databases for these experiments. +> +> ## Implementation +> If the investigation identifies an improvement supported by evidence, implement the smallest maintainable solution that meaningfully improves efficiency. +> Prefer configurable, bounded concurrency and appropriate batching over unrestricted parallel processing. +> Preserve compatibility with the existing deployment configuration. Any new settings must have safe defaults and be documented. +> Add metrics where necessary to distinguish time spent waiting for incoming messages, processing, queueing, writing to PostgreSQL and waiting for database connections. +> Avoid unrelated refactors. +> If the proposed optimization introduces unacceptable correctness risks or cannot demonstrate meaningful improvements, do not force it through. Document the findings and propose a safer alternative. +> +> ## Deliverable +> Create a dedicated branch and submit a GitHub pull request to `gadgethd/ukmesh`. +> The PR description must contain: +> 1. Root-cause analysis, with code references. +> 2. Explanation of the proposed implementation and why it is safe. +> 3. Before-and-after benchmark results, including CPU efficiency, throughput, latency, batching and memory consumption. +> 4. Details of tests executed and their results. +> 5. Reliability, privacy and concurrency risks considered. +> 6. Configuration changes, if any, and compatibility implications. +> 7. A rollback procedure for a future deployment. +> 8. Explicit confirmation that production was not modified. +> If performance claims could not be benchmarked, state that clearly. Do not invent speedups. +> +> **Final instruction:** Push the development branch and open the PR only after confirming this cannot trigger production deployment. Leave the PR unmerged and the live system completely untouched. +> Report the PR URL, the findings and the measured improvement. Do not deploy anything. Wait for my review. diff --git a/apply-review-fixes-v2.py b/apply-review-fixes-v2.py new file mode 100644 index 0000000..851b6ba --- /dev/null +++ b/apply-review-fixes-v2.py @@ -0,0 +1,337 @@ +#!/usr/bin/env python3 +"""Review fixes v2 — the claim is registered INSIDE waitForInFlightPacket, in the +same microtask as the emptiness check, so exactly one successor can ever claim. + +Run from the worktree root AFTER `git checkout -- backend/src/mqtt/client.ts +backend/src/metrics.ts` (v1 edits reverted).""" +import os + +W = os.getcwd() +MQ = os.path.join(W, "backend/src/mqtt") +METRICS = os.path.join(W, "backend/src/metrics.ts") + +# ---------------------------------------------------------------- A. helper module +helper = '''/** + * Claim arbitration for the in-flight packet dedupe registry. + * + * Multiple copies of the same packet can arrive concurrently (broker + * redeliveries, relay copies fanned across observers, several ingest + * connections). Only one copy may be mid-persist at a time; everyone else + * waits for its outcome: + * + * - the attempt succeeds -> every waiter resolves as a duplicate (no write); + * - the attempt fails -> waiters re-arbitrate. The re-check and the claim + * registration happen in the SAME microtask inside this function, so + * exactly one successor claims; every later waiter queues behind that + * successor rather than racing it. This closes the race where two waiters + * both fall through after a failure and persist the packet concurrently + * (double insert: packets carry no unique constraint). + * + * Contract: the caller passes the promise that will settle when its own + * attempt settles (`claim`). On 'claimed', the caller MUST settle that + * promise and remove it from the registry when done. + */ +export async function waitForInFlightPacket( + registry: Map>, + dedupeKey: string, + claim: Promise, +): Promise<'claimed' | 'duplicate'> { + for (;;) { + const holder = registry.get(dedupeKey); + if (!holder) { + registry.set(dedupeKey, claim); + return 'claimed'; + } + const persisted = await holder.then(() => true, () => false); + if (persisted) return 'duplicate'; + } +} +''' +open(os.path.join(MQ, "dedupeRegistry.ts"), "w").write(helper) +print("A. dedupeRegistry.ts written (v2)") + +# ---------------------------------------------------------------- B. client.ts +p = os.path.join(MQ, "client.ts") +s = open(p).read() + +# B0. import +a0 = "import { createDurableMqttHandleMessage } from './durableHandler.js';" +assert s.count(a0) == 1, "B0 import anchor" +s = s.replace(a0, a0 + "\nimport { waitForInFlightPacket } from './dedupeRegistry.js';", 1) + +# B1. claim creation + arbitration replaces the single-shot if +old1 = """ const inFlight = inFlightPackets.get(dedupeKey); + if (inFlight) { + // The same packet is already being persisted by a concurrent ingest. + // Wait for its outcome: success makes this copy a confirmed duplicate, + // failure means nothing was committed and this copy persists below. + const persisted = await inFlight.then(() => true, () => false); + if (persisted) { + markMqttPersistenceHealthy(); + mqttIngestOutcomesTotal.inc({ outcome: 'duplicate_packet' }); + return; + } + }""" +assert s.count(old1) == 1, "B1 in-flight block anchor" +new1 = """ // Arbitration against concurrent copies of this packet. The claim below is + // registered inside waitForInFlightPacket in the same microtask as its + // re-check, so after a failed predecessor exactly one waiter becomes the + // successor and the rest queue behind it (never two concurrent retries). + let claimResolve!: () => void; + let claimReject!: (error: unknown) => void; + const claim = new Promise((resolve, reject) => { + claimResolve = resolve; + claimReject = reject; + }); + claim.catch(() => { /* waiters observe failure via their own handlers */ }); + + const claimState = await waitForInFlightPacket(inFlightPackets, dedupeKey, claim); + if (claimState === 'duplicate') { + claimResolve(); // never registered; settle for tidiness + markMqttPersistenceHealthy(); + mqttIngestOutcomesTotal.inc({ outcome: 'duplicate_packet' }); + return; + }""" +s = s.replace(old1, new1, 1) + +# B2. remove the pre-claim redelivered probe +old2 = """ // After a restart there is no in-memory dedup state to match against: a + // QoS 1 redelivery whose original commit survived the crash must not be + // persisted or counted a second time. Confirm against the retained window. + if (redelivered && await packetAlreadyPersisted(finalHash, observerKey, topic, network)) { + markMqttPersistenceHealthy(); + mqttIngestOutcomesTotal.inc({ outcome: 'duplicate_packet' }); + return; + } + +""" +assert s.count(old2) == 1, "B2 probe block anchor" +s = s.replace(old2, "", 1) + +# B3. probe moves inside the claimed IIFE +old3 = """ const ingest = (async () => { + try { + await persistOnce();""" +assert s.count(old3) == 1, "B3 ingest head anchor" +new3 = """ const ingest = (async () => { + // Claimed above. After a restart there is no in-memory dedup state to + // match against: a QoS 1 redelivery whose original commit survived the + // crash must not be persisted or counted a second time. Confirm against + // the retained window before writing (inside the claim so the registered + // claim promise covers this path too). + if (redelivered && await packetAlreadyPersisted(finalHash, observerKey, topic, network)) { + markMqttPersistenceHealthy(); + mqttIngestOutcomesTotal.inc({ outcome: 'duplicate_packet' }); + return; + } + try { + await persistOnce();""" +s = s.replace(old3, new3, 1) + +# B4. module state for the follow-up bound +old4 = "const inFlightPackets = new Map>();" +assert s.count(old4) == 1, "B4 registry decl anchor" +new4 = old4 + """ +// Best-effort post-persist follow-ups are bounded: a stalled database must +// not grow the in-flight set without limit. Over the bound they are dropped +// (safe: everything here is explicitly best-effort) and counted. +const POST_PERSIST_MAX_INFLIGHT = 64; +let postPersistInFlight = 0;""" +s = s.replace(old4, new4, 1) + +# B5. wire ingest settlement to the claim + cleanup keyed on the claim +old5 = """ inFlightPackets.set(dedupeKey, ingest); + try { + await ingest; + } finally { + if (inFlightPackets.get(dedupeKey) === ingest) inFlightPackets.delete(dedupeKey); + } +}""" +assert s.count(old5) == 1, "B5 registration block anchor" +new5 = """ void ingest.then( + () => claimResolve(), + (err: unknown) => claimReject(err), + ); + try { + await ingest; + } finally { + if (inFlightPackets.get(dedupeKey) === claim) inFlightPackets.delete(dedupeKey); + } +}""" +s = s.replace(old5, new5, 1) + +# B6. bounded follow-up dispatch +old6 = """ // Best-effort live-map follow-ups run once the durable write is done and + // outside the acknowledgement path: PUBACK is released as soon as the + // write commits and these resolve asynchronously. + void runPostPersistFollowUps({ + finalHash, + decodedPathHashSizeBytes, + path, + decodedRouteType, + network, + resolvedPacketType, + }).catch((err: unknown) => { + console.error( + '[mqtt] post-persist follow-up failed:', + err instanceof Error ? err.message : String(err), + ); + });""" +assert s.count(old6) == 1, "B6 follow-up block anchor" +new6 = """ // Best-effort live-map follow-ups run once the durable write is done and + // outside the acknowledgement path: PUBACK is released as soon as the + // write commits and these resolve asynchronously. The in-flight count is + // bounded so a database stall cannot grow them without limit; over the + // bound they are dropped (best-effort) and counted. + if (postPersistInFlight >= POST_PERSIST_MAX_INFLIGHT) { + postPersistDroppedTotal.inc(); + } else { + postPersistInFlight++; + postPersistInFlightGauge.set(postPersistInFlight); + void runPostPersistFollowUps({ + finalHash, + decodedPathHashSizeBytes, + path, + decodedRouteType, + network, + resolvedPacketType, + }).catch((err: unknown) => { + console.error( + '[mqtt] post-persist follow-up failed:', + err instanceof Error ? err.message : String(err), + ); + }).finally(() => { + postPersistInFlight--; + postPersistInFlightGauge.set(postPersistInFlight); + }); + }""" +s = s.replace(old6, new6, 1) + +# B7. metrics import additions +old7 = """import { + boundedNetworkMetricLabel, + mqttIngestAckDuration, + mqttIngestActive, + mqttIngestOutcomesTotal, + mqttMessagesTotal, +} from '../metrics.js';""" +assert s.count(old7) == 1, "B7 metrics import anchor" +new7 = """import { + boundedNetworkMetricLabel, + mqttIngestAckDuration, + mqttIngestActive, + mqttIngestOutcomesTotal, + mqttMessagesTotal, + postPersistDroppedTotal, + postPersistInFlightGauge, +} from '../metrics.js';""" +s = s.replace(old7, new7, 1) + +open(p, "w").write(s) +print("B. client.ts patched (v2)") + +# ---------------------------------------------------------------- C. metrics.ts +m = open(METRICS).read() +addition = """ +export const postPersistInFlightGauge = new Gauge({ + name: 'meshcore_mqtt_post_persist_inflight', + help: 'Best-effort post-persist follow-ups currently in flight (bounded).', +}); + +export const postPersistDroppedTotal = new Counter({ + name: 'meshcore_mqtt_post_persist_dropped_total', + help: 'Best-effort post-persist follow-ups dropped because the in-flight bound was reached.', +}); +""" +assert "postPersistInFlightGauge" not in m, "C: metrics already patched?" +open(METRICS, "a").write(addition) +print("C. metrics.ts appended (v2)") + +# ---------------------------------------------------------------- D. tests (v2 contract) +tests = '''import assert from 'node:assert/strict'; +import test from 'node:test'; +import { waitForInFlightPacket } from './dedupeRegistry.js'; + +interface ClaimHandle { + claim: Promise; + resolve: () => void; + reject: (error: unknown) => void; +} + +function claimHandle(): ClaimHandle { + let resolve!: () => void; + let reject!: (error: unknown) => void; + const claim = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + claim.catch(() => { /* observed by waiters or the test */ }); + return { claim, resolve, reject }; +} + +test('claims (and registers) when no attempt is in flight', async () => { + const registry = new Map>(); + const mine = claimHandle(); + assert.equal(await waitForInFlightPacket(registry, 'k', mine.claim), 'claimed'); + assert.equal(registry.get('k'), mine.claim); +}); + +test('resolves as duplicate when the in-flight attempt succeeds', async () => { + const registry = new Map>(); + const holder = claimHandle(); + registry.set('k', holder.claim); + const mine = claimHandle(); + assert.equal(await waitForInFlightPacket(registry, 'k', mine.claim), 'duplicate'); + assert.equal(registry.get('k'), holder.claim); // untouched +}); + +test('re-arbitrates after a failed attempt: exactly one successor claims', async () => { + const registry = new Map>(); + const first = claimHandle(); + registry.set('k', first.claim); + + const b = claimHandle(); + const c = claimHandle(); + const bResult = waitForInFlightPacket(registry, 'k', b.claim); + const cResult = waitForInFlightPacket(registry, 'k', c.claim); + + // The failed attempt rejects; its cleanup (handler finally) removes it + // before waiters re-check, exactly as in production. + first.reject(new Error('db down')); + registry.delete('k'); + + const outcomes = (await Promise.all([bResult, cResult])).sort(); + assert.deepEqual(outcomes, ['claimed', 'duplicate']); + // The successor's claim is registered; the packet is not lost. + assert.equal(registry.get('k'), b.claim); + b.resolve(); + await b.claim; +}); + +test('successive failures chain without double-claiming or losing the packet', async () => { + const registry = new Map>(); + const first = claimHandle(); + registry.set('k', first.claim); + + const b = claimHandle(); + const c = claimHandle(); + const bResult = waitForInFlightPacket(registry, 'k', b.claim); + + first.reject(new Error('first failure')); + registry.delete('k'); + assert.equal(await bResult, 'claimed'); + + // c starts waiting behind b; b's attempt also fails. + const cResult = waitForInFlightPacket(registry, 'k', c.claim); + b.reject(new Error('second failure')); + registry.delete('k'); + assert.equal(await cResult, 'claimed'); + + // The third attempt succeeds; exactly one persist wins overall. + c.resolve(); + await c.claim; +}); +''' +open(os.path.join(MQ, "dedupeRegistry.test.ts"), "w").write(tests) +print("D. dedupeRegistry.test.ts written (v2)") +print("ALL EDITS APPLIED (v2)") diff --git a/apply-review-fixes.py b/apply-review-fixes.py new file mode 100644 index 0000000..34cef58 --- /dev/null +++ b/apply-review-fixes.py @@ -0,0 +1,306 @@ +#!/usr/bin/env python3 +"""Apply review fixes (ChatGPT points 2+3) to the perf worktree on the VPS. + +Run from the worktree root: python3 apply-review-fixes.py +All edits are asserted before writing; a failed assert aborts with nothing written +for that file. +""" +import os + +W = os.getcwd() +MQ = os.path.join(W, "backend/src/mqtt") +METRICS = os.path.join(W, "backend/src/metrics.ts") + +# ---------------------------------------------------------------- A. helper module +helper = '''/** + * Claim arbitration for the in-flight packet dedupe registry. + * + * Multiple copies of the same packet can arrive concurrently (broker + * redeliveries, relay copies fanned across observers, several ingest + * connections). Only one copy may be mid-persist at a time; everyone else + * waits for its outcome: + * + * - the attempt succeeds -> every waiter resolves as a duplicate (no write); + * - the attempt fails -> waiters re-arbitrate: the first waiter whose + * re-check finds no claimant returns 'claimed' and its caller registers + * its own attempt synchronously (before any other await), so the remaining + * waiters queue behind that successor. This closes the race where two + * waiters both fall through after a failure and persist the packet + * concurrently (double insert, since packets carry no unique constraint). + */ +export async function waitForInFlightPacket( + registry: Map>, + dedupeKey: string, +): Promise<'claimed' | 'duplicate'> { + for (;;) { + const holder = registry.get(dedupeKey); + if (!holder) return 'claimed'; + const persisted = await holder.then(() => true, () => false); + if (persisted) return 'duplicate'; + } +} +''' +open(os.path.join(MQ, "dedupeRegistry.ts"), "w").write(helper) +print("A. dedupeRegistry.ts written") + +# ---------------------------------------------------------------- B. client.ts +p = os.path.join(MQ, "client.ts") +s = open(p).read() + +# B0. import +a0 = "import { createDurableMqttHandleMessage } from './durableHandler.js';" +assert s.count(a0) == 1, "B0 import anchor" +s = s.replace(a0, a0 + "\nimport { waitForInFlightPacket } from './dedupeRegistry.js';", 1) + +# B1. claim loop replaces the single-shot if +old1 = """ const inFlight = inFlightPackets.get(dedupeKey); + if (inFlight) { + // The same packet is already being persisted by a concurrent ingest. + // Wait for its outcome: success makes this copy a confirmed duplicate, + // failure means nothing was committed and this copy persists below. + const persisted = await inFlight.then(() => true, () => false); + if (persisted) { + markMqttPersistenceHealthy(); + mqttIngestOutcomesTotal.inc({ outcome: 'duplicate_packet' }); + return; + } + }""" +assert s.count(old1) == 1, "B1 in-flight block anchor" +new1 = """ // Wait for any concurrent copy of this packet already mid-persist. On its + // success this copy is a duplicate; on its failure we re-arbitrate so that + // exactly one waiter takes over the claim instead of racing it. The + // successor claim below is registered synchronously (no await between this + // return and inFlightPackets.set). + if ((await waitForInFlightPacket(inFlightPackets, dedupeKey)) === 'duplicate') { + markMqttPersistenceHealthy(); + mqttIngestOutcomesTotal.inc({ outcome: 'duplicate_packet' }); + return; + }""" +s = s.replace(old1, new1, 1) + +# B2. remove the pre-claim redelivered probe +old2 = """ // After a restart there is no in-memory dedup state to match against: a + // QoS 1 redelivery whose original commit survived the crash must not be + // persisted or counted a second time. Confirm against the retained window. + if (redelivered && await packetAlreadyPersisted(finalHash, observerKey, topic, network)) { + markMqttPersistenceHealthy(); + mqttIngestOutcomesTotal.inc({ outcome: 'duplicate_packet' }); + return; + } + +""" +assert s.count(old2) == 1, "B2 probe block anchor" +s = s.replace(old2, "", 1) + +# B3. probe (and its comment) moves inside the claimed IIFE +old3 = """ const ingest = (async () => { + try { + await persistOnce();""" +assert s.count(old3) == 1, "B3 ingest head anchor" +new3 = """ const ingest = (async () => { + // Claimed above. After a restart there is no in-memory dedup state to + // match against: a QoS 1 redelivery whose original commit survived the + // crash must not be persisted or counted a second time. Confirm against + // the retained window before writing (inside the claim so the synchronous + // registration right after this IIFE covers this path too). + if (redelivered && await packetAlreadyPersisted(finalHash, observerKey, topic, network)) { + markMqttPersistenceHealthy(); + mqttIngestOutcomesTotal.inc({ outcome: 'duplicate_packet' }); + return; + } + try { + await persistOnce();""" +s = s.replace(old3, new3, 1) + +# B4. module state for the follow-up bound +old4 = "const inFlightPackets = new Map>();" +assert s.count(old4) == 1, "B4 registry decl anchor" +new4 = old4 + """ +// Best-effort post-persist follow-ups are bounded: a stalled database must +// not grow the in-flight set without limit. Over the bound they are dropped +// (safe: everything here is explicitly best-effort) and counted. +const POST_PERSIST_MAX_INFLIGHT = 64; +let postPersistInFlight = 0;""" +s = s.replace(old4, new4, 1) + +# B5. bounded follow-up dispatch +old5 = """ // Best-effort live-map follow-ups run once the durable write is done and + // outside the acknowledgement path: PUBACK is released as soon as the + // write commits and these resolve asynchronously. + void runPostPersistFollowUps({ + finalHash, + decodedPathHashSizeBytes, + path, + decodedRouteType, + network, + resolvedPacketType, + }).catch((err: unknown) => { + console.error( + '[mqtt] post-persist follow-up failed:', + err instanceof Error ? err.message : String(err), + ); + });""" +assert s.count(old5) == 1, "B5 follow-up block anchor" +new5 = """ // Best-effort live-map follow-ups run once the durable write is done and + // outside the acknowledgement path: PUBACK is released as soon as the + // write commits and these resolve asynchronously. The in-flight count is + // bounded so a database stall cannot grow them without limit; over the + // bound they are dropped (best-effort) and counted. + if (postPersistInFlight >= POST_PERSIST_MAX_INFLIGHT) { + postPersistDroppedTotal.inc(); + } else { + postPersistInFlight++; + postPersistInFlightGauge.set(postPersistInFlight); + void runPostPersistFollowUps({ + finalHash, + decodedPathHashSizeBytes, + path, + decodedRouteType, + network, + resolvedPacketType, + }).catch((err: unknown) => { + console.error( + '[mqtt] post-persist follow-up failed:', + err instanceof Error ? err.message : String(err), + ); + }).finally(() => { + postPersistInFlight--; + postPersistInFlightGauge.set(postPersistInFlight); + }); + }""" +s = s.replace(old5, new5, 1) + +# B6. metrics import additions +old6 = """import { + boundedNetworkMetricLabel, + mqttIngestAckDuration, + mqttIngestActive, + mqttIngestOutcomesTotal, + mqttMessagesTotal, +} from '../metrics.js';""" +assert s.count(old6) == 1, "B6 metrics import anchor" +new6 = """import { + boundedNetworkMetricLabel, + mqttIngestAckDuration, + mqttIngestActive, + mqttIngestOutcomesTotal, + mqttMessagesTotal, + postPersistDroppedTotal, + postPersistInFlightGauge, +} from '../metrics.js';""" +s = s.replace(old6, new6, 1) + +open(p, "w").write(s) +print("B. client.ts patched") + +# ---------------------------------------------------------------- C. metrics.ts +m = open(METRICS).read() +addition = """ +export const postPersistInFlightGauge = new Gauge({ + name: 'meshcore_mqtt_post_persist_inflight', + help: 'Best-effort post-persist follow-ups currently in flight (bounded).', +}); + +export const postPersistDroppedTotal = new Counter({ + name: 'meshcore_mqtt_post_persist_dropped_total', + help: 'Best-effort post-persist follow-ups dropped because the in-flight bound was reached.', +}); +""" +assert "postPersistInFlightGauge" not in m, "C: metrics already patched?" +open(METRICS, "a").write(addition) +print("C. metrics.ts appended") + +# ---------------------------------------------------------------- D. tests +tests = '''import assert from 'node:assert/strict'; +import test from 'node:test'; +import { waitForInFlightPacket } from './dedupeRegistry.js'; + +test('claims when no attempt is in flight', async () => { + const registry = new Map>(); + assert.equal(await waitForInFlightPacket(registry, 'k'), 'claimed'); +}); + +test('resolves as duplicate when the in-flight attempt succeeds', async () => { + const registry = new Map>(); + registry.set('k', Promise.resolve()); + assert.equal(await waitForInFlightPacket(registry, 'k'), 'duplicate'); +}); + +test('re-arbitrates after a failed attempt: exactly one successor claims', async () => { + const registry = new Map>(); + let failFirst!: (error: unknown) => void; + const first = new Promise((_resolve, reject) => { failFirst = reject; }); + registry.set('k', first); + + // Mirror the handler contract: after 'claimed' the caller registers its own + // attempt synchronously, then resolves it. + const contender = async (): Promise<'claimed' | 'duplicate'> => { + const state = await waitForInFlightPacket(registry, 'k'); + if (state === 'duplicate') return 'duplicate'; + let resolveMine!: () => void; + const mine = new Promise((resolve) => { resolveMine = resolve; }); + registry.set('k', mine); + queueMicrotask(resolveMine); + await mine; + return 'claimed'; + }; + + const b = contender(); + const c = contender(); + // The failed attempt's cleanup (handler finally) removes it before waiters + // re-check, exactly as in production. + failFirst(new Error('db down')); + registry.delete('k'); + + const outcomes = (await Promise.all([b, c])).sort(); + assert.deepEqual(outcomes, ['claimed', 'duplicate']); +}); + +test('successive failures chain without double-claiming or losing the packet', async () => { + const registry = new Map>(); + + interface Claim { resolve: () => void; reject: (e: unknown) => void; done: Promise; } + const contender = async (): Promise<'duplicate' | Claim> => { + const state = await waitForInFlightPacket(registry, 'k'); + if (state === 'duplicate') return 'duplicate'; + let resolve!: () => void; + let reject!: (e: unknown) => void; + const mine = new Promise((res, rej) => { resolve = res; reject = rej; }); + registry.set('k', mine); + return { + resolve, + reject, + done: mine.then(() => 'claimed-persisted', () => 'claimed-failed'), + }; + }; + + let failFirst!: (e: unknown) => void; + registry.set('k', new Promise((_res, rej) => { failFirst = rej; })); + + const b = contender(); + const c = contender(); + + failFirst(new Error('first failure')); + registry.delete('k'); + + const bRes = await b; + assert.notEqual(bRes, 'duplicate'); + const claimB = bRes as Claim; + + // c is now queued behind b's claim; b's claim also fails. + claimB.reject(new Error('second failure')); + registry.delete('k'); + + const cRes = await c; + assert.notEqual(cRes, 'duplicate'); + const claimC = cRes as Claim; + + // The third attempt succeeds; exactly one persist wins overall. + claimC.resolve(); + assert.equal(await claimB.done, 'claimed-failed'); + assert.equal(await claimC.done, 'claimed-persisted'); +}); +''' +open(os.path.join(MQ, "dedupeRegistry.test.ts"), "w").write(tests) +print("D. dedupeRegistry.test.ts written") +print("ALL EDITS APPLIED")