Skip to content

fix(precompute): separate event and closure watermarks - #483

Merged
zzylol merged 1 commit into
mainfrom
fix/precompute-watermark-separation
Sep 3, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/precompute-watermark-separation

Conversation

@zzylol

@zzylol zzylol commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Why

PR #480 prevented one short bulk-ingest failure by refreshing a pane idle timer, but periodic flushes still manufactured event time and unrelated groups could close each other. allowed_lateness_ms also did not delay event-time closure.

Before this PR

For a fixed timestamp, every timer flush advanced previous_watermark_ms by 1 ms. After enough flushes, valid input was classified as late. A newer group on the same worker could also force-close another group through max/min watermark propagation.

After this PR

  • Track maximum observed event time separately from the closure watermark.
  • Compute the event watermark as max_event_time - allowed_lateness.
  • Never mutate observed event time from a periodic flush.
  • Keep group closure independent; worker watermark atomics are diagnostic only.
  • Apply ForwardToStore consistently to raw samples rejected by either lateness or prior closure.
  • Update the precompute design document to match.

Example: with a 10-second window and 5-second allowed lateness, observing event time 10s leaves [0s, 10s) open; observing 15s closes it. Repeated timer flushes do not change that result.

Verification

  • cargo fmt -p data_plane -- --check
  • cargo check -p data_plane --tests
  • cargo test -p data_plane precompute_engine:: -- --nocapture (260 passed)

New regression coverage verifies repeated flushes, per-group isolation, allowed-lateness closure, and raw correction forwarding.

@zzylol
zzylol merged commit 01ce093 into main Sep 3, 2026
1 check passed
@zzylol
zzylol deleted the fix/precompute-watermark-separation branch September 3, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant