Skip to content

Conversation

Yunyung
Copy link
Collaborator

@Yunyung Yunyung commented Aug 18, 2025

  • Move the RaftManager interface to raft module, and remove the
    register and leaderAndEpoch methods since they are already part of
    the RaftClient APIs.
  • Rename RaftManager.scala to KafkaRaftManager.scala.

@github-actions github-actions bot added triage PRs from the community core Kafka Broker kraft small Small PRs labels Aug 18, 2025
Copy link
Contributor

@frankvicky frankvicky left a comment

Choose a reason for hiding this comment

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

Please fix the build error

@chia7712
Copy link
Member

> Task :raft:checkstyleMain
[ant:checkstyle] [ERROR] /home/chia7712/project/kafka/raft/src/main/java/org/apache/kafka/raft/RaftManager.java:1: Line does not match expected header line of '/*'. [Header]

@github-actions github-actions bot removed the small Small PRs label Aug 18, 2025
@Yunyung
Copy link
Collaborator Author

Yunyung commented Aug 18, 2025

Updated. Add license header to new file to fix build error. Thanks.

Copy link
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@Yunyung thanks for this patch

long createdTimeMs
);

void register(RaftClient.Listener<T> listener);
Copy link
Member

Choose a reason for hiding this comment

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

register and leaderAndEpoch are also a part of RaftClient APIs. We could remove them from RaftManager, since callers can use client instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great. Done.

@github-actions github-actions bot removed the triage PRs from the community label Aug 19, 2025
Copy link
Collaborator

@m1a2st m1a2st left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

Copy link
Contributor

@frankvicky frankvicky left a comment

Choose a reason for hiding this comment

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

@Yunyung: LGTM

override def voterNode(id: Int, listener: ListenerName): Option[Node] = {
client.voterNode(id, listener).toScala
override def voterNode(id: Int, listener: ListenerName): Optional[Node] = {
client.voterNode(id, listener)
Copy link
Member

Choose a reason for hiding this comment

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

If the delegation is only a single line and the client is an interface, perhaps we don't need to wrap it with RaftManager

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Kafka Broker kraft
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants