Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

autothrottle Kafka native API support #399

Merged
merged 35 commits into from
May 3, 2022

Conversation

jamiealquiza
Copy link
Collaborator

@jamiealquiza jamiealquiza commented Apr 21, 2022

Changes

  • Updates autothrottle to optionally perform all operations that currently read from ZooKeeper to use the Kafka Admin API (via the kafka-kit/kafkaadmin library):
    • KIP-455 compatible reassignment lookups
    • Topic states
    • Broker, topic throttle application
    • Broker, topic throttle clearing
  • The feature is enabled via the --kafka-native-mode flag (defaults to false for now, which means autothrottle will function the same as it does before this PR)
  • Deletes deprecated autothrottle API routes

Functional notes

  • Internally (@ Datadog) we currently set static, broker level overrides on possibly not-yet-existing brokers. The Kafka APIs have seemingly unbounded waits when setting dynamic configs on non-existent resources. I need to validate whether we're exposed to this in these changes.
  • The Kafka API is generally a bit more flakey to deal with than reading/writing znodes, which tends to succeed regardless of the Kafka cluster state. It's almost certain that we're going to see weird client behavior that will need more elaborate conditional behavior and timeout tuning.
  • This entire change is a functional no-op unless you flag on --kafka-native-mode.

PR notes

  • This is easiest to read by stepping through commits sequentially
  • Much of the ZK based code has been organized and branded as "legacy" for eventual removal
  • Autothrottle needs significant refactoring, but this PR introduces a considerable number of complicated changes. Accordingly, a minimal amount of restructuring was done in order to satisfy the introduction of this new functionality. A dedicated refactor will be a follow up task:
    • Added the autothrottle/internal/api and autothrottle/internal/throttlestore
    • Further refactoring will move a significant amount of the currently flat autothrottle codebase to better structured packages

TODOs

  • Finish test coverage for newly introduced code
  • Complete system testing

@jamiealquiza jamiealquiza marked this pull request as ready for review April 22, 2022 14:28
@jamiealquiza jamiealquiza changed the title [wip/testing] autothrottle Kafka native API support [testing] autothrottle Kafka native API support Apr 22, 2022
@jamiealquiza jamiealquiza force-pushed the jamie/autothrottle-admin-rpc branch from 3c29a52 to 0105db0 Compare April 25, 2022 17:26
@jamiealquiza jamiealquiza changed the title [testing] autothrottle Kafka native API support autothrottle Kafka native API support Apr 26, 2022
cmd/autothrottle/main.go Outdated Show resolved Hide resolved
@jamiealquiza jamiealquiza requested a review from scanterog May 3, 2022 14:51
@jamiealquiza jamiealquiza merged commit 0052781 into master May 3, 2022
@jamiealquiza jamiealquiza deleted the jamie/autothrottle-admin-rpc branch May 3, 2022 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants