acgtun commented on PR #24091: URL: https://github.com/apache/kafka/pull/24091#issuecomment-6588516859
Supplemental: manual verification on a 3-controller / 7-broker KRaft cluster (not part of the commit message) Two images from the same trunk commit (`da5316228e`, with and without this patch), each deployed as 3 KRaft controllers + 7 brokers (A=100 … G=106) on minikube, 4.5.0-SNAPSHOT. Broker D (103) was held out of the ISR with a 1 B/s follower throttle so reassignments stayed in flight while the controller state was inspected with `kafka-reassign-partitions.sh --list` and `kafka-topics.sh --describe`. Every partition starts as `[A,B,C]` → `[A,B,D]` with `adding=[D]`; the rows below are the overrides issued with `--execute --additional` while D is still adding. | Override of `[A,B,D]` in flight | unfixed trunk | with this patch | |---|---|---| | `[A,B,D,E]` (RF 3→4, keeps D) | `adding=[E]`; completes as soon as E is in sync: `replicas=[A,B,D,E] isr=[A,B,E]`, nothing in flight, D never in ISR | `adding=[D,E] removing=[C]`; stays in flight until D is in sync | | `[A,B,D,E]` then `[A,B,D,E,F]` (chained, Cruise Control pattern) | `adding=[E]` then `adding=[F]`; completes as `replicas=[A,B,D,E,F] isr=[A,B,F]` | `adding=[D,E]` then `adding=[D,E,F]`; stays in flight | | `[A,D,E]` (RF-keeping, drops B) | `adding=[E] removing=[B,C]`; D silently promoted, only the `adding < removing` guard in `maybeCompleteReassignment` keeps it from finishing | `adding=[D,E] removing=[B,C]` | | `[B,C,D]` (removes the leader A) | `adding=[D] removing=[A]` (correct by accident: the new adding set is empty, so the builder leaves the old one in place) | `adding=[D] removing=[A]` | | `[A,B,D]` (same target re-submitted) | `adding=[D]` kept | `adding=[D]` kept | | `[A,B,E]` (drops D) | `adding=[E] removing=[C,D]`, completes as `[A,B,E]` | same | | `[A,B,D,E]` with `--disallow-replication-factor-change` | rejected (`replication factor is changed from 3 to 4`), in-flight reassignment untouched | same | | `--cancel` | reverts to `[A,B,C]` | same | | fresh `[A,B,C]` → `[D,E,F]` with `--throttle` / `--verify` | completes, `--verify` reports completed | same | After removing the throttle every partition on both builds converged to its target with a full ISR and no reassignment left in flight; the difference is only whether the controller ever let an out-of-sync replica become a plain replica. Controller leadership did not change during the runs. -- 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]
