Skip to content

fix(precompute): force-close open windows on worker shutdown - #376

Merged
zzylol merged 1 commit into
mainfrom
fix/shutdown-force-close
Jun 16, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/shutdown-force-close

Conversation

@zzylol

@zzylol zzylol commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Problem

flush_all only advances the per-group watermark by +1ms (plus the wall-clock fallback, which is gated on grace having elapsed on a flush tick). For a one-shot batch ingest whose records all fall in a single window — so event-time never advances past the window end — the trailing window never closes, emit_batch is never called, and that window's data never reaches the store.

The Shutdown handler previously just called flush_all(), so the trailing window was lost.

Fix

Add Worker::force_close_all(), invoked from the Shutdown handler after the final flush_all(). For every bucket with open panes it closes and emits every remaining window unconditionally (no more samples will arrive after shutdown), covering both pane maps:

  • active_panes (sample aggregation) → merge_panes_for_window
  • sketch_panes (OTLP-delivered sketches) → merge_sketch_panes_for_window

To advance past the open windows it uses a finite bound derived from the largest open pane (max_pane + window_size_ms), not i64::MAX: WindowManager::closed_windows enumerates window starts one slide at a time, so i64::MAX would loop ~i64::MAX/slide times and overflow. max_pane + window_size_ms is the smallest watermark that closes the latest open window.

Idempotent: drained panes are removed from both maps and their wall-clock bookkeeping is pruned, so a second call emits nothing.

Tests

  • shutdown_force_close_emits_trailing_sample_window (active_panes path)
  • shutdown_force_close_emits_trailing_sketch_window (sketch_panes path, 10 DDSketches at frozen event-time → one merged window emitted with total_count() == 10)

Verification

  • cargo check --lib --tests — clean (pre-existing warnings only)
  • precompute_engine::worker30 passed (incl. 2 new)
  • precompute_engine (module) — 241 passed

Context

Companion to ProjectASAP/ASAPQuery#400, which ports the wall-clock grace fallback into the frontend and adds the same shutdown force-close there. This PR adds the shutdown force-close to the backend, where the wall-clock fallback already exists.

🤖 Generated with Claude Code

flush_all only advances the watermark by +1ms (plus the wall-clock fallback,
whose grace may not have elapsed for a one-shot batch), so a batch whose
records all fall in a single window — event-time never advancing past the
window end — leaves the trailing window open and its data never reaches the
store.

Add Worker::force_close_all(), invoked from the Shutdown handler after the
final flush_all(): for every bucket with open panes, advance to a finite
bound (max_pane + window_size_ms) and emit every remaining window, covering
both active_panes (sample aggregation) and sketch_panes (OTLP sketches). A
finite bound is used deliberately — WindowManager::closed_windows enumerates
window starts one slide at a time, so passing i64::MAX would loop
~i64::MAX/slide times and overflow. Idempotent: drained panes are removed and
their wall-clock bookkeeping pruned.

Tests: shutdown_force_close_emits_trailing_sample_window and
shutdown_force_close_emits_trailing_sketch_window.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 49b381b into main Jun 16, 2026
@zzylol
zzylol deleted the fix/shutdown-force-close branch June 16, 2026 16:45
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