Dennis-Mircea opened a new pull request, #1179:
URL: https://github.com/apache/flink-kubernetes-operator/pull/1179

   ## What is the purpose of the change
   
   A source vertex can be scaled far beyond its partition count. Every subtask 
above that count is assigned no split and stays idle, together with anything 
chained onto the source, so the slots are paid for and do no work.
   
   Reproduction: Kafka source with 15 partitions, current parallelism 100, 
operator max parallelism 180. Values are what `JobVertexScaler#scale` returns.
   
   | raw target | new default (BALANCED) | legacy default (EVENLY_SPREAD) | 
correct |
   |---|---|---|---|
   | 180 (scale up) | 180 | 100 (scale blocked) | 15 |
   | 50 (scale down) | 50 | 15 | 15 |
   | 10 (scale down) | 15 | 15 | 15 |
   
   Only the third row, where the target already sits at or below the partition 
count, is handled correctly by either.
   
   ## Brief change log
   
   Two defects, one fix each, both inside the alignment package.
   
   **The built-in modes cap the search region but not the result.** 
`firstAlignedInRegion` scans `[target, regionEnd]` derived from 
`upperBoundForAlignment = min(N, maxParallelism, parallelismUpperLimit)`. When 
the target is already above that region the loop body never runs and the search 
returns 0, and `alignOrKeepTarget` then keeps the unaligned target. It now 
bounds that kept target by the cap.
   
   **The legacy modes reach the right value and then discard it.** 
`relaxedDownwardFallback` correctly arrives at 15, but `invertsDirection` 
(added in FLINK-39299 to stop the search drifting past `currentParallelism` and 
silently inverting a scale) cannot tell that apart from a candidate that is 
legitimately far below current because the partition count is a hard ceiling. 
Landing exactly on the cap is no longer treated as an inversion. Only downwards 
though, since `relaxedDownwardFallback` clamps up to `parallelismLowerLimit` 
and can land on a cap above the current parallelism, which would turn a 
requested scale-down into a scale-up.
   
   The cap is a no-op for keyed vertices. `scale()` clamps the target to 
`min(maxParallelism, parallelismUpperLimit)` before calling the aligner, so 
`target > upperBoundForAlignment` is only reachable when `N` is the binding 
term, and for a keyed vertex `N == maxParallelism`. Sources are the only case 
where `N` is independent of that clamp. The second fix also only affects 
scale-ups: on a scale-down the candidate already sits below the current 
parallelism, so it was never read as an inversion, which is why the legacy 
modes got the scale-down rows right to begin with.
   
   `OFF` overrides `alignParallelism` directly and is untouched, so explicitly 
disabling alignment still uses the computed target as-is.
   
   An earlier attempt clamped the target in `JobVertexScaler#scale` instead. It 
produced the same parallelism but suppressed the `SCALING_LIMITED` event, 
because alignment then saw a target already equal to the cap and reported no 
deviation, regressing `JobVertexScalerTest#testSendingScalingLimitedEvents`. 
Fixing this inside the aligner keeps that event intact for the legacy modes.
   
   ## Verifying this change
   
   Coverage is a mode × direction matrix over all four aligning modes 
(`BALANCED`, `EVENLY_SPREAD`, and both legacy modes), each combination a 
separate parameterized invocation so no leg is masked by an earlier failing 
assertion in the same method.
   
   Added to `AlignmentModeTest`:
   
   - `targetAbovePartitionCountIsCappedInEitherDirection`, every mode against a 
target above the cap on a scale-up (180) and on a scale-down (50, 16), plus a 
target already on the cap (15) that has to be left alone
   - `offKeepsTheTargetAbovePartitionCount`, `OFF` opts out in both directions
   - `keyedVertexIsCappedAtItsKeyGroupCount`, the `numSourcePartitions == 0` 
path where `N` falls back to `maxParallelism`
   - `legacyEmitsWheneverCappedAtPartitionCount`, `SCALING_LIMITED` for every 
capped target
   - `alreadyAtThePartitionCountBlocksAScaleUp`, running exactly on the cap
   - `movingUpToTheCapOnAScaleDownIsStillAnInversion` and 
`lowerLimitClampAboveCurrentStillBlocksAScaleDown`, the scale-down inversions 
the exemption must leave alone, one landing on the cap and one below it
   
   Added to `JobVertexScalerTest`: 
`testSourceCappedAtPartitionCountWhenOverProvisioned`, the reproduction 
end-to-end through `scale()`, under both legacy modes, both built-in modes, and 
`OFF`. This is the level the table above is stated at, and the module's other 
alignment tests all run with the legacy mode pinned in `setup()`, so the new 
default had no end-to-end coverage.
   
   Each fix was reverted in isolation to confirm the tests catch it:
   
   | reverted | failing tests |
   |---|---|
   | built-in clamp | the end-to-end case under `BALANCED`, plus 8 aligner 
cases covering both built-in modes at targets 180, 50 and 16 |
   | inversion exemption | the end-to-end case under the legacy mode, plus 4 
aligner cases covering both legacy modes on the scale-up |
   
   `legacyModesBlockAndEmit` covers the FLINK-39299 scale-up inversion and 
still blocks at 22. Full `flink-autoscaler` suite passes: 289 tests, 257 
pre-existing plus 32 added.
   
   ## Does this pull request potentially affect one of the following parts
   
   - Dependencies (does it add or upgrade a dependency): no
   - The public API, i.e., is any changed class annotated with 
`@Public(Evolving)`: no
   - Core observer or reconciler logic that is regularly executed: yes, the 
autoscaler alignment path
   
   ## Documentation
   
   - Does this pull request introduce a new feature? no
   - If yes, how is the feature documented? not applicable
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   <!--
   If generative AI tooling has been used in the process of authoring this PR, 
please
   change the checkbox below to `[X]` followed by the name of the tool, and 
uncomment the
   "Generated-by" line. See the ASF Generative Tooling Guidance for details:
   https://www.apache.org/legal/generative-tooling.html
   
   You are responsible for the quality and correctness of every change in this 
PR
   regardless of the tooling used. Low-effort AI-generated PRs will be closed. 
See
   AGENTS.md for the full guidance.
   -->
   
   - [X] Yes (please specify the tool below)
   
   Generated-by: [Claude Opus]
   
   


-- 
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]

Reply via email to