Skip to content

fix(controller): sum AdditionalPU from all active schedules for desiredMinPUs#215

Merged
rustycl0ck merged 2 commits into
masterfrom
fix/schedule-desired-min-pu-sum
Apr 22, 2026
Merged

fix(controller): sum AdditionalPU from all active schedules for desiredMinPUs#215
rustycl0ck merged 2 commits into
masterfrom
fix/schedule-desired-min-pu-sum

Conversation

@tkuchiki
Copy link
Copy Markdown
Contributor

What this PR does

  • Fixes calcDesiredPURange to use sum (instead of min) when computing desiredMinPUs from active schedules, matching the existing desiredMaxPUs logic
  • Replaces the exactlyOneOfBools helper in validateTargetCPUUtilization with a direct XOR boolean expression

Why we need it

When multiple SpannerAutoscaleSchedule windows overlap, desiredMinPUs was computed by taking the minimum AdditionalPU across all active schedules. This meant that if a later schedule had a smaller AdditionalPU than an earlier one, it would overwrite the effective minimum and silently reduce the PU floor — even during the earlier schedule's active window.

Example

Schedule Window AdditionalPU
A 6:40–8:20 +5,000
B 7:20–8:20 +1,000

At 7:20, when both A and B are active:

  • Before: desiredMinPUs = spec.min + min(5000, 1000) = spec.min + 1000 — schedule A's +5,000 is overwritten by B's smaller value, causing an unintended scale-down
  • After: desiredMinPUs = spec.min + (5000 + 1000) = spec.min + 6,000 — all schedules contribute independently

desiredMaxPUs already used sum and is unchanged. Users who avoided overlapping schedule windows as a workaround are unaffected.

Test plan

  • Added unit tests for calcDesiredPURange (no schedules, single schedule, overlapping schedules, rounding, no-change detection)
  • make test passes

🤖 Generated with Claude Code

tkuchiki and others added 2 commits April 20, 2026 16:08
…edMinPUs

Previously calcDesiredPURange used min() across active schedules for
desiredMinPUs, so when schedules with different AdditionalPU values
overlapped, only the smallest was applied. This caused the autoscaler
to silently ignore larger schedules and even scale down unexpectedly.

Change to sum (matching the existing desiredMaxPUs logic) so every
active schedule's contribution is counted independently.

Also add unit tests for calcDesiredPURange, which had no test coverage.

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

Replace the exactlyOneOfBools helper (variadic, general-purpose) with a
direct boolean XOR expression. The function only ever had two fields to
check, so the generality added no value.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@tkuchiki tkuchiki marked this pull request as ready for review April 20, 2026 07:18
@tkuchiki tkuchiki requested a review from a team as a code owner April 20, 2026 07:18
@rustycl0ck rustycl0ck added release/patch Indicates patch update to action-release-label and removed release/patch Indicates patch update to action-release-label labels Apr 22, 2026
@rustycl0ck rustycl0ck merged commit e5302a2 into master Apr 22, 2026
5 checks passed
@rustycl0ck rustycl0ck deleted the fix/schedule-desired-min-pu-sum branch April 22, 2026 02:02
@github-actions
Copy link
Copy Markdown

A tag with version v0.7.1 has been created! 🎉

Changes: v0.7.0...v0.7.1

@github-actions
Copy link
Copy Markdown

A new github release with version v0.7.1 has been created! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release/patch Indicates patch update to action-release-label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants