Preserve input weight ordering when no weighers are configured#918
Merged
Merged
Conversation
PhilippMatthes
requested review from
SoWieMarkus,
auhlig,
juliusclausnitzer,
mblos and
umswmayj
as code owners
📝 WalkthroughWalkthroughThe PR adds a conditional normalization path to the weight pipeline. When weighers are configured, input weights undergo ChangesConditional Weight Normalization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
PhilippMatthes
changed the title
Contributor
Test Coverage ReportTest Coverage 📊: 69.5% |
PhilippMatthes
added a commit
that referenced
this pull request
Jun 8, 2026
## 2026-06-08 — [#919](#919) ### cortex v0.1.0 (sha-a0373875) Breaking changes: - `Client.AddRemote()` signature changed — added `insecureSkipTLSVerify bool` parameter between `caCert` and `labels`. Any external caller of `pkg/multicluster.Client.AddRemote` must update their call site. ([#911](#911)) Non-breaking changes: - Cleanup candidate reservations when confirming VM ([#871](#871)) - Add `InsecureSkipTLSVerify` option to multicluster `RemoteConfig` ([#911](#911)) - Add KVM HANA stacking KPI ([#905](#905)) - Preserve input weight ordering when no weighers are configured ([#918](#918)) - Make nova alerts region- and value-aware ([#902](#902)) - Update cpu steal time query ([#904](#904)) - Bump datasource parallel reconciles to 3 to reduce queue lag ([#907](#907)) - Move `prometheusDatasourceControllerParallelReconciles` value from secrets to bundle ([#912](#912)) - Update `github.com/sapcc/go-bits` ([#903](#903), [#913](#913), [#915](#915)) - Update External dependencies ([#908](#908), [#910](#910), [#914](#914)) - Update kube-prometheus-stack Docker tag to v86 ([#895](#895)) ### cortex-crds v0.0.74 Includes updated chart cortex v0.1.0. ### cortex-nova v0.0.74 Includes updated chart cortex v0.1.0. ### cortex-cinder v0.0.74 Includes updated chart cortex v0.1.0. ### cortex-pods v0.0.74 Includes updated chart cortex v0.1.0. ### cortex-ironcore v0.0.74 Includes updated chart cortex v0.1.0. ### cortex-manila v0.0.74 Includes updated chart cortex v0.1.0. --- **Related PRs:** - Bump PR: [#920](#920) - Changelog PR: [#921](#921)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
filterWeigherPipeline.Run always applied math.Tanh to the requester-supplied input weights via normalizeInputWeights. With high Nova-style values (e.g. 50, 55, 60) the tanh output saturates to ~1.0 within machine epsilon for every input — already at tanh(20) the result is indistinguishable from 1.0 in float64. When the pipeline has no weighers configured, those saturated values flowed straight into sortHostsByWeights, whose comparator is not stable under ties, so Go's sort.Slice returned an order driven by randomized map iteration and the requester's original host ordering was lost.
The fix gates normalizeInputWeights on len(p.weighers) > 0 inside Run. When weighers are configured the behavior is unchanged: input weights are tanh-normalized so they live on a comparable scale with the (also tanh-bounded) weigher contributions. When no weighers are configured there is nothing on the other side of the scale to combine with, so we clone the raw request weights and keep the original ordering intact. Filters do not contribute weights — they only remove hosts — so they are deliberately not part of the condition.
Covered by TestPipeline_Run_NoWeighers_PreservesInputOrdering, which runs the empty pipeline 50 times against weights {host1: 50, host2: 55, host3: 60} and asserts the descending order [host3, host2, host1] holds every iteration; without the fix this test fails intermittently due to map iteration order.
Assisted-by: Claude Code:claude-opus-4-5 [Bash] [Read]