Skip to content

fix(asap-tools): keep 10 s more engine history for late query sends - #893

Merged
milindsrivastava1997 merged 1 commit into
mainfrom
fix/retention-slack-client-lag
Oct 15, 2026
Merged

milindsrivastava1997 merged 1 commit into
mainfrom
fix/retention-slack-client-lag

Conversation

@milindsrivastava1997

Copy link
Copy Markdown
Contributor

Before this PR

get_retention_slack_ms gave the query engine extra history equal to the client's largest query_time_offset plus one ingestion interval: 11 s for the sketch-bench e2e configs (offset 10 s, 1 s interval). That leaves no room for the client sending late.

In a classic sketch-bench smoke run (#824), every minute the 60s query group (46 queries, several returning 10,000 series) fires alongside the 1s group. The 1s queries then go out 2–3 s behind schedule, about 12.1–12.5 s after their evaluation time. With 1 planned window + 11 s slack, the 1s DDSketch kept 12 windows (time_map_len=12), so the window was already evicted. The store lookup returned 1 windows requested, 0 matched, and 7 of 300 instant-quantile evaluations came back empty (destination=none_unsupported).

After this PR

  • CLIENT_SEND_LAG_SLACK_MS = 10_000 is added to the slack, so a query sent up to ~10 s behind schedule still finds its window. It applies to every runner that uses get_retention_slack_ms (e2e, ClickHouse, Grafana demo).
  • Memory cost: ~10 more retained windows per short-window aggregation.

Validation

  • tests/test_retention_slack.py: updated expectations, plus a regression test for a query 12.4 s past its evaluation time with a 10 s offset. It fails on the old slack.
  • CloudLab rerun of the smoke config to follow in a comment (expect 0 empty instant quantiles).

🤖 Generated with Claude Code

The query engine's retention slack covered the client's query_time_offset
plus one ingestion interval, with no room for the client sending late. When
a 60s query group fires alongside a 1s group, the 1s queries go out ~2-3 s
behind schedule, and a 1s-window instant query then found its window already
evicted (7 of 300 evaluations in a classic sketch-bench smoke run).

Add CLIENT_SEND_LAG_SLACK_MS (10 s) to get_retention_slack_ms.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@milindsrivastava1997
milindsrivastava1997 merged commit 1eebdba into main Oct 15, 2026
7 of 11 checks passed
@milindsrivastava1997
milindsrivastava1997 deleted the fix/retention-slack-client-lag branch October 15, 2026 01:06
@milindsrivastava1997

Copy link
Copy Markdown
Contributor Author

CloudLab verification (nodes 2–3, classic sketch-bench table, 1 sample/s per series, 20 min starting delay, 300 s of queries, ASAPQuery only):

  • The engine config has retention_slack_ms: 21000 (11000 before).
  • Instant quantiles: 300 of 300 evaluations non-empty for each of the 5 quantile levels. The same config before this change had 7 of 300 empty, and 10 of 300 with debug logging.
  • The client still sent queries up to 12.5 s after their evaluation time, about as late as before. They are now within the retained history.
  • The only remaining none_unsupported results (90) are the 1h/6h/24h quantile_over_time queries, which can't be answered until the run has that much data.

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