Field import: camp (2026-08-27) - #223
Closed
rolker wants to merge 3 commits into
Closed
Conversation
added 3 commits
August 27, 2026 07:44
The live-coverage catalog subscription was created transient-local to match the boat-side producer. But camp runs on the operator side and never sees that publisher: it receives the catalog as republished by udp_bridge, which emits VOLATILE. A transient-local subscriber is QoS-incompatible with a volatile publisher, so the subscription never matched, handleCatalog() never fired, and the ADR-0006 D4 anti-entropy reconcile never pruned in-memory tiles, their GPU textures, or the on-disk copies. Stale coverage persisted on screen indefinitely after a boat-side store reset. Measured live on pandy during the BizzyBoat deployment, on /bizzy/sensors/m3/cube_bathymetry/coverage_catalog: publisher /operator/udp_bridge RELIABLE/VOLATILE subscriber /operator/camp RELIABLE/TRANSIENT_LOCAL (never matches) subscriber coverage_renderer RELIABLE/VOLATILE (matched fine) Confirmed both directions with `ros2 topic echo --once --qos-durability`: volatile returns a catalog immediately, transient_local returns nothing. Switch the durability to VOLATILE, keeping RELIABLE and depth 1. The tradeoff is explicit and accepted: there is no latched sample, so camp no longer gets the current catalog on join and must wait for the next publication. A never-matching subscription delivers nothing at all, so waiting strictly dominates. The camp#169 buffering in handleCatalog() is unchanged and stays correct — it is now more load-bearing, not less. Its premise was the once-only delivery of a latched sample; under VOLATILE there is no latched sample either, and the catalog is published only on change while the boat's catalog is stable, so a sample dropped while disabled may not recur for the rest of the session. disableLiveCoverage() tears down only the tile stream (unsubscribeTiles); catalog_sub_ is reset only in the destructor, so samples keep arriving and landing in last_catalog_ throughout a disabled window for enableLiveCoverage() to replay. Comments on the buffer, the member, and the class doc corrected to state the real reason. The QoS moves into catalogSubscriptionQos() so a regression test can pin it. Nothing pinned the durability before, which is exactly why the mismatch was invisible; test_sonar_live_catalog_qos now asserts VOLATILE/RELIABLE/ depth 1. Verified: camp builds; 312 tests, 0 failures (1 pre-existing GL skip in test_gggs_render). A camp restart is required for this to take effect.
The ADR-0006 D4 anti-entropy reconcile pruned only the fine tiles. The overview pyramid built by foldIntoParent() was entirely exempt: handleCatalog() erased from `tiles_` and deleted `<cache_dir>/<stem>.tif`, and never touched `overview_tiles_` or `overviews/`. The comment on foldIntoParent() asserted that exemption as intended design — "a local derived product; prune-on-absence must not touch them" — which conflated ADR-0010 D4 (overviews are never entered into the reconciler, true and unchanged) with an exemption from the prune itself. So a retracted region kept its coarse coverage forever, and warm-load reinstated it on every restart. Measured on pandy during the BizzyBoat deployment after a boat-side store reset: with the 532b6df QoS fix in place the fine cache pruned correctly (107M -> 44M, every survivor newer than the restart) while 41M of `overviews/` survived and the operator still saw the pre-reset coverage. No code path could ever reflect a boat-side reset in the pyramid. Compounding it, the fold is purely accumulative — foldChild() only ever folds data IN and has no inverse — so a withdrawn contribution cannot be subtracted from a parent in place. reconcilePyramid() runs at the end of every reconcile, after the fine prune. The catalog is authoritative over the fine tiles and the pyramid is a function of them, so an overview should exist exactly when it is an ancestor of a live fine index. Two failure modes, two repairs: * stale by absence — no live descendant: remove it from `overview_tiles_`, free its GL texture under the renderer's context (the camp#134 discipline the fine prune already follows), and delete its `overviews/` file. The DIRECTORY is swept, not just the resident map: eviction phase 2 frees an overview Entry while deliberately keeping its disk copy (ADR-0010 D2), so a stale overview can be absent from memory and still be warm-loaded back. That is exactly how the pandy pyramid kept returning. * dirty by partial withdrawal — some descendants live, one was withdrawn: rebuild the tile from its surviving children. This reproduces the original construction (D3 folds the WHOLE parent into the grandparent, so every level above the fine tiles is "the fold of my children"), drawing inputs from a child repaired earlier in the same pass, else the resident copy, else the disk copy. A tile with no surviving child is removed instead — a zoomed-out gap is honest, stale coverage is not. Two properties are load-bearing. Proportionality: every fine tile's ancestor chain reaches level 0, so invalidating the chain of each pruned tile would destroy the whole pyramid on any ordinary retraction, and tiles are withdrawn and re-added in normal operation. Nothing is removed while a live descendant remains, and only ancestors of something really withdrawn are rebuilt. Prune-gate parity: a generation_time of 0 disables prune-on-absence in the reconciler (ADR-0008 D4b), so it disables the pyramid sweep too, and the live set is unioned with the ancestors of the fine tiles still held locally so a held tile too NEW to prune keeps its ancestors alive. Budget accounting is preserved: removals shrink overviewResidentBytes(), a rebuild that pulls a non-resident overview back into memory is followed by evictIfOverBudget(), and overviews still never enter the reconciler. Two supporting changes. cancelPendingWrite() drops a deleted tile's queued write-through (a coalesced relaunch would otherwise re-create the .tif the prune just removed); an in-flight write is left to finish with its WriteState intact, because erasing that state would let a later fold launch a second worker racing it on the shared <stem>.tif.tmp path. And the `<level>_<row>_<col>` cache stem, open-coded at three sites, moves into tileStem()/fineTilePath()/overviewTilePath() with indexFromStem() as the inverse the disk sweep needs — a divergence between the writer's name and a deleter's would orphan files silently rather than fail. test_sonar_live_overview_prune pins all of it (GL-free, ROS-free, same offscreen harness as test_sonar_live_eviction): stale overviews leave memory and disk; ancestors with a surviving catalogued descendant are kept (the over-deletion guard); a shared ancestor is rebuilt without the withdrawn descendant's contribution, in memory and on disk; a catalog that retracts nothing changes nothing; an unstamped catalog sweeps nothing. Verified the first two fail against a stubbed reconcilePyramid, and that the over-deletion guard fails against the rejected naive chain-invalidation design. ADR-0010 gains D7 and retires the "overview lifecycle-on-retraction / catalog-prune propagation" deferred follow-up it had carried since 2026-08-20. Per-overview provenance was rejected as an alternative: it cannot survive warm-load, which is precisely the path that resurrects a stale pyramid. Verified: camp builds clean; 317 tests, 0 failures (baseline 312, +4 new gtest cases +1 ctest aggregate entry), 1 pre-existing GL skip in test_gggs_render. A camp restart is required for this to take effect.
… tile The live-coverage draw list appended EVERY resident overview tile, coarse->fine, then the fine tiles on top. Its stated premise (camp#160) was "where a fine tile is present it fully covers its parent" — false: a child covers a QUARTER of its parent. Wherever fine coverage is sparse, which is most of a survey in progress, every coarse ancestor painted through, and the apex is resident by design (kApexProtectLevel, ADR-0010 D1) so the coarsest level of all drew at every zoom. Measured on pandy during the BizzyBoat deployment: every cached tile is 960x960 cells at every level, so ground cell size doubles per level up and a level-0 cell is ~667 x 926 m carrying the MEAN of everything folded beneath it (4 valid cells at level 0; 1264 at level 11). The operator saw a solid ~650 m block over a good portion of the survey area, covering the chart — including water the survey has never touched, since an overview's own extent is 4^n fine tiles wide. Coarse data may now be drawn ONLY as a placeholder for a fine tile that is known to exist and has not loaded, and only over that tile's own footprint. planDraw() resolves exactly two contributions: for each pending index, the finest resident ancestor overview painted over the FINE index's extent through the sub-rect of that ancestor it occupies; and every resident fine tile, whole, on top. Nothing else. "Known to exist" is derived, not guessed: the catalog is authoritative for what exists (catalogued_fine_, assigned from every catalog since a complete catalog is a snapshot), unioned with evicted_fine_indices_ for the warm start with no link, minus what is resident. Scale never enters it. A placeholder exists exactly while a real tile is missing, so zoom-out needs no threshold: zoomed out, nearly every catalogued tile is non-resident and the union of the footprints IS the coverage. The alternative considered first — cell size vs. screen pixel — was rejected: it would have stopped level 0 at survey zoom, but at any zoom where a coarse level did qualify it would still smear that level across its full extent, unsurveyed water included. Clipping needed a renderer change. RasterFieldItem carries one texture and a geographic extent, and the renderer stretches the whole texture across whatever bounds it is given, so narrowing the bounds would SQUASH the overview into the small box rather than clip it. The item gains a normalized texture window [u0,v0]-[u1,v1] (u west->east, v north->south, texture row 0 = north) honoured by RasterGlRenderer's texcoord generation. It defaults to the whole texture, so RasterLayer, GggsTileLayer and this layer's own resident tiles emit exactly the coordinates they did before — verified by reading all three call sites (each default-constructs the item and assigns members, none aggregate-initialises). The window comes from GGGS index arithmetic (rows and columns both double per level, and the +-72/+-80 latitudeScaleFactor bands are bounded by whole grid rows at every level, so an ancestor and its descendants share a band), making it an exact power-of-two fraction; a descendant that does not resolve inside its ancestor draws nothing rather than guessing. The CPU-side alternative — crop each sub-window into its own small texture, no renderer change — was rejected: at zoom-out the pending set is the whole survey, so it pays an upload per placeholder per frame, or keeps a second resident texture per pending index, which is the memory eviction exists to reclaim. camp#103 offscreen culling is intact (the clip test is still before textureFor, now in planDraw), as is the camp#134 GL-context discipline. camp#172 reload is unchanged and consistent by construction: every index it wants to reload is evicted, hence pending, hence currently shown as a placeholder, and a completed reload replaces that placeholder with the real thing. The converse deliberately does not hold — a catalogued tile never received has no disk copy, so it is a request candidate, not a reload one. test_sonar_live_placeholder_draw pins the rule (GL-free, ROS-free, same offscreen harness as test_sonar_live_overview_prune): nothing coarse while the fine tiles are resident (the reported defect); a known-but-unloaded tile IS covered, over its own footprint and through a strict sub-rect of the ancestor, asserted against an independent geographic derivation of the same window; whole-survey coverage survives evicting every fine tile, with no catalog at all; and unsurveyed water inside the apex's extent draws nothing at either zoom. Verified the first, third and fourth fail against the pre-fix draw list and the second and third against an unclipped placeholder. test_raster_gl_renderer gains the renderer half: the default window still samples the whole texture, and each quadrant window samples exactly that quadrant (the north/south pair catches a flipped v). Verified it fails against the pre-change texcoords. ADR-0010 D5 is replaced (draw-order fallback -> clipped placeholder) and D1's guarantee is restated in the shape it now has: the protected apex is not drawn at zoom-out, it is the guaranteed resident SOURCE a zoomed-out view samples through the footprints of the missing tiles. Two consequences recorded: a pyramid whose fine tiles are neither catalogued nor on disk now draws nothing, and zoom-out issues one small quad per missing tile (sharing the same few textures) rather than one per overview. Verified: camp builds; 323 tests, 0 failures (baseline 317, +5 new gtest cases +1 new ctest aggregate entry), 1 pre-existing GL skip in test_gggs_render. A camp restart is required for this to take effect.
Owner
Author
|
Duplicate of #220, which already carries these same three commits ( Pre-review findings from that pass have been moved to #220 so the analysis is not lost. Closing this and deleting |
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
1 participant
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.
Imports three field commits made on pandy during the BizzyBoat Isles of Shoals deployment.
Closes #222.
Three field commits made on pandy (operator station) during the BizzyBoat
Isles of Shoals deployment and pushed to the gitcloud field remote.
campisnot diverged —
gitcloud/jazzyis a strict superset oforigin/jazzy, sothese arrive with their original SHAs: no cherry-pick, no rewrite.
Commits
532b6dffix: subscribe to coverage_catalog VOLATILE so it matches udp_bridgeaf5e378fix: propagate catalog prune-on-absence into the overview pyramid0fc6408fix: draw coarse coverage only as a clipped placeholder for a missing tile2085 insertions / 102 deletions across 10 files.
Pre-review against the Quality Standard
This batch is unusually complete for field work. Every one of the three
commits ships with a new test and registers it in
CMakeLists.txt, and two ofthem update
docs/decisions/0010-bounded-eviction-overview-pyramid.mdalongsidethe code:
532b6dftest/test_sonar_live_catalog_qos.cpp(44)af5e378test/test_sonar_live_overview_prune.cpp(487)0fc6408test/test_sonar_live_placeholder_draw.cpp(471),test/test_raster_gl_renderer.cpp(68)The QoS fix is measured rather than reasoned: the commit records the live
publisher/subscriber QoS pair observed on pandy, and confirms it both ways with
ros2 topic echo --once --qos-durability.Finding 1 — the VOLATILE downgrade is framed as permanent, but the defect it works around is tracked for repair
532b6dfswitches the catalog subscription to VOLATILE and states the tradeoffas accepted: no latched sample, so camp waits for the next publication
instead of getting the current catalog on join.
That reasoning is sound today — a never-matching subscription delivers
nothing, so waiting strictly dominates. But the root cause is a relay defect
that is already tracked:
udp_bridge#78 — "Relay downgrades publisher durability: TRANSIENT_LOCAL
republished VOLATILE, silently unmatching every correct consumer".
The same mismatch was hit by the web-view coverage renderer on the same
deployment, and that commit
(
unh_marine_autonomy1ffc459) marks itselfFIELD WORKAROUND ... Revert to TRANSIENT_LOCAL once the relay is fixed.camp's carries no such marker.
Consequence: when udp_bridge#78 lands and the relay preserves producer
durability, a TRANSIENT_LOCAL publisher still matches camp's VOLATILE
subscriber — so nothing breaks, and nobody is prompted. camp simply keeps the
late-join loss permanently, for a bug that no longer exists. Suggest recording
the revert condition in the code or the ADR so the two consumers stay in step.
Finding 2 — ADR edits arrive without separate review
af5e378and0fc6408amenddocs/decisions/0010-*.mdby 181 lines combined.That is the right instinct (a change carrying its consequences), but ADR text is
governance and normally gets its own read. Flagging for human attention in this
PR rather than treating it as ordinary code.
Clean
/bizzy/sensors/m3/cube_bathymetry/coverage_catalog.(prune-on-absence, missing-tile placeholder).
Merge state
Not diverged —
origin/jazzyfast-forwards togitcloud/jazzy. No merge needed.Authored-By:
Claude Code AgentModel:
Claude Opus 5 (1M context)Test plan
CMakeLists.txt(verified per-commit at import).🤖 Generated with Claude Code
https://claude.ai/code/session_014eGztbXhDcsyFFx8Lp1BAr