Skip to content

test(precompute): add real-pipeline e2e regression test for #474 - #538

Merged
milindsrivastava1997 merged 3 commits into
mainfrom
474-precompute-wall-clock-e2e-test
Aug 18, 2026
Merged

test(precompute): add real-pipeline e2e regression test for #474#538
milindsrivastava1997 merged 3 commits into
mainfrom
474-precompute-wall-clock-e2e-test

Conversation

@milindsrivastava1997

Copy link
Copy Markdown
Contributor

Summary

  • Stacked on test(precompute): add a wall-clock override hook to PrecomputeEngine #537. Adds JsonFileIngestConfig::batch_delay_ms (default 0, a no-op everywhere in production and in the existing netflow_single_second e2e test) — a real, explicit delay after each batch send, so an integration test can force ingest to span real wall-clock time deterministically instead of hoping incidental scheduling overhead is enough.
  • New e2e test drives the real JsonFileIngestSource → PrecomputeEngine → sink pipeline with a bulk single-timestamp load spread across 10 batches, using test(precompute): add a wall-clock override hook to PrecomputeEngine #537's wall-clock override (unscaled — an earlier scaled version amplified ordinary scheduling jitter into spurious failures) and generous, seconds-scale margins so the periodic flush timer gets many real chances to land mid-ingest without depending on tight timing.
  • Verified red against the pre-fix birth-time semantics (temporarily reverted the fix from fix(precompute): keep wall-clock fallback from force-closing panes still under active ingest #536, ran, confirmed rows were dropped mid-ingest, restored the fix) before counting it as a real regression test.
  • Along the way, fixed allowed_lateness_ms in the new test's config: every flush_all() call unconditionally nudges the event-time watermark forward by 1ms (the "boundary advance" that lets an idle stream make progress), independent of the wall-clock fallback. With allowed_lateness_ms=0 and multiple batches spaced across several flush ticks, that 1ms drift alone was enough to mark the next same-timestamp batch "late" via a wholly different code path, unrelated to fix(precompute): bulk single-timestamp ingest silently drops rows when wall-clock fallback closes the window mid-ingest #474. A generous, production-realistic value avoids that.

Test plan

  • New e2e test passes; verified red against pre-fix code first.
  • Existing e2e_netflow_single_second and e2e_precompute_equivalence still pass.
  • Full precompute_engine:: unit suite (81 tests) passes.
  • cargo clippy -p query_engine_rust --lib --tests clean.

🤖 Generated with Claude Code

Comment thread asap-query-engine/src/precompute_engine/json_ingest.rs Outdated
Comment thread asap-query-engine/tests/e2e_precompute_wall_clock_fallback_active_ingest.rs Outdated
milindsrivastava1997 added a commit that referenced this pull request Aug 18, 2026
Two review comments from @zzylol on #538:

- batch_delay_ms's doc comment called it "test-support only," but it lives
  on JsonFileIngestConfig, a real production struct, and nothing about it
  is actually test-gated -- it's an ordinary throttling knob that just has
  no caller setting it nonzero yet. Reworded to describe what it does, not
  who currently uses it.
- The new e2e test's module doc led with "regression test for issue #474."
  Reframed to lead with the behavior/property under test (bulk single-
  timestamp ingest must not lose data while still actively receiving
  samples), keeping the causal explanation of the prior bug as supporting
  context rather than the test's identity.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
milindsrivastava1997 added a commit that referenced this pull request Aug 18, 2026
Two review comments from @zzylol on #538:

- batch_delay_ms's doc comment called it "test-support only," but it lives
  on JsonFileIngestConfig, a real production struct, and nothing about it
  is actually test-gated -- it's an ordinary throttling knob that just has
  no caller setting it nonzero yet. Reworded to describe what it does, not
  who currently uses it.
- The new e2e test's module doc led with "regression test for issue #474."
  Reframed to lead with the behavior/property under test (bulk single-
  timestamp ingest must not lose data while still actively receiving
  samples), keeping the causal explanation of the prior bug as supporting
  context rather than the test's identity.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@milindsrivastava1997
milindsrivastava1997 force-pushed the 474-precompute-wall-clock-e2e-test branch from dbf7a94 to ce9ab06 Compare August 18, 2026 18:12
Base automatically changed from 474-precompute-wall-clock-e2e-test-hook to main August 18, 2026 18:14
milindsrivastava1997 and others added 3 commits August 18, 2026 14:14
Stacked on the wall-clock override hook. Adds JsonFileIngestConfig::
batch_delay_ms (default 0, a no-op everywhere in production and the
existing netflow_single_second e2e test) — a real, explicit delay after
each batch send, so an integration test can force ingest to span real
wall-clock time deterministically instead of hoping incidental scheduling
overhead is enough.

New e2e test drives the real JsonFileIngestSource -> PrecomputeEngine ->
sink pipeline with a bulk single-timestamp load spread across 10 batches,
using the wall-clock override (unscaled — an earlier scaled version
amplified ordinary scheduling jitter into spurious failures) and generous,
seconds-scale margins so the periodic flush timer gets many real chances to
land mid-ingest without the test depending on tight timing. Verified red
against the pre-fix birth-time semantics (temporarily reverted, ran,
confirmed rows were dropped mid-ingest, restored the fix) before counting
it as a real regression test.

Also fixed allowed_lateness_ms in the new test's config: every flush_all()
call unconditionally nudges the event-time watermark forward by 1ms (the
"boundary advance" that lets an idle stream make progress), independent of
the wall-clock fallback. With allowed_lateness_ms=0 and multiple batches
spaced across several flush ticks, that 1ms drift alone was enough to mark
the next same-timestamp batch "late" via a wholly different code path,
unrelated to #474. A generous, production-realistic value avoids that.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two review comments from @zzylol on #538:

- batch_delay_ms's doc comment called it "test-support only," but it lives
  on JsonFileIngestConfig, a real production struct, and nothing about it
  is actually test-gated -- it's an ordinary throttling knob that just has
  no caller setting it nonzero yet. Reworded to describe what it does, not
  who currently uses it.
- The new e2e test's module doc led with "regression test for issue #474."
  Reframed to lead with the behavior/property under test (bulk single-
  timestamp ingest must not lose data while still actively receiving
  samples), keeping the causal explanation of the prior bug as supporting
  context rather than the test's identity.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@milindsrivastava1997
milindsrivastava1997 force-pushed the 474-precompute-wall-clock-e2e-test branch from ce9ab06 to 7ae3b29 Compare August 18, 2026 18:14
@milindsrivastava1997
milindsrivastava1997 merged commit cd0f271 into main Aug 18, 2026
4 checks passed
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.

2 participants