fix(gold): spillable joins and headroom for deploy-time builds by aleksdotbar · Pull Request #1896 · constructorfabric/insight

aleksdotbar · 2026-07-24T08:32:48Z

The deploy-time gold build can exceed its query memory cap on large
instances: the external group-by/sort thresholds spill aggregation and
sort state, but hash-join tables spill under neither, so join-heavy
observation models fail outright once they outgrow the cap.

join_algorithm: grace_hash,hash — joins partition to disk under
pressure; plain hash fallback for join shapes grace does not support.
Memory cap 1.5 → 3 GiB for spill overhead headroom.
One uniform settings block on every gold observation model; the ai
model — the last observation view — is materialized as a sorted
table like its siblings, and wiki gains the bounds it shipped
without.
Helm --atomic timeout 10m → 30m: it bounds rollout waits plus the
migrate hook, and spilling builds are deliberately slower — dbt's
per-query timeout (25m) was pointless with helm rolling back at 10.

Settings verified to reach ClickHouse on the build inserts; results
unchanged.

🤖 Generated with Claude Code

Summary by CodeRabbit

Performance
- Improved reliability of data model builds by tuning execution limits (including higher memory availability) and using a more robust join strategy.
- Added/updated query settings to better handle scenarios where workloads exceed memory, including spill behavior for aggregation, sorting, and joins.
- Converted AI metric observations from a view to a persistent table for more efficient access.
Bug Fixes
- Extended deployment operation timeout to allow longer rollout and migration steps to complete before rollback.

The git observation build exceeded its 1.5 GiB query cap on a grown instance: the external group-by/sort thresholds spill aggregation and sort state, but hash-join tables spill under neither, so the join-heavy models fail outright once their build outgrows the cap. Set join_algorithm to grace_hash (hash fallback for join shapes grace does not support) so joins partition to disk under pressure, raise the cap to 3 GiB for spill overhead headroom, and apply one uniform settings block to every table-materialized gold model — wiki had shipped with no bounds at all. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>

coderabbitai · 2026-07-24T08:33:07Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 99a527eb-64ab-4ebf-9a8b-db7e44e3953e

📥 Commits

Reviewing files that changed from the base of the PR and between b987243 and a6244b1.

📒 Files selected for processing (1)

deploy/gitops/Makefile

🚧 Files skipped from review as they are similar to previous changes (1)

deploy/gitops/Makefile

📝 Walkthrough

Walkthrough

The change extends the default Helm timeout and updates ClickHouse gold ingestion models with table materialization and explicit resource, spill, and join execution settings.

Changes

Deployment timing

Layer / File(s)	Summary
Extend Helm deployment timeout `deploy/gitops/Makefile`	The default Helm timeout increases from 10 minutes to 30 minutes, with documentation covering rollout waits and the ClickHouse migration hook.

Gold execution tuning

Layer / File(s)	Summary
Materialize AI observations `src/ingestion/gold/ai_metric_observations.sql`	The AI observations model changes from a view to a MergeTree table and adds ordering, schema, and explicit ClickHouse query settings.
Standardize ClickHouse query settings `src/ingestion/gold/{collab_metric_observations,git_metric_observations,task_issue_state,task_metric_observations,task_status_spans,wiki_metric_observations}.sql`	Gold models update memory limits, external aggregation/sort thresholds, and join algorithms; spill comments include join state where applicable.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

constructorfabric/insight#1480: Adds the ClickHouse migration hook connected to the extended Helm timeout.
constructorfabric/insight#1733: Modifies ClickHouse query execution and resource limits for the Git metrics model.
constructorfabric/insight#1757: Updates the collaboration metrics model touched by these execution-setting changes.

Suggested reviewers: ktursunov

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly reflects the main change: enabling spillable joins and increasing memory headroom for deploy-time gold builds.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/gold-build-memory

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

The ai model was the one observation view left; every other family is a sorted table. Same materialization, ordering key, and bounded-build settings as its siblings, so runtime queries index-prune instead of re-deriving from silver per request. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>

