You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by "Neha Narkhede (Created) (JIRA)" <ji...@apache.org> on 2012/03/16 02:14:37 UTC

[jira] [Created] (KAFKA-307) Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper

Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper
--------------------------------------------------------------------------------------

                 Key: KAFKA-307
                 URL: https://issues.apache.org/jira/browse/KAFKA-307
             Project: Kafka
          Issue Type: Bug
    Affects Versions: 0.7, 0.8
            Reporter: Neha Narkhede


Currently, LogManager wraps KafkaZooKeeper which is meant for all zookeeper interaction of a Kafka server. With replication, KafkaZookeeper will handle leader election, various state change listeners and then start replicas. Due to interdependency between LogManager and KafkaZookeeper, starting replicas is not possible until LogManager starts up completely. Due to this, we have to separate the broker startup procedures required for replication to get around this problem.

It will be good to refactor and clean up the server code, before diving deeper into replication.

--
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

        

[jira] [Updated] (KAFKA-307) Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper

Posted by "Neha Narkhede (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/KAFKA-307?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Neha Narkhede updated KAFKA-307:
--------------------------------

    Attachment:     (was: kafka-307-draft.patch)
    
> Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper
> --------------------------------------------------------------------------------------
>
>                 Key: KAFKA-307
>                 URL: https://issues.apache.org/jira/browse/KAFKA-307
>             Project: Kafka
>          Issue Type: Bug
>    Affects Versions: 0.7, 0.8
>            Reporter: Neha Narkhede
>         Attachments: kafka-307-draft.patch
>
>
> Currently, LogManager wraps KafkaZooKeeper which is meant for all zookeeper interaction of a Kafka server. With replication, KafkaZookeeper will handle leader election, various state change listeners and then start replicas. Due to interdependency between LogManager and KafkaZookeeper, starting replicas is not possible until LogManager starts up completely. Due to this, we have to separate the broker startup procedures required for replication to get around this problem.
> It will be good to refactor and clean up the server code, before diving deeper into replication.

--
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

        

[jira] [Commented] (KAFKA-307) Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper

Posted by "Jun Rao (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/KAFKA-307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13236175#comment-13236175 ] 

Jun Rao commented on KAFKA-307:
-------------------------------

Thanks for patch v2. Looks cleaner. A couple of other comments:

3. Is it better for KafkaZooKeeper to call ReplicaManager.addLocalReplica directly, instead of going through KafkaServer? For all replicas added in KafkaZookeeper, we know they are all local and should have a log. We could call LogManager.getOrCreateLog to get the log and pass it to ReplicaManager.

4. There are unused imports in KafkaServer.
                
> Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper
> --------------------------------------------------------------------------------------
>
>                 Key: KAFKA-307
>                 URL: https://issues.apache.org/jira/browse/KAFKA-307
>             Project: Kafka
>          Issue Type: Bug
>    Affects Versions: 0.7, 0.8
>            Reporter: Neha Narkhede
>         Attachments: kafka-307-draft.patch, kafka-307-v2.patch
>
>
> Currently, LogManager wraps KafkaZooKeeper which is meant for all zookeeper interaction of a Kafka server. With replication, KafkaZookeeper will handle leader election, various state change listeners and then start replicas. Due to interdependency between LogManager and KafkaZookeeper, starting replicas is not possible until LogManager starts up completely. Due to this, we have to separate the broker startup procedures required for replication to get around this problem.
> It will be good to refactor and clean up the server code, before diving deeper into replication.

--
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

        

[jira] [Updated] (KAFKA-307) Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper

Posted by "Neha Narkhede (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/KAFKA-307?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Neha Narkhede updated KAFKA-307:
--------------------------------

    Attachment: kafka-307-v3.patch

3. Fixed addReplica logic to use getOrCreateLog instead of getLog
4. Cleaned up the imports
                
> Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper
> --------------------------------------------------------------------------------------
>
>                 Key: KAFKA-307
>                 URL: https://issues.apache.org/jira/browse/KAFKA-307
>             Project: Kafka
>          Issue Type: Bug
>    Affects Versions: 0.7, 0.8
>            Reporter: Neha Narkhede
>         Attachments: kafka-307-draft.patch, kafka-307-v2.patch, kafka-307-v3.patch
>
>
> Currently, LogManager wraps KafkaZooKeeper which is meant for all zookeeper interaction of a Kafka server. With replication, KafkaZookeeper will handle leader election, various state change listeners and then start replicas. Due to interdependency between LogManager and KafkaZookeeper, starting replicas is not possible until LogManager starts up completely. Due to this, we have to separate the broker startup procedures required for replication to get around this problem.
> It will be good to refactor and clean up the server code, before diving deeper into replication.

--
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

        

[jira] [Commented] (KAFKA-307) Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper

Posted by "Jun Rao (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/KAFKA-307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13236734#comment-13236734 ] 

Jun Rao commented on KAFKA-307:
-------------------------------

+1 on patch v3.
                
> Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper
> --------------------------------------------------------------------------------------
>
>                 Key: KAFKA-307
>                 URL: https://issues.apache.org/jira/browse/KAFKA-307
>             Project: Kafka
>          Issue Type: Bug
>    Affects Versions: 0.7, 0.8
>            Reporter: Neha Narkhede
>         Attachments: kafka-307-draft.patch, kafka-307-v2.patch, kafka-307-v3.patch
>
>
> Currently, LogManager wraps KafkaZooKeeper which is meant for all zookeeper interaction of a Kafka server. With replication, KafkaZookeeper will handle leader election, various state change listeners and then start replicas. Due to interdependency between LogManager and KafkaZookeeper, starting replicas is not possible until LogManager starts up completely. Due to this, we have to separate the broker startup procedures required for replication to get around this problem.
> It will be good to refactor and clean up the server code, before diving deeper into replication.

--
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

        

[jira] [Resolved] (KAFKA-307) Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper

Posted by "Neha Narkhede (Resolved) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/KAFKA-307?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Neha Narkhede resolved KAFKA-307.
---------------------------------

    Resolution: Fixed
      Assignee: Neha Narkhede

Thanks for the review ! Committed this.
                
> Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper
> --------------------------------------------------------------------------------------
>
>                 Key: KAFKA-307
>                 URL: https://issues.apache.org/jira/browse/KAFKA-307
>             Project: Kafka
>          Issue Type: Bug
>    Affects Versions: 0.7, 0.8
>            Reporter: Neha Narkhede
>            Assignee: Neha Narkhede
>         Attachments: kafka-307-draft.patch, kafka-307-v2.patch, kafka-307-v3.patch
>
>
> Currently, LogManager wraps KafkaZooKeeper which is meant for all zookeeper interaction of a Kafka server. With replication, KafkaZookeeper will handle leader election, various state change listeners and then start replicas. Due to interdependency between LogManager and KafkaZookeeper, starting replicas is not possible until LogManager starts up completely. Due to this, we have to separate the broker startup procedures required for replication to get around this problem.
> It will be good to refactor and clean up the server code, before diving deeper into replication.

--
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

        

[jira] [Commented] (KAFKA-307) Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper

Posted by "Jun Rao (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/KAFKA-307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13235707#comment-13235707 ] 

Jun Rao commented on KAFKA-307:
-------------------------------

Overall looks good. Some suggestions:

1. KafkaServer:
Replicas are important data structures on each broker. Could we create a separate class Replicas that manage all replicas needed on a broker? Also, I see 2 different apis for adding a replica data structure. For replicas physically assigned to a broker, they always need a local Log. So, when adding those replicas, we need an api that creates a new log, if it's not there already (e.g. for newly created topics). The rest of replica data structures needed on a broker are not physically assigned to this broker and they are used to track the progress of the replicas in the followers. When adding those replicas, we need another api that doesn't force the creation of a local log.

