-
Notifications
You must be signed in to change notification settings - Fork 14.5k
KAFKA-18486: Remove ReplicaManager#becomeLeaderOrFollower #20037
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In ReplicaManager, there is one unused variable
@volatile private[server] var controllerEpoch: Int = 0
And there are plenty of code in LeaderAndIsrRequest.java could be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TaiJuWu thanks for you patch. alterPartitionManager
is not used, so please remove it also.
byte leaderRecoveryState; | ||
|
||
public PartitionState() { | ||
this.topicName = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please remove the default constructor by initializing these variables directly?
public class PartitionState {
String topicName = "";
int partitionIndex = 0;
int leader = 0;
completeDelayedOperationsWhenNotPartitionLeader(partition.topicPartition, partition.topicId) | ||
} | ||
|
||
if (isShuttingDown.get()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isShuttingDown
is useless now, so could you please remove it
Thanks for review, we can't remove
|
@@ -354,7 +354,6 @@ class BrokerServer( | |||
logDirFailureChannel = logDirFailureChannel, | |||
alterPartitionManager = alterPartitionManager, | |||
brokerTopicStats = brokerTopicStats, | |||
isShuttingDown = isShuttingDown, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could remove isShuttingDown
from BrokerServer
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. we could have follow-ups for advanced cleanup in order to avoid pouring changes to this PR.
The follow-ups are listed below.
|
Remove FlattenedIterator. It’s no longer used anywhere after #20037. Reviewers: TengYao Chi <kitingiao@gmail.com>, Lan Ding <isDing_L@163.com>, Chia-Ping Tsai <chia7712@gmail.com>
The PR do following: 1. Remove ReplicaManager#becomeLeaderOrFollower. 2. Remove `LeaderAndIsrRequest` and `LeaderAndIsrResponse` 3. Migrate `LeaderAndIsrRequest.PartitionState` to server-common module and change to `PartitionState` 4. Remove `ControllerEpoch` from PartitionState 5. Remove `isShuttingDown` from BrokerServer and ReplicaManager Reviewers: Kuan-Po Tseng <brandboat@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>
Remove FlattenedIterator. It’s no longer used anywhere after apache#20037. Reviewers: TengYao Chi <kitingiao@gmail.com>, Lan Ding <isDing_L@163.com>, Chia-Ping Tsai <chia7712@gmail.com>
The PR do following: 1. Remove ReplicaManager#becomeLeaderOrFollower. 2. Remove `LeaderAndIsrRequest` and `LeaderAndIsrResponse` 3. Migrate `LeaderAndIsrRequest.PartitionState` to server-common module and change to `PartitionState` 4. Remove `ControllerEpoch` from PartitionState 5. Remove `isShuttingDown` from BrokerServer and ReplicaManager Reviewers: Kuan-Po Tseng <brandboat@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>
Remove FlattenedIterator. It’s no longer used anywhere after apache#20037. Reviewers: TengYao Chi <kitingiao@gmail.com>, Lan Ding <isDing_L@163.com>, Chia-Ping Tsai <chia7712@gmail.com>
The PR do following:
LeaderAndIsrRequest
andLeaderAndIsrResponse
LeaderAndIsrRequest.PartitionState
to server-common moduleand change to
PartitionState
ControllerEpoch
from PartitionStateisShuttingDown
from BrokerServer and ReplicaManagerReviewers: Kuan-Po Tseng brandboat@gmail.com, Chia-Ping Tsai
chia7712@gmail.com