The 10m --atomic timeout bounds rollout waits plus the post-upgrade clickhouse-migrate hook. Gold builds now spill to disk under memory pressure — deliberately slower — and dbt's own per-query timeout is 25m, so helm was the binding constraint that would roll back a slow but succeeding build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>

coderabbitai

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 `@deploy/gitops/Makefile`:
- Around line 57-60: Update the deployment guidance in the GitOps README to
document TIMEOUT ?= 30m, matching the default defined in the Makefile and used
by deploy-insight’s --timeout option. Preserve the surrounding deployment
instructions and examples.

🪄 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 Plus

Run ID: f3415403-440f-45d2-8905-5a68e89daa6c

📥 Commits

Reviewing files that changed from the base of the PR and between 38dcb05 and b987243.

📒 Files selected for processing (8)

deploy/gitops/Makefile
src/ingestion/gold/ai_metric_observations.sql
src/ingestion/gold/collab_metric_observations.sql
src/ingestion/gold/git_metric_observations.sql
src/ingestion/gold/task_issue_state.sql
src/ingestion/gold/task_metric_observations.sql
src/ingestion/gold/task_status_spans.sql
src/ingestion/gold/wiki_metric_observations.sql

coderabbitai · 2026-07-27T10:33:51Z

+# Covers rollout waits plus the post-upgrade clickhouse-migrate hook; the
+# gold build inside that hook spills to disk under memory pressure, which
+# trades speed for reliability — give it room before --atomic rolls back.
+TIMEOUT          ?= 30m


📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the documented default timeout.

Line 60 now sets TIMEOUT to 30m, but docs/components/deployment/gitops/README.md:488-506 still documents TIMEOUT ?= 10m. Update the README so deployment guidance matches the actual --timeout used by deploy-insight.

🤖 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 `@deploy/gitops/Makefile` around lines 57 - 60, Update the deployment guidance in the GitOps README to document TIMEOUT ?= 30m, matching the default defined in the Makefile and used by deploy-insight’s --timeout option. Preserve the surrounding deployment instructions and examples.

github-actions · 2026-07-27T10:35:25Z

⚠️ Regenerate the connectors-ddl snapshot

This PR changes src/ingestion/**. If your change affects any
bronze / silver / gold schema, regenerate the committed DDL snapshot
and include it in this PR:

cd src/ingestion/scripts/bootstrap-db
set -a; source pins.env; source .env; set +a
./bootstrap-db.sh connectors-config.yaml   # fresh ClickHouse 25.7.5
./dump-ddl.sh                              # writes scripts/connectors-ddl/*.sql

Commit the resulting scripts/connectors-ddl/*.sql diff. If nothing
changed, no snapshot update is needed. (Regeneration is manual for now.)

aleksdotbar requested a review from a team as a code owner July 24, 2026 08:32

aleksdotbar and others added 5 commits July 24, 2026 10:34

Merge branch 'main' into fix/gold-build-memory

bca4d27

Merge branch 'main' into fix/gold-build-memory

039eadd

Merge branch 'main' into fix/gold-build-memory

b987243

aleksdotbar enabled auto-merge July 27, 2026 10:29

coderabbitai Bot reviewed Jul 27, 2026

View reviewed changes

mitasovr approved these changes Jul 27, 2026

View reviewed changes

Merge branch 'main' into fix/gold-build-memory

a6244b1

aleksdotbar merged commit 6068e88 into main Jul 27, 2026
35 checks passed

coderabbitai Bot mentioned this pull request Aug 3, 2026

fix: prevent metric gold build memory failures #2144

Merged

Navigation Menu

Uh oh!

fix(gold): spillable joins and headroom for deploy-time builds - #1896

fix(gold): spillable joins and headroom for deploy-time builds#1896
aleksdotbar merged 7 commits into
mainfrom
fix/gold-build-memory

aleksdotbar commented Jul 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jul 27, 2026

Uh oh!

github-actions Bot commented Jul 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

aleksdotbar commented Jul 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 27, 2026

⚠️ Regenerate the connectors-ddl snapshot

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aleksdotbar commented Jul 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading