Hourly rollup scheduler + atomic watermark + rollup-aware query path - #4
Merged
Conversation
Wires the rollup builder up to a tokio background task. Each tick
(default every 30s) reads the manifest, computes a target watermark
of floor((now - safety_lag) / 1h) * 1h, and for each hour in the gap
[current_watermark, target):
1. Scans raw segments overlapping the hour
2. Aggregates events whose timestamp falls inside the hour, grouped
by bucket (account-derived) and dimension canonical-JSON
3. Writes one rollup segment per bucket touched (rollup_<uuid>.rseg)
The manifest update — appending the new SegmentMeta entries and
advancing watermarks.hourly_rollup_ms — happens in a single save(),
so a crash mid-tick leaves the previous watermark in place and the
next tick re-does the work. Re-tick over an already-rolled hour is a
no-op (covered by test).
Query path now distinguishes RawEvents vs RollupHourly:
- RawEvents: as before, raw segments + memtable
- RollupHourly: rollup segments for hours <= watermark, plus raw scan
for the open-period tail (timestamps > watermark). SUM(quantity)
matches across both sources (covered by test). COUNT differs —
documented as 1-per-rollup-row, not per underlying event.
Other changes:
- HourlyRollupKey/Record drop the per-builder u32 dimensions_key in
favor of canonical JSON, so records are cross-segment comparable.
- RollupSegmentWriter::finish returns (rows, checksum); meta carries
the checksum like raw segments now do.
- New RollupSegmentReader verifies the checksum on open.
- Config gains rollup_tick_interval_secs and rollup_safety_lag_ms.
- main.rs spawns the worker with a Notify-based shutdown signal.
- Fix in collect_raw_events: per-event time filter at read time, not
just segment-level pruning, so the rollup-fallback path doesn't
double-count events that are already in a rollup segment.
Tests (tests/rollups.rs, 4 tests):
- tick seals completed hours and advances the watermark
- second tick over the same window is a no-op (idempotency)
- query through rollup returns the same SUM as raw scan
- open hour stays visible via raw fallback merged with rollup data
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
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
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.
Summary
Closes review item #9. Wires the orphaned rollup builder up to a real background scheduler. The headline spec use case — "monthly account usage from rollups" — now actually goes through rollups instead of falling back to a raw scan.
What works now
Scheduler. A tokio background task runs every
rollup_tick_interval_secs(30s default). Each tick:target = floor((now - safety_lag) / 1h) * 1h[current_watermark, target), scans raw segments that overlap the hour, aggregates events whose timestamp falls inside it (grouped by bucket and dimension canonical JSON), writes onerollup_<uuid>.rsegsegment per bucket touchedSegmentMetaentries tomanifest.rollup_segmentsand advancesmanifest.watermarks.hourly_rollup_msin a singlesave()— a crash mid-tick leaves the previous watermark in place and the next tick redoes the work, so the operation is idempotent (covered by test)Query path.
RollupHourlyqueries now scan rollup segments for hours ≤ watermark, then fall back to raw segments + memtable for the open-period tail (timestamps > watermark).SUM(quantity)matches acrossRawEventsandRollupHourlyregardless of how the data is split (covered by test).Open-period correctness. A separate test sets up two hours of data, ticks the worker with a
nowthat places the watermark between them, and verifies that the rollup-source query sees both: the older hour from rollups, the newer hour from raw fallback. Bug found and fixed during this:collect_raw_eventswas returning all events in a segment regardless of the caller-supplied time range — fine forRawEvents(the aggregator filters later) but it caused double-counting on the fallback path. Now it filters per-event.Notable design choices
HourlyRollupKey/HourlyRollupRecorddrop the per-builderu32 dimensions_key(meaningless across segments) in favor of canonical JSON. Records are now self-describing and cross-segment comparable.RollupSegmentWriter::finish()returns(rows, checksum). Rollup segment metadata carries a blake3 checksum like raw segments, verified on every open.RollupHourlyre-derives the bucket per event fromaccount_idrather than trusting the source segment's bucket label, so future compaction that mixes buckets can't poison the rollup.Configgainsrollup_tick_interval_secs(default 30) androllup_safety_lag_ms(default 60_000). The safety lag bounds the open-period duration the query path has to merge across.COUNT caveat
Each rollup row counts as 1, not as the number of underlying events. This is documented in the executor doc-comment and in the README — callers needing exact event counts should use the
RawEventssource.SUM(quantity)is correct in both.Test plan
cargo build --all-targetsclean with-D warningscargo test --all-targets— 27 tests pass (was 23; +4 intests/rollups.rs)Still on the backlog
rejectedcounts (Phase D operability: explain + verify endpoints #14)🤖 Generated with Claude Code