[
https://issues.apache.org/jira/browse/KAFKA-813?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13606649#comment-13606649
]
Neha Narkhede commented on KAFKA-813:
-------------------------------------
Thanks for the cleanup patch, Swapnil. Here are some minor suggestions after
which we can check it in -
1. ControllerBrokerRequestBatch
1.1 Can we explicitly pass in controller id and controller client id ? It is
better to avoid object passing if we can.
1.2 How about clientId() instead of toString(). This is because it makes it
very explicit the purpose of the usage of that API. It is unlikely anything
else would want to call toString() on KafkaController
2. PartitionStateMachine
Let's not set the leaderSelector to null as default. Each invocation of
handleStateChanges should pass in the correct leader selector for better code
readability.
> Minor cleanup in Controller
> ---------------------------
>
> Key: KAFKA-813
> URL: https://issues.apache.org/jira/browse/KAFKA-813
> Project: Kafka
> Issue Type: Bug
> Affects Versions: 0.8
> Reporter: Swapnil Ghike
> Assignee: Swapnil Ghike
> Fix For: 0.8
>
> Attachments: kafka-813-v1.patch
>
>
> Before starting work on delete topic support, uploading a patch first to
> address some minor hiccups that touch a bunch of files:
> 1. Change PartitionOfflineException to PartitionUnavailableException because
> in the partition state machine we mark a partition offline when its leader is
> down, whereas the PartitionOfflineException is thrown when all the assigned
> replicas of the partition are down.
> 2. Change PartitionOfflineRate to UnavailablePartitionRate
> 3. Remove default leader selector from partition state machine's
> handleStateChange. We can specify null as default when we don't need to use a
> leader selector.
> 4. Include controller info in the client id of LeaderAndIsrRequest.
> 5. Rename controllerContext.allleaders to something more meaningful -
> partitionLeadershipInfo.
> 6. We don't need to put partition in OnlinePartition state in partition state
> machine initializeLeaderAndIsrForPartition, the state change occurs in
> handleStateChange.
> 7. Add todo in handleStateChanges
> 8. Left a comment above ReassignedPartitionLeaderSelector that reassigned
> replicas are already in the ISR (this is not true for other leader
> selectors), renamed the vals in the selector.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
