perf(analytics): cut git observation view query memory - #1733
Conversation
The git_metric_observations view runs live on every metric query and its measure branches execute concurrently, so peak memory is per-branch. Pre-aggregate file changes to commit x category grain before the authorship join, so per-file rows and path strings never enter a join side or a measure aggregation. Drop FINAL from the two reads whose uniqExact vote cannot be inflated by duplicate row versions; keep it on the commit and pull-request reads where it is the cheapest correct dedup. Result parity verified; dedup semantics unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Observation views execute live with parallel union branches, so per-query memory scales with thread count. Cap max_threads and set a per-query max_memory_usage in the ClickHouse client config, alongside the existing execution timeout, so one heavy query fails alone with a typed error instead of pushing the shared server memory tracker over its limit and aborting every in-flight query on the instance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughClickHouse clients now support per-query thread and memory limits, with analytics applying explicit bounds. The Git metrics view also changes file-change aggregation and removes unnecessary deduplication qualifiers from author-email election. ChangesClickHouse query resource limits
Git metrics query optimization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AnalyticsApiGear
participant ClickHouseConfig
participant ClickHouseClient
participant QueryHandle
AnalyticsApiGear->>ClickHouseConfig: configure thread and memory limits
ClickHouseConfig->>ClickHouseClient: create client
ClickHouseClient->>QueryHandle: apply max_threads and max_memory_usage
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ingestion/gold/git_metric_observations.sql`:
- Around line 44-55: Clarify the memory-shape comment so only the two
pr_commit_emails reads are described as avoiding FINAL; explicitly state that
file_changes retains FINAL for deduplication, while pre-aggregation only reduces
join and aggregation memory. Update the comment near the references to FINAL,
pr_commit_emails, and file_changes without changing query behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1e1b56f-df05-4647-81dd-ffef7fe052d3
📒 Files selected for processing (4)
src/backend/libs/insight-clickhouse/src/config.rssrc/backend/libs/insight-clickhouse/src/lib.rssrc/backend/services/analytics/src/gear.rssrc/ingestion/gold/git_metric_observations.sql
| -- | ||
| -- Memory shape (this view executes live on every metric query, and its | ||
| -- measure branches run concurrently within one query, so per-branch memory | ||
| -- multiplies): FINAL is the cheapest dedup here — a streaming merge of | ||
| -- sorted parts — and stays wherever dedup is needed. Version-ordered | ||
| -- `ORDER BY .. LIMIT 1 BY` is not an alternative: it buffers a full sort | ||
| -- of the read (measured ~2x the memory of FINAL at scale). The two reads | ||
| -- that avoid FINAL do so because they need no dedup at all: the identity | ||
| -- vote in pr_commit_emails aggregates by uniqExact, which duplicate row | ||
| -- versions cannot inflate. file_changes pre-aggregates to commit x category | ||
| -- grain before joining, so per-file rows and file_path strings never enter | ||
| -- a join side or a measure aggregation. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify that file_changes still uses FINAL; the current wording reads as if it avoids it.
The block introduces "the two reads that avoid FINAL" and then, under the same justification, describes file_changes pre-aggregation. But line 116 retains FINAL on class_git_file_changes, and correctly so: sum(lines_added) over grouped rows is not dedup-safe — stale ReplacingMergeTree versions would inflate the sum. Grouping to commit×category grain reduces join/aggregation memory but does not remove the need for dedup.
The only reads that actually drop FINAL are the two in pr_commit_emails. Recommend tightening the wording so a future reader does not mistakenly remove FINAL at line 116.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ingestion/gold/git_metric_observations.sql` around lines 44 - 55, Clarify
the memory-shape comment so only the two pr_commit_emails reads are described as
avoiding FINAL; explicitly state that file_changes retains FINAL for
deduplication, while pre-aggregation only reduces join and aggregation memory.
Update the comment near the references to FINAL, pr_commit_emails, and
file_changes without changing query behavior.
Cheaper equivalents for three compiled query patterns, results unchanged: - Peer view computes all three quartiles from one quantilesExactIf per metric (a single sort) instead of three separate quantileExactIf calls; min/max come back already-Nullable from *IfOrNull, dropping a redundant toNullable. - Dimension extraction locates the tuple once with indexOf on the key column and reuses the index, replacing three arrayFilter passes over the tuple array per row. - Histogram derives each entity's value bounds with a window pass over the events, replacing the bounds self-join and its second scan of the observation view. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
| .with_query_max_threads(4) | ||
| .with_query_max_memory_bytes(1_610_612_736), | ||
| ); | ||
| let _q = client.query("SELECT 1"); |
There was a problem hiding this comment.
Assert missing in this test.
3 participants
The
git_metric_observationsview runs live on every metric query, and its measure branches execute as concurrent pipelines — so peak memory is per-branch, not per-query. ItsFINALreads and cross-table joins pushed the shared ClickHouse memory tracker over its ceiling under load, aborting queries.View
FINALfrom the two reads whoseuniqExactvote duplicate row versions cannot inflate.FINALstays on the commit and pull-request reads, where it is the cheapest correct dedup.Query bounds
max_threadsand set a per-querymax_memory_usagein the ClickHouse client config, alongside the existing execution timeout. One enforcement point covers metric queries, validator probes, and the legacy query endpoints.Refs #1706.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Performance