2. log4j.properties: Do we really want to turn off logging for all kafka during unit tests? How about keeping it at ERROR level. We can probably turn off logging for ZK.
                
> Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper
> --------------------------------------------------------------------------------------
>
>                 Key: KAFKA-307
>                 URL: https://issues.apache.org/jira/browse/KAFKA-307
>             Project: Kafka
>          Issue Type: Bug
>    Affects Versions: 0.7, 0.8
>            Reporter: Neha Narkhede
>         Attachments: kafka-307-draft.patch
>
>
> Currently, LogManager wraps KafkaZooKeeper which is meant for all zookeeper interaction of a Kafka server. With replication, KafkaZookeeper will handle leader election, various state change listeners and then start replicas. Due to interdependency between LogManager and KafkaZookeeper, starting replicas is not possible until LogManager starts up completely. Due to this, we have to separate the broker startup procedures required for replication to get around this problem.
> It will be good to refactor and clean up the server code, before diving deeper into replication.

--
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

        

[jira] [Commented] (KAFKA-307) Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper

Posted by "Neha Narkhede (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/KAFKA-307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13236178#comment-13236178 ] 

Neha Narkhede commented on KAFKA-307:
-------------------------------------

3. Yeah, I thought about this, but then that requires LogManager to be referenced inside KafkaZooKeeper again. So, thats why it goes through KafkaServer. The point was to have only KafkaServer have access to LogManager, ReplicaManager and KafkaZookeeper, but not have those 3 components have interdependencies. What do you think ?

4. Will clean it up before committing the patch.
                
> Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper
> --------------------------------------------------------------------------------------
>
>                 Key: KAFKA-307
>                 URL: https://issues.apache.org/jira/browse/KAFKA-307
>             Project: Kafka
>          Issue Type: Bug
>    Affects Versions: 0.7, 0.8
>            Reporter: Neha Narkhede
>         Attachments: kafka-307-draft.patch, kafka-307-v2.patch
>
>
> Currently, LogManager wraps KafkaZooKeeper which is meant for all zookeeper interaction of a Kafka server. With replication, KafkaZookeeper will handle leader election, various state change listeners and then start replicas. Due to interdependency between LogManager and KafkaZookeeper, starting replicas is not possible until LogManager starts up completely. Due to this, we have to separate the broker startup procedures required for replication to get around this problem.
> It will be good to refactor and clean up the server code, before diving deeper into replication.

--
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

        

[jira] [Updated] (KAFKA-307) Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper

Posted by "Neha Narkhede (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/KAFKA-307?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Neha Narkhede updated KAFKA-307:
--------------------------------

    Attachment: kafka-307-v2.patch

Thanks for the review !

1. I like your suggestion. I wasn't quite satisfied with the way replicas were being managed. Incorporated the suggested changes. Looks much better now.

2. Didn't mean to check it in. But yes, I think zookeeper should be on ERROR and so should Kafka.
                
> Refactor server code to remove interdependencies between LogManager and KafkaZooKeeper
> --------------------------------------------------------------------------------------
>
>                 Key: KAFKA-307
>                 URL: https://issues.apache.org/jira/browse/KAFKA-307
>             Project: Kafka
>          Issue Type: Bug
>    Affects Versions: 0.7, 0.8
>            Reporter: Neha Narkhede
>         Attachments: kafka-307-draft.patch, kafka-307-v2.patch
>
>
> Currently, LogManager wraps KafkaZooKeeper which is meant for all zookeeper interaction of a Kafka server. With replication, KafkaZookeeper will handle leader election, various state change listeners and then start replicas. Due to interdependency between LogManager and KafkaZookeeper, starting replicas is not possible until LogManager starts up completely. Due to this, we have to separate the broker startup procedures required for replication to get around this problem.
> It will be good to refactor and clean up the server code, before diving deeper into replication.

--
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