bhouse-nexthop opened a new pull request, #14111:
URL: https://github.com/apache/cloudstack/pull/14111
> [!WARNING]
> **Draft — stacked on two other PRs. Please do not review the whole diff.**
>
> This branch contains the commits from #14108 and #14109 as well as its
own, because it depends on
> both. Only the **last four commits** are this PR:
>
> | Commit | |
> |---|---|
> | `Stop DRS planning from redoing the same work` | performance |
> | `Add 'weighted' DRS algorithm` | the feature |
> | `Fix defects found in review of the weighted DRS algorithm` | review
fixes |
> | `Verify equivalent VMs share one candidate host lookup` | test |
>
> Everything before those belongs to:
>
> - **#14108** — DRS anti-affinity fixes (targets `4.22`; the commits appear
here cherry-picked
> forward, and will disappear from this diff once that merges forward)
> - **#14109** — the `balancedweighted` allocation algorithm, which this
reuses for its
> utilisation figures
>
> It will be taken out of draft and rebased once both have merged. Raising
it now so the design can
> be discussed alongside them, since the three are easier to judge together
than apart.
### Description
Adds an opt-in DRS algorithm, `weighted`, for `drs.algorithm`, alongside
`balanced` and `condensed`.
The default is unchanged and the existing algorithms are untouched.
**The problem.** The existing algorithms balance a single metric chosen by
`drs.metric`, so whichever
is not chosen goes unwatched. A cluster can sit comfortably inside its
imbalance threshold on memory
while CPU load varies several fold across its hosts, and DRS will correctly
do nothing.
Allocation is also a poor stand-in for load under overprovisioning.
Allocated figures are measured
against a total that has already been multiplied by the overprovisioning
factor, so a host whose
cores are saturated can still report a small percentage allocated.
**What it balances.** A blend of four figures per host - CPU and memory
allocated, CPU and memory in
use:
```
w_ca*cpu_alloc + w_ma*mem_alloc + w_cu*cpu_util + w_mu*mem_util
metric(h) = -------------------------------------------------------------
w_ca + w_ma + w_cu + w_mu
imbalance = stddev( metric(h) for h in cluster ) / mean( metric(h) )
```
where the allocated fractions are measured against what a host can hand out
- its total scaled by
the cluster's overcommit ratio, with reserved capacity taken off that.
Imbalance keeps the shape the other algorithms use, so `drs.imbalance` still
behaves the same way,
but it is now computed over a blend rather than over one metric and is
therefore **not calibrated
the same**. Worth re-checking that setting after switching.
`drs.metric`, `drs.metric.type` and `drs.metric.use.ratio` choose and shape
the single metric the
other algorithms balance. They do not apply here and are ignored.
**Utilisation is used only when every host in the cluster has been
sampled.** Imbalance compares
hosts against each other, so measuring one host on a blend and another on
allocation alone would
report a difference that is an artefact of the monitoring rather than of the
load - and would
evacuate whichever host had stopped reporting. When any host is unmeasured
the cluster falls back to
allocation figures for all of them.
**Cost and benefit.** Both are expressed in units of imbalance, so the
caller's `benefit > cost` test
means "is this worth what it costs". A migration that has to move storage
has to earn more than one
that does not, by a configurable margin.
### Settings
| Setting | Default | Scope |
|---|---|---|
| `drs.weighted.cpu.allocated.weight` | `1.0` | Cluster |
| `drs.weighted.cpu.used.weight` | `2.0` | Cluster |
| `drs.weighted.memory.allocated.weight` | `1.0` | Cluster |
| `drs.weighted.memory.used.weight` | `2.0` | Cluster |
| `drs.weighted.storage.motion.cost` | `0.02` | Cluster |
Weights are relative to each other; `0` disables a term.
### Also: DRS plan generation is much faster
Independent of the algorithm, and useful on its own. Plan generation took
minutes on a large
cluster, for two reasons that are both about repeating work:
- Working out where a VM could go runs the host allocators and inspects
every volume, and it was
done once per VM. VMs that would get the same answer now share one pass. A
VM is only grouped when
it carries none of the per-VM inputs that change the answer - a custom
offering whose size comes
from the VM, a boot mode, a device setting - rather than trying to
enumerate everything that could
matter and risk missing one.
- Affinity was re-evaluated for every VM in the cluster on every iteration,
up to
`drs.max.migrations` times. Only VMs sharing a group with the one just
moved can have changed.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] Build/CI
- [ ] Test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [x] Major
- [ ] Minor
### How Has This Been Tested?
- `WeightedTest` - 13 cases: an even cluster reads as balanced; CPU load
imbalance is seen when
memory is even and vice versa; allocation beyond what a host can hand out
stays distinguishable;
reserved capacity does not make a host look fuller; utilisation is dropped
when any host cannot be
measured and used when all can; moving a VM off the busier host is an
improvement and onto it is
not; storage motion has to earn more.
- `ClusterDrsEquivalenceTest` - 11 cases on what may and may not share a
candidate-host lookup,
including custom offerings and boot modes, which must never be grouped.
- `ClusterDrsServiceImplTest` - 34 cases, including one asserting that two
interchangeable VMs cost
one lookup rather than two, so the grouping cannot be silently disabled.
Full `mvn test` on `api`, `server` and all three DRS plugins with checkstyle
and license checks
enabled: 0 failures.
#### How did you try to break this feature and the system with this change?
Adversarial review found four defects that would have made this unusable,
all fixed:
- **The plugin had no `module.properties`**, so it was never discovered,
never instantiated and
never registered. Selecting it would have failed every DRS run with
"Invalid algorithm
configured".
- **It injected a service from a sibling Spring module**, which is not
visible to it. Once the
module did load, the management server would have failed to start - for
anyone with the plugin on
the classpath, regardless of configuration. The caller now works out the
load once per plan and
passes it in.
- **`getMetrics` read two cluster details and four settings from the
database on every call**, and
it is called for every candidate VM and host - hundreds of thousands of
times for a large cluster.
That would have swamped the performance work in the same PR.
- **Grouping VMs was unsound**: the key enumerated the inputs it thought
mattered and missed several
that are held per VM rather than per offering. Two VMs could share a host
list neither should have
had. Inverted to only group VMs that provably carry none of them.
Also found and fixed: reserved capacity was multiplied by the overcommit
ratio instead of subtracted
from the scaled total, so a host 11% fuller than its peer read 67% fuller
and DRS would have
evacuated it for no reason; hosts with and without utilisation samples were
compared on different
bases; and the cost and benefit terms cancelled out, making storage motion
free despite the claim
otherwise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
