C0urante opened a new pull request, #12019: URL: https://github.com/apache/kafka/pull/12019
[Jira](https://issues.apache.org/jira/browse/KAFKA-13764) Depends on https://github.com/apache/kafka/pull/11983 The primary goal of this PR is to address several outstanding issues with incremental rebalancing that lead to stable-but-unbalanced clusters. However, other small bug fixes are also applied, and some liberty is taken with refactoring to improve readability and flexibility in the code base. This should also address [KAFKA-12495](https://issues.apache.org/jira/browse/KAFKA-12495), and includes an adapted [test case](https://github.com/apache/kafka/pull/10367/files#diff-244a837479b827768c3daaabee8c7ad2064731377f1ad26a11bd890214252b7fR211-R358) from https://github.com/apache/kafka/pull/10367, which addresses that issue but with a different approach. High-level changes: - Refine the logic for load-balancing revocations: - - Perform load-balancing revocations any time the cluster appears imbalanced and there are still connectors and tasks that can be revoked from workers, instead of only when the number of workers in the cluster changes - - Remove the "rough estimation" logic and replace it with a precise calculation of exactly how to allocate all currently-configured connectors and tasks as evenly as possible across a cluster - - Account for load-balancing revocations when assigning new and lost connectors and tasks across the cluster - Improve code quality: - - Extract the `ConnectorsAndTasks` class into its own file, enrich it and its builder class with developer-friendly methods, make its contents completely immutable, and use `Set` instead of generic `Collection` instances to store connectors and tasks - - Where possible, identify logic that is shared for connectors and tasks (`IncrementalCooperativeAssignor::assignConnectors` and `IncrementalCooperativeAssignor::assignTasks`, for example) and abstract it into a single reusable method - - Use the `final` keyword for base and derived sets in `IncrementalCooperativeAssignor::performTaskAssignment` (tracking mutations across a 100+ line method is difficult) - - Reword unnecessary and confusing comments ("... is a derived set from the set difference of ..." is not very informative) - - Reorganize the grouping of methods in `IncrementalCooperativeAssignor` to place static utility methods together at the bottom of the class - - Demote visibility of testing-only methods and fields from `protected` to package-private (`protected` implies that the field/method is intended for use by subclasses, which is not the case for any of these) - Testing: - - Add several new tests to cover a variety of new cases, many of which result in imbalanced allocation with the current rebalancing logic, but which are all correctly handled with the improvements in this PR - - Add a few testing utility methods to help "hand wave" test cases without having to specify fine-grained expectations like how many rounds of rebalance are required to reach stability after some changes have been applied to the cluster - - Add coverage to all tests that ensures that no connectors or tasks are both revoked and assigned from the sam worker, and that the leader's view of the complete assignment of connectors and tasks across the cluster appears to be correct after each rebalance - Miscellaneous: - - Demote a ton of noisy `DEBUG`-level log messages to `TRACE` ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade notes) -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
