Mailing-List: contact kafka-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: kafka-dev@incubator.apache.org
Date: Fri, 8 Jun 2012 17:36:23 +0000 (UTC)
From: "Jun Rao (JIRA)" <jira@apache.org>
To: kafka-dev@incubator.apache.org
Message-ID: <699084157.53537.1339176983358.JavaMail.jiratomcat@issues-vm>
In-Reply-To: 
 <1069004142.35227.1336410649162.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] [Commented] (KAFKA-335) Implement an embedded controller
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/KAFKA-335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13291872#comment-13291872 ] 

Jun Rao commented on KAFKA-335:
-------------------------------

Thanks for the patch. Some comments:

1. KafkaController:
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.

2. RequestSendThread:
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.

4. BlockingChannel:
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.

5. ZkUtils: getController and getTopicPartitionLeaderAndISR are not used anywhere.

                
> Implement an embedded controller
> --------------------------------
>
>                 Key: KAFKA-335
>                 URL: https://issues.apache.org/jira/browse/KAFKA-335
>             Project: Kafka
>          Issue Type: Sub-task
>          Components: core
>    Affects Versions: 0.8
>            Reporter: Jun Rao
>            Assignee: Yang Ye
>             Fix For: 0.8
>
>         Attachments: kafka_controller_v1.patch
>
>   Original Estimate: 168h
>  Remaining Estimate: 168h
>
> This ticket will implement a controller as described in https://cwiki.apache.org/confluence/display/KAFKA/kafka+Detailed+Replication+Design+V3
> This includes creating the controller path, setting up necessary watchers (e.g, Broker path and TopicPath), and failover the controller.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira