Skip to content

feat: add millisecond precision support for planner and query engine#437

Merged
milindsrivastava1997 merged 15 commits into
mainfrom
398-featasap-planner-rs-add-millisecond-precision-support-for-sub-second-scrape-intervals
Jun 26, 2026
Merged

feat: add millisecond precision support for planner and query engine#437
milindsrivastava1997 merged 15 commits into
mainfrom
398-featasap-planner-rs-add-millisecond-precision-support-for-sub-second-scrape-intervals

Conversation

@milindsrivastava1997

@milindsrivastava1997 milindsrivastava1997 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor
  • feat(planner): added millisecond precision support
  • feat: added millisecond-level precision support in planner and query engine

@milindsrivastava1997 milindsrivastava1997 linked an issue Jun 24, 2026 that may be closed by this pull request
feat(asap-planner-rs): add millisecond precision support for sub-second scrape intervals #398
Closed
milindsrivastava1997
milindsrivastava1997 commented Jun 25, 2026
View reviewed changes
Comment thread asap-query-engine/src/engines/simple_engine/mod.rs
Comment thread asap-query-engine/src/engines/simple_engine/sql.rs
Comment thread asap-tools/experiments/experiment_utils/config.py
Comment thread asap-planner-rs/src/query_log/frequency.rs
Comment thread asap-planner-rs/src/planner/sql.rs
@milindsrivastava1997 milindsrivastava1997 mentioned this pull request Jun 25, 2026
Closed
@milindsrivastava1997 @claude
docs(query-engine,planner): clarify unit ambiguities raised in PR #437 …
5e5be89 
…review

- StoreQueryParams/QueryTimestamps: doc start/end fields as ms since epoch
- frequency.rs step_ms: inline comment that LogEntry.step is seconds (Prometheus log format), explaining the *1000
- sql.rs SQLPatternMatcher boundary and config.py dual-key transform: confirmed intentional, already documented; no change needed
- SQLPatternMatcher ms migration partially tracked by #427; prometheus_client ms migration tracked by #442

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@milindsrivastava1997 milindsrivastava1997 mentioned this pull request Jun 25, 2026
Closed
@milindsrivastava1997 milindsrivastava1997 marked this pull request as ready for review June 25, 2026 16:00
@milindsrivastava1997 @claude
fix(config): remove stale do_profiling field from engine config YAMLs
330db67 
PR #343 removed do_profiling from EngineConfig (deny_unknown_fields) but
missed updating benchmarks/config/engine_config.yaml and
examples/engine_config.yaml. The field causes the query engine to fail
at startup, which broke the full-stack CI.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@milindsrivastava1997

milindsrivastava1997 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Asymmetric Python public API (judgement call)

generate_controller_compose.py renamed its public CLI arg to --prometheus-scrape-interval-ms (milliseconds), but experiment_utils/services/misc.py's start() still takes prometheus_scrape_interval in seconds and converts internally. Two tools in the same experiment_utils package now follow opposite conventions for the same concept — one ms at the boundary, one seconds at the boundary.

@milindsrivastava1997

milindsrivastava1997 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Constant names no longer match their values (judgement call)

In asap-planner-rs/src/generator.rs, the string constants KEY_SLIDE_INTERVAL and KEY_WINDOW_SIZE now hold the values "slideIntervalMs" and "windowSizeMs" respectively. The constant names are missing the _MS suffix, so the name no longer describes the value — opposite to the naming discipline this PR is establishing everywhere else.

@milindsrivastava1997

milindsrivastava1997 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Legacy tumblingWindowSize keys left in benchmark fixtures (judgement call)

asap-tools/execution-utilities/asap_benchmark_pipeline/streaming_config.yaml and asap-tools/execution-utilities/elastic-asap-benchmarking/streaming_config.yaml still contain tumblingWindowSize: 10 alongside the new windowSizeMs. The key is no longer read, so it's harmless debris — but it contradicts the PR's goal of cleaning up unit-ambiguous field names.

@milindsrivastava1997 milindsrivastava1997 marked this pull request as draft June 25, 2026 21:09
@milindsrivastava1997 milindsrivastava1997 marked this pull request as ready for review June 26, 2026 12:46
@milindsrivastava1997
fixed bug in CI due: benchmarks/docker-compose.yml overrides the imag…
c400a81 
…e to the freshly built SHA tag but doesn't override the command, so the new binary (which uses _ms arg names) received the old arg names from asap-quickstart/docker-compose.yml
@milindsrivastava1997
more CI fixes because cmd line args have been changed
426ef8e
@milindsrivastava1997 milindsrivastava1997 merged commit 2051ee0 into main Jun 26, 2026
28 checks passed
@milindsrivastava1997 milindsrivastava1997 deleted the 398-featasap-planner-rs-add-millisecond-precision-support-for-sub-second-scrape-intervals branch June 26, 2026 13:27
@milindsrivastava1997 milindsrivastava1997 mentioned this pull request Jun 26, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat(asap-planner-rs): add millisecond precision support for sub-second scrape intervals

1 participant

@milindsrivastava1997