Skip to content

fix(replica): no-issue: let reporting-schema replicas fit a replica node - #167

Merged
passcod merged 2 commits into
mainfrom
fix/reporting-schema-cpu-floor
Oct 19, 2026
Merged

passcod merged 2 commits into
mainfrom
fix/reporting-schema-cpu-floor

Conversation

@dannash100

@dannash100 dannash100 commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Reporting-schema builds restore, then sit Pending forever: the Postgres pod requests 2 CPU, and the db-replica pool's largest node (x8g.large / r6g.large) has 1930m allocatable. Karpenter reports no instance type fits.

  • Request 500m with no CPU limit, the same shape as upgrade, so dbt bursts into the rest of the node
  • Nothing pins one replica per node, so builds can share a node; that only slows them, it never blocks scheduling
  • Seen on the internal test group's 2.54.14 build today

🦸 Review Hero

  • Run Review Hero
  • Run Review Hero on Claude (before merge) Final review with Claude just before merging a substantive PR. Tick one or the other.

@dannash100
dannash100 requested a review from passcod October 18, 2026 22:27
Comment thread src/controllers/canopy/intent.rs
@review-hero

review-hero Bot commented Oct 18, 2026

Copy link
Copy Markdown

🦸 Review Hero Summary (round 1) · reviewed 776bbe6 · on z-ai/glm-5.3-flash:nitro
3 agents reviewed this PR | 0 critical | 1 suggestion | 0 nitpicks | Filtering: consensus 3 voters, 1 below threshold

Below consensus threshold (1 unique issue not confirmed by majority)
Location Agent Severity Comment
src/controllers/canopy/intent.rs:561 Bugs & Correctness suggestion The stated goal is that the pod fits a 2 vCPU replica node, but only the CPU request achieves that: the memory request is snapshot-derived in resolve_postgres_resources (src/controllers/restore/b...
Local fix prompt (copy to your coding agent)
Fix these issues identified on the pull request. One commit per issue fixed.

-------

`src/controllers/canopy/intent.rs:559`: The new comment asserts "the build bursts into the rest of the node, which it has alone", but nothing in the code enforces that exclusivity: the spec is built with `affinity: None` and no topology spread (see PostgresPhysicalReplicaSpec in to_replica_spec), and scheduling only considers requests. With the request lowered from 2 to 500m CPU, the scheduler will happily pack up to ~4 reporting-schema replicas on the same 2-vCPU node, and with no CPU limit each bursty dbt build competes for the cores the comment says it owns. Under the old 2-CPU request the packing was implicitly prevented. Either soften the comment to acknowledge co-tenancy is possible, or add anti-affinity/spread for reporting-schema replicas if node exclusivity is actually intended.

@passcod
passcod merged commit c75641e into main Oct 19, 2026
23 checks passed
@passcod
passcod deleted the fix/reporting-schema-cpu-floor branch October 19, 2026 02:37
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