mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-08-18 11:49:46 +00:00
df61660a5e
## Summary Mirrors the distance-index lazy pattern (#1011): the subpath and path-hop index builds are no longer part of `Load()`'s synchronous critical section. They now run in **two parallel background goroutines** kicked off after `s.loaded = true`, so HTTP comes up immediately even at Cascadia scale (5M observations, previously ~60s blocked on these two builds inside `Load()` under `s.mu`). Fixes #1008. ## Approach Two new `atomic.Bool` fields on `PacketStore` (`subpathReady`, `pathHopReady`) plus a one-shot broadcast channel (`indexReadyChan`) for waiters. `Load()` removes the synchronous `s.buildSubpathIndex()` / `s.buildPathHopIndex()` calls and instead kicks `s.startBackgroundIndexBuilds()` right before returning. That function spawns **two independent goroutines** (review m7), one per index. Each goroutine: 1. acquires `s.mu.Lock()` (blocks until `Load()`'s deferred Unlock fires), 2. runs its builder, releases the lock, stores its `ready = true`, 3. closes the broadcast channel if both flags are now true, 4. logs `[startup] index build complete: subpath (Xs)` (or pathHop). Analytics handlers whose entire response IS the index aggregate — `/api/analytics/subpaths`, `/api/analytics/subpaths-bulk`, `/api/analytics/subpath-detail`, `/api/nodes/{pubkey}/paths` — gate reads behind the corresponding atomic and respond with `503 Service Unavailable`, `Retry-After: 5`, body `{"error":"index loading","retryAfter":5}` until the build completes — matching the triage spec. ### Handler scope (review M2) A second class of handlers also touches these indexes — `/api/nodes`, `/api/nodes/{pubkey}`, the `GetRepeaterRelayInfoMap` / `GetRepeaterUsefulnessScoreMap` / `GetBridgeScore` enrichment helpers, and `repeater_liveness` / `repeater_usefulness`. These are **intentionally NOT 503-gated**: they expose the index via optional enrichment fields that callers already treat as "may be empty", and 503-ing the SPA bootstrap to wait for an index that only affects relay-activity badges would be a worse UX than a 30–60s window of "—" values. The rationale is documented in the package doc-comment at the top of `index_ready_1008.go`. The recomputer's synchronous prewarm path (`StartRepeaterEnrichmentRecomputer`) gates on `WaitIndexesReady(60s)` (review M1) so it never snapshots an empty `byPathHop` into `s.repeaterRelayCache`; on timeout it skips the prewarm and lets the 5-minute ticker pick up the populated index. ## Concurrency safety Each build goroutine acquires `s.mu.Lock()` before calling the existing `buildSubpathIndex()` / `buildPathHopIndex()` helpers, which replace `s.spIndex` / `s.spTxIndex` / `s.byPathHop` with freshly-allocated maps. Visibility of the populated maps to handlers that observe `Ready()==true` is established by Go 1.19+ sync/atomic acquire-release semantics: the atomic store of `true` happens-after `s.mu.Unlock()`, and the handler's atomic load synchronizes-with that store. The handler's subsequent `s.mu.RLock` serializes against concurrent ingest writers, not against the builder. The existing `main.go` boot sequence does not start ingest goroutines until after `store.Load()` returns and graph init completes, so the brief window between `Load()` returning and the two goroutines acquiring `s.mu` does not race with concurrent ingest writes. ## TDD: red → green - **Red** commit `63e79e11`: `cmd/server/index_ready_1008_test.go` adds four assertions; `cmd/server/index_ready_1008.go` adds compile-only stubs returning `true` so the tests fail on assertions, not build errors. - **Green** commit `fb1d22b0`: implements the real atomic gates, the background goroutine, and the four handler 503 branches; also updates four existing tests that read indexes directly post-`Load()` to call `store.WaitIndexesReady(5s)` first. - **Race-fix commit `b77d56eb`** (review m8 — test-infra exemption): adds `WaitIndexesReady` calls in test helpers/setup paths so the race detector no longer flags the read-after-Load() pattern in existing tests. Per AGENTS.md, race-detector flakes are observable evidence (test crashes under `-race`) and qualify for the test-infra exemption from the TDD red-commit requirement; no behavior change in production code. - **Polish round 2 — M1 red `408c7462` / green `85e82c8a`**: `TestIssue1008_M1_PrewarmWaitsForIndexes` asserts the recomputer prewarm SKIPs when indexes are not ready. Red commit adds the assertion + a stub `repeaterEnrichmentPrewarmWait` var; green commit wires `WaitIndexesReady` into the prewarm path and adds the handler-scope docs for M2. - **Polish round 2 — minor cleanups `fd089bd0`** (m3..m7): chunk-loader wires `markIndexesReadySync`, memory-model comment rewritten to cite acquire-release, sentinel deleted, polling replaced with a broadcast channel, two parallel goroutines for the builds. `TestIssue1008_m7_BothFlagsSetAfterParallelStart` covers the parallel path. ## Reproduction ``` git fetch origin fix/issue-1008 git checkout 63e79e11 # red commit cd cmd/server && go test -run TestIssue1008_ -count=1 . # FAILs git checkout fix/issue-1008 # latest green cd cmd/server && go test -run TestIssue1008 -count=1 -race . # all pass cd cmd/server && go test -count=1 -race -short ./... # full suite ok ``` ## Files changed | file | role | |---|---| | `cmd/server/store.go` | atomic.Bool fields + indexReadyChan broadcast field; remove sync build calls in Load(); kick goroutines; wire markIndexesReadySync from chunk loader | | `cmd/server/index_ready_1008.go` | ready flags, two-goroutine background builds, 503 helper, channel-based WaitIndexesReady, handler-scope docs | | `cmd/server/index_ready_1008_test.go` | red-commit contract tests + parallel-start assertion | | `cmd/server/repeater_enrich_recomputer.go` | gate prewarm on WaitIndexesReady (M1) | | `cmd/server/repeater_enrich_recomputer_1008_test.go` | M1 red+green assertions | | `cmd/server/routes.go` | 503 gate on 4 analytics handlers | | `cmd/server/routes_test.go` | setup helpers wait for ready; collision test waits | | `cmd/server/coverage_test.go` | three tests wait for ready before reading indexes | ## Out of scope - Distance index (already deferred in #1011) — untouched. - The `pickBestObservation` + `indexByNode` per-tx loop in `Load()` — kept synchronous per triage Findings (ordering-sensitive, contiguous-memory, fast). --------- Co-authored-by: bot <bot@noreply.local> Co-authored-by: openclaw-bot <bot@openclaw.local> Co-authored-by: mc-bot <mc-bot@users.noreply.github.com>