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.

Sign up for GitHub

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

Jump to bottom

KAFKA-17951: Share parition rotate strategy #18651

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

KAFKA-17951: Share parition rotate strategy #18651

wants to merge 1 commit into from
+448 −164

Conversation

apoorvmittal10
Copy link
Collaborator

Defined PartitionRotateStrategy class and round robin implementation for partitions. This is required to let not starve any single partition during share-fetch.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@github-actions github-actions bot added triage PRs from the community core Kafka Broker KIP-932 Queues for Kafka labels Jan 20, 2025
@AndrewJSchofield AndrewJSchofield removed the triage PRs from the community label Jan 21, 2025
adixitconfluent
adixitconfluent reviewed Jan 24, 2025
View reviewed changes
Copy link
Contributor

@adixitconfluent adixitconfluent left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Overall LGTM.

adixitconfluent
adixitconfluent reviewed Jan 24, 2025
View reviewed changes
Copy link
Contributor

@adixitconfluent adixitconfluent left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just 1 minor comment.

server/src/main/java/org/apache/kafka/server/share/fetch/PartitionRotateStrategy.java
return topicIdPartitions;
}

// Once the partition max bytes is removed then the partition will be a linked list and rotation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a TODO here before this comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adixitconfluent adixitconfluent adixitconfluent left review comments

@AndrewJSchofield AndrewJSchofield Awaiting requested review from AndrewJSchofield

Assignees
No one assigned
Labels
core Kafka Broker KIP-932 Queues for Kafka
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@apoorvmittal10 @adixitconfluent @AndrewJSchofield