test(dq): pin presigned-blob-link + tombstone-voiding parity#8
Closed
zer0stars wants to merge 1 commit into
Closed
test(dq): pin presigned-blob-link + tombstone-voiding parity#8zer0stars wants to merge 1 commit into
zer0stars wants to merge 1 commit into
Conversation
Lock in the two behaviors ported from the legacy ClickHouse eventrepo (superseded PRs #3, #4), now served natively from the DuckLake layer: - PresignBlobURL: a blob key becomes a presigned S3 GET URL for the configured blob bucket; nil presigner, empty bucket, and presigner failure all error loudly rather than panic or silently inline. Complements TestLakeEventService_BlobIndexKey (blob-key routing into the resolver's HasPrefix(key, BlobKeyPrefix) presign branch). - Voiding under reverse ingest order: a tombstone whose timestamp precedes the event it voids still hides that event (the anti-join keys on voids_id == id, not on time). Complements TestLakeEventService_VoidingExcludes (tombstone-after).
This was referenced Jun 25, 2026
Member
Author
|
Landed directly on |
zer0stars
added a commit
that referenced
this pull request
Jul 7, 2026
…nts serving From a 4-lens adversarial regression review of the load-review PRs: - #5b count: captureRollupDelta inherited the anti-join's 30d probe clamp, so an ancient (>30d) redelivery counted as new and signals_latest.count drifted +1 vs RecomputeRollup. The exact-timestamp match is already partition-pruned; dropped the redundant BETWEEN so an ancient redelivery finds its existing row (count stays exact). - #8 location floor: the 90d gap-fill lookback returned (0,0) for a vehicle whose last GPS fix was 90d-1y old but still within LAKE_DECODED_RETENTION — a behavior regression vs the pre-#8 unbounded LocationAt. Widened past any realistic retention (the base is retention-pruned, so it costs nothing) to restore the prior semantics. - #5a events_latest serving: GetEventSummaries served the rollup as soon as the table EXISTED, so during the first-create migration rebuild (or for a brand-new subject before the next flush) it returned zero events. Now falls back to the live base scan when the rollup is empty for the subject. - #5a events_latest migration (Q3b): the first-create rebuild used an in-memory flag set from pre-DDL existence, so a crash mid-rebuild silently stranded dormant subjects on restart. Re-derived from DB state (events non-empty AND rollup empty) each boot, crash- safe like the loc_ts marker. Tests: ancient-redelivery count parity, empty-rollup fallback, crash-mid-migration restart rebuild; location equivalence updated. Full suite + PG concurrency green.
zer0stars
added a commit
that referenced
this pull request
Jul 8, 2026
…m cap (load review) (#15) * perf(dq): batch segment/daily location gap-fills into one as-of join (#8) 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. * perf(dq): cap the idle query DuckDB on a materializer pod (#7) 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. * feat(dq): events_latest rollup for GetEventSummaries; #5b floor deferred (#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. * feat(dq): re-decode backfill tool + cursor-reset alert; #1c deferred (#1) (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).
zer0stars
added a commit
that referenced
this pull request
Jul 8, 2026
…nts serving From a 4-lens adversarial regression review of the load-review PRs: - #5b count: captureRollupDelta inherited the anti-join's 30d probe clamp, so an ancient (>30d) redelivery counted as new and signals_latest.count drifted +1 vs RecomputeRollup. The exact-timestamp match is already partition-pruned; dropped the redundant BETWEEN so an ancient redelivery finds its existing row (count stays exact). - #8 location floor: the 90d gap-fill lookback returned (0,0) for a vehicle whose last GPS fix was 90d-1y old but still within LAKE_DECODED_RETENTION — a behavior regression vs the pre-#8 unbounded LocationAt. Widened past any realistic retention (the base is retention-pruned, so it costs nothing) to restore the prior semantics. - #5a events_latest serving: GetEventSummaries served the rollup as soon as the table EXISTED, so during the first-create migration rebuild (or for a brand-new subject before the next flush) it returned zero events. Now falls back to the live base scan when the rollup is empty for the subject. - #5a events_latest migration (Q3b): the first-create rebuild used an in-memory flag set from pre-DDL existence, so a crash mid-rebuild silently stranded dormant subjects on restart. Re-derived from DB state (events non-empty AND rollup empty) each boot, crash- safe like the loc_ts marker. Tests: ancient-redelivery count parity, empty-rollup fallback, crash-mid-migration restart rebuild; location equivalence updated. Full suite + PG concurrency green.
zer0stars
added a commit
that referenced
this pull request
Jul 8, 2026
…ven) (#16) * perf(dq): paginate a single oversized snapshot into byte/row-bounded windows (#1c) A single fat snapshot (span can't drop below 1) with a large blob fan-out materialized its whole readDelta slice + resolveBlobs payloads at once and could OOM the single writer, which then crash-looped on the identical delta. The (cursor, to] insert delta is now read+decoded+written in row-key-ordered windows bounded by DUCKDB window byte budget (64 MiB) / row cap (100k): intermediate windows are written idempotently WITHOUT advancing the cursor, and only the final window's transaction advances it — so "cursor advanced <=> every window durable" still holds. A crash mid-span leaves the cursor un-advanced; restart re-reads the whole span and the cloud_event_id anti-join collapses windows that already landed. Proven: crash-mid-span exactly-once recovery (embedded), paginated fat snapshot under two concurrent materializers on a real Postgres catalog (exactly-once, no dupes), and small-snapshot single-window (common path unchanged). * perf(dq): maintain signals_latest incrementally at commit time (#5b) The steady-state rollup recomputed each dirty subject's signals_latest over its ENTIRE retained history every flush (O(history) x active fleet). It is now folded INCREMENTALLY inside the decode-commit transaction (O(batch)): - recency (timestamp/value_*/loc_*/loc_ts) is recomputed from the base but BOUNDED to `timestamp >= the row's prior latest`, so it prunes every older day-partition yet stays exact and self-healing (the new latest is in the batch or the prior row still qualifies; deduped via ORDER BY timestamp DESC, cloud_event_id ASC). - count = prev.count + a NOT-EXISTS delta over DISTINCT (subject,name,timestamp), captured BEFORE the base insert so a redelivery, a same-(s,n,ts) collision, and a replayed window each contribute 0 (idempotent). first_seen min-folds. Composes with the #1c windows (each window folds in its own tx) and with concurrent writers (temp scratch tables are per-connection; the fold rides the atomic commit). The full-history rollupSelectSQL is retained only for the boot/DR rebuild and the deferred bulk-backfill catch-up. events_latest still uses the recompute (far lower volume); mirroring the fold to it is a follow-up. Proven: differential test (incremental == RecomputeRollup) across redelivery, same-timestamp collision, out-of-order, multi-window, dormant re-report; crash-replay idempotency; a 25-round randomized fuzz; and exactness under two concurrent paginated materializers on a real Postgres catalog. * test(dq): 10-loop adversarial verification campaign for #1c/#5b exactly-once * fix(dq): adversarial-review regressions — count clamp, loc floor, events serving From a 4-lens adversarial regression review of the load-review PRs: - #5b count: captureRollupDelta inherited the anti-join's 30d probe clamp, so an ancient (>30d) redelivery counted as new and signals_latest.count drifted +1 vs RecomputeRollup. The exact-timestamp match is already partition-pruned; dropped the redundant BETWEEN so an ancient redelivery finds its existing row (count stays exact). - #8 location floor: the 90d gap-fill lookback returned (0,0) for a vehicle whose last GPS fix was 90d-1y old but still within LAKE_DECODED_RETENTION — a behavior regression vs the pre-#8 unbounded LocationAt. Widened past any realistic retention (the base is retention-pruned, so it costs nothing) to restore the prior semantics. - #5a events_latest serving: GetEventSummaries served the rollup as soon as the table EXISTED, so during the first-create migration rebuild (or for a brand-new subject before the next flush) it returned zero events. Now falls back to the live base scan when the rollup is empty for the subject. - #5a events_latest migration (Q3b): the first-create rebuild used an in-memory flag set from pre-DDL existence, so a crash mid-rebuild silently stranded dormant subjects on restart. Re-derived from DB state (events non-empty AND rollup empty) each boot, crash- safe like the loc_ts marker. Tests: ancient-redelivery count parity, empty-rollup fallback, crash-mid-migration restart rebuild; location equivalence updated. Full suite + PG concurrency green.
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.
Verification tests for the functionality of PRs #3 (presigned blob links) and #4 (tombstone voiding), which were superseded by the DuckLake-native reimplementation on
main(theBLOB_BUCKETrefactor). Those PRs target the removed ClickHouseeventrepo; rebasing them would resurrectclickhouse-go/sqlboiler. These tests lock in that the behavior survives natively so #3/#4 can be closed safely.Added (
internal/service/duck/pr3_pr4_parity_test.go):TestLakeEventService_PresignBlobURL— blob key → presigned S3 GET URL for the configured blob bucket; nil presigner / empty bucket / presigner error all fail loudly. Complements existingTestLakeEventService_BlobIndexKey(blob-key routing into the resolver'sHasPrefix(key, BlobKeyPrefix)presign branch).TestLakeEventService_VoidingTombstoneBeforeEvent— a tombstone whose timestamp precedes the event it voids still hides it (anti-join keys onvoids_id == id, not time). Fills the reverse-order gap a review of Hide events based on new tombstone rows #4 flagged. Complements existingTestLakeEventService_VoidingExcludes.Scope (unchanged from #4): voiding hides raw CloudEvents (
cloudEvents/latestCloudEvent); decodedsignals/eventsare not voided (materializer keeps_ = voidsID).go build ./... && go vet && go test ./internal/service/duck/ ./internal/graph/ ./pkg/eventrepo/all green.