1.1 Typo satup
1.2 Sometimes, multiple new lines are added between methods. Can we make this consistent?
1.3 It's probably better to create a separate class ControllerChannelManager that manages all queues and RequestSendThreads. ControllerChannelManager will support the following API:
def sendRequest(brokerId : Int, request : RequestOrResponse, callback: (RequestOrResponse) => Unit = null)
def addBroker(brokerId: Int, broker: Broker)
def removeBroker(brokerId: Int)
1.4 BrokerChangeListener.handleChildChange(): can we rename curChilds to javaCurChildren and scalaCurChildren to curChildren since the code is written in scala?
1.5 We don't need to fill in the content of the listener in this jira. Just add a TODO comment.
-----> the brokerListener is needed
2.1 run(): We can't shutdown this thread if no request is added to the queue since queue.take() is blocking. We need to do something like ReplicaFetcherThread by interrupting the thread to make the thread shutdownable.
3. KafkaZookeeper: It seems to me that it's better to move the controller election logic to KafkaController. On startup, KafkaController checks if it can become the controller. If yes, it registers the listeners on brokers and topics. Otherwise, it registers a listener on the controller path for controller failover. This way, all logic related to controller is self contained in KafkaController.
Seems that's not applicable, talk in person
4.1 useDefaultBufferSize => UseDefaultBufferSize
4.2 remove new lines added after the constructor
4.3 Today, we have logic in SyncProducer and SimpleConsumer to reconnect if there is any socket IO exception while sending/receiving data. We probably need something like that in RequestSendThread too. Instead of duplicating this logic. It's probably better to move this logic to BlockingChannel. I was thinking that instead of have a send and a receive API, BlockingChannel can just support 1 sendAndReceive api and implements the reconnect logic for socket exception.
-----> Let me do it separately, is it ok?
5. ZkUtils: getController and getTopicPartitionLeaderAndISR are not used anywhere.
-----> removed in the patch