perf(dq): batch location gap-fills, events rollup, re-decode tool, mem cap (load review)#15
Merged
Merged
Conversation
) Segment and daily-activity enrichment gap-filled each boundary's location with a separate LocationAt reverse-scan point query — O(2*segments) serial queries per request (up to thousands on a sparse-GPS vehicle with the idling candidate cap), each holding a pooled connection. Add duck.Queries.LocationsAt: one ASOF LEFT JOIN resolves the nearest non-origin fix at or before every requested boundary timestamp, index-aligned. It is exactly equivalent to per-point LocationAt — the right side excludes (0,0) fixes before deduping (subject,name,timestamp) to the lowest cloud_event_id, so the as-of match is tie-free and deterministic — proven by TestLakeQueries_LocationsAt_MatchesLocationAt across the (0,0)-skip, tie-break, and lookback-floor cases. Also add a 90d lookback floor to both LocationAt and LocationsAt so a fix-less vehicle no longer full-reverse-scans its retained partition; the floor is shared so the two paths stay equivalent. Wire GetSegments and GetDailyActivity to collect every fix-less boundary and resolve them in a single batched pass (BatchLocationAtSource), falling back to the (0,0) sentinel unchanged. TestGetSegments_BatchesLocationGapFill and TestGetDailyActivity_BatchesLocationGapFill prove O(1) location queries (one batched call, zero point calls) and correct scatter-by-index.
A materializer release always builds the query DuckDB backend (main.go serves the query HTTP/gRPC unconditionally) even though it serves no reads: the overlay disables ingress and the query fleet is a separate release with its own Service selector, so no query/fetch traffic is routed to it. That idle instance still honored DUCKDB_MEMORY_LIMIT=6GiB, so decode(6) + query(6) could reach 12GiB on an 8Gi pod and OOM. Fully skipping the query backend is entangled (main.go serves the query HTTP/gRPC and probes duckSvc for readiness unconditionally) and higher-risk, so take the lower-risk config route: add DUCKDB_QUERY_MEMORY_LIMIT, applied via queryDuckConfig ONLY when MATERIALIZER_ENABLED, to cap the idle query instance; the decode instance keeps the full DUCKDB_MEMORY_LIMIT. Set it to 1GiB in values-materializer.yaml so the two limits sum to 7GiB < 8Gi. A query pod (MATERIALIZER_ENABLED=false) ignores the override entirely. TestQueryDuckConfig_MaterializerMemoryCap pins all three cases.
…red (#5) (a) GetEventSummaries full-history-scanned lake.events per dataSummary, with no rollup — asymmetric with the now-cheap signal side. Add lake.events_latest, a per-(subject,name) count + first/last-seen rollup maintained by the materializer exactly like signals_latest: a dirtyEventSubjects set marked post-commit, FlushEventRollup recomputing only dirty subjects bucket-chunked off the decode commit, RecomputeEventRollup for the disaster-recovery / first-create backfill, and an orphan-prune in PruneDecoded. eventRollupSelectSQL mirrors GetEventSummaries' (subject,timestamp,name,source) dedup + GROUP BY name, so the rollup is a materialized view by construction. GetEventSummaries serves from it, falling back to the base scan until the table exists (self-healing, guards a rollout where a query pod predates the materializer creating the new table). ensureSchema escalates the FIRST FlushEventRollup to a full rebuild when events_latest is created over a pre-existing lake.events (the migration case) so dormant vehicles are backfilled and never read an empty summary; a fresh catalog skips it (no snapshot churn). LAKE_REBUILD_ROLLUP_ON_BOOT rebuilds both rollups. Proven by tests/ducklake_event_rollup_test.go: end-to-end incremental via the real decode path (with a cross-cloud_event_id duplicate to prove read-dedup), full-rebuild == per-batch parity, and first-create dormant-subject backfill. (b) The signals rollup recompute timestamp floor is DEFERRED as TODO(load-review #5b): a naive floor undercounts count/first_seen (full-history aggregates) and an incremental count fold can't stay exact because the write anti-join keys on cloud_event_id, so a different-id duplicate is stored and only the read QUALIFY dedup collapses it — an incremental += would double-count it, breaking the rollup-exactness invariant the tests assert. The code carries a precise split-recency/cumulative-column design for a proven follow-up. The commit/cursor-advance exactly-once path is untouched; both rollups are materialized views maintained off the commit.
) (a) When the decode cursor lags past LAKE_SNAPSHOT_RETENTION, maybeRecoverExpired skips the unretained prefix WITHOUT decoding it and only cursorResetsTotal records the loss — no tool existed to recover the gap. Add BackfillTimeRange: it reads raw_events in [from, to) DIRECTLY from the base table (not the expired change feed; the rows survive din's separate row retention), re-decodes via the existing decode path, and idempotent-inserts into lake.signals/events WITHOUT touching the ingest_progress cursor (out-of-band repair). Idempotency uses the same cloud_event_id anti-join but with the UNCLAMPED [min,max] timestamp window (minMaxTime, factored out of timeRange) so re-decoding arbitrarily old data still finds and skips existing rows — the steady-state 30d probe-floor clamp would miss old duplicates and double-insert. Exposed as `dq -backfill-from <RFC3339> -backfill-to <RFC3339>` (runs once and exits), which flushes both rollups after. Proven idempotent + cursor-untouched + skipped-range-recovery by tests/ducklake_backfill_test.go. (b) The DQMaterializerCursorReset alert already existed; corrected its stale "reset to head" wording (the code skips only the unretained prefix) and made it actionable — it now names the backfill invocation to recover the gap. (c) The per-pass byte budget is DEFERRED as TODO(load-review #1c): the real OOM is one oversized snapshot (span can't drop below 1), which only intra-snapshot row-key-window pagination can bound — and that decouples the atomic insert+cursor-advance the chaos-proven exactly-once protocol relies on, so it must be re-proven under SIGKILL, not just unit-tested. The code carries the precise pagination design (page by (subject,timestamp,cloud_event_id), idempotent intermediate windows, cursor coupled only to the final window's insert). The commit/cursor-advance exactly-once path is untouched; readDelta only gained a shared scan helper (readRawByTime reuses it).
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.
Load/efficiency fixes (dq) — findings #1, #5, #7, #8
From the end-to-end din+dq+vt load review (
REVIEW-din-dq-vt-load-2026-07-07.md). Two items are partial by design — the exactly-once-sensitive halves are deferred with code TODOs + designs (see bottom).#8 — N+1 LocationAt gap-fills in segments (MEDIUM-HIGH)
Segment/dailyActivity enrichment issued a separate
LocationAtreverse-scan per segment boundary (up to thousands of serial point queries on a sparse-GPS vehicle).duck.Queries.LocationsAt: oneASOF LEFT JOINresolves the nearest non-origin fix ≤ each boundary timestamp, index-aligned;(0,0)-skip +cloud_event_idtie-break make it exactly equal to the per-pointLocationAt. Shared 90d lookback floor.GetSegments/GetDailyActivitycollect fix-less boundaries and resolve them in one batched pass.#7 — materializer pod runs a redundant 2nd DuckDB instance (MEDIUM)
app.Newalways builds the query backend + the materializer's own DuckDB; each honoredDUCKDB_MEMORY_LIMIT=6GiBvs an 8Gi pod. The materializer release doesn't serve reads (ingress.enabled:false), butmain.goserves query/probesduckSvcunconditionally, so skipping construction is entangled.DUCKDB_QUERY_MEMORY_LIMITcaps the idle query instance only whenMATERIALIZER_ENABLED(set to 1GiB → 6+1 < 8Gi); decode instance keeps the full budget; query pods ignore it.#5 — events rollup (+ signals-rollup floor deferred)
events_latestrollup (done):GetEventSummariesfull-history scannedlake.eventsperdataSummary(no events equivalent ofsignals_latest). Addedlake.events_latest, a faithful mirror of the signals rollup (dirty-subject, bucket-chunked, off-commit, self-healing), with a first-create migration backfill for dormant subjects;GetEventSummariesserves from it with an existence fallback.#1 — single materializer (re-decode tool + alert; pagination deferred)
dq -backfill-from <RFC3339> -backfill-to <RFC3339>re-decodes araw_eventstime range from the base table (survives change-feed expiry) idempotently, without advancing the cursor — closes the silent-loss half of cursor-expiry (resetCursorskips undecoded spans; previously no recovery tool existed).DQMaterializerCursorResetalert wording to name the backfill invocation.Deferred (exactly-once-sensitive — need SIGKILL re-proving, not unit tests)
TODO(load-review #5b)inducklake.go. A naive floor undercounts cumulativecount/movesfirst_seen; needs a per-(subject,name)counted-through watermark with its own incremental==full-recompute-under-redelivery tests.TODO(load-review #1c). A single oversized snapshot can't drop below span=1; bounding it means paging one snapshot's rows and decoupling the atomic insert+cursor-advance the chaos-proven protocol relies on, so it must be re-proven under SIGKILL. Interim: sizeLAKE_SNAPSHOT_RETENTION+ pod memory so one snapshot's working set fits.Verification
go build/go vetclean;go test ./...(14 pkgs ok, 0 FAIL);golangci-lint run ./...= 0 issues. Correctness-critical tests run on real embedded DuckLake:LocationsAt≡LocationAt, batched-gap-fill O(1); events-rollup incremental (cross-cloud_event_iddup proves read-dedup) + full-rebuild parity + first-create backfill; backfill idempotent + cursor-untouched + recovers-skipped-range; materializer memory cap.🤖 Generated with Claude Code
https://claude.ai/code/session_01SAgMMjdGVCD7M1yLy7BUPq