[jira] [Comment Edited] (KAFKA-46) Commit thread, ReplicaFetcherThread for intra-cluster replication

["Neha Narkhede (JIRA)" <ji...@apache.org>]

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

Neha Narkhede edited comment on KAFKA-46 at 5/26/12 2:32 AM:
-------------------------------------------------------------

Jay,

Thanks for thinking through the code structure, I've included more refactoring changes in this patch. Some of the suggestions are orthogonal to this patch and I'd prefer to fix it in another JIRA, given the complexity of this patch. Maybe I can create a 'refactoring' JIRA after this one to cover some of these -

2. Makes sense. I guess that was an over optimization.
3. This is a good suggestion, al though would prefer keeping it to refactoring JIRA
4. Picked descriptive names
5. Somehow I like the idea of wrapping up enough logic inside Replica to figure out if it is a follower or leader. ReplicaFetcherThread inside Replica allows that. Al though, I'm not sure that is the best way to achieve it.
6. Yeah, probably something to think about. Will move it to the refactoring JIRA
7. I like Option 4 there, hoping that can be fixed in a separate JIRA
8. Yeah, I moved some zookeeper client access to ReplicaManager so that all replication specific logic can be moved there.
9. Changed configs to replication.*
11. Simplified the ISR expiration. Looks better now.
12. Hmm, Utils.newThread returns Thread, but I think it is useful to use some APIs specific to ReplicaFetcherThread like getIfFollowerAndLeader(). But I see your point here. Given a choice, it is always better to use a helper method. I set the daemon property and the thread handles all Throwables. 
13. Yeah, this is a good suggestion. This also fits in generic refactoring category that can be fixed separately.
14. This is another great suggestion. Please see the included patch if you like it. 
15. Fixed it
16. Fixed it
17. Yeah, this will keep changing with the v3 code. Will be good to keep this in mind though.

Overall, I liked your refactoring suggestions, and I might have been lazy to describe all of the changes I made here. Will really appreciate it if you can read through the new patch and suggest improvements. I'm fine with working through more in this patch itself, if you feel that works better. 
                
      was (Author: nehanarkhede):
    Jay,

Thanks for thinking through the code structure, I've included more refactoring changes in this patch. Some of the suggestions are orthogonal to this patch and I'd prefer to fix it in another JIRA, given the complexity of this patch. Maybe I can create a 'refactoring' JIRA after this one to cover some of these -

2. Makes sense. I guess that was an over optimization.
3. This is a good suggestion, al though would prefer keeping it to refactoring JIRA
4. Picked descriptive names
5. Somehow I like the idea of wrapping up enough logic inside Replica to figure out if it is a follower or leader. ReplicaFetcherThread inside Replica allows that. Al though, I'm not sure that is the best way to achieve it.
6. Yeah, probably something to think about. Will move it to the refactoring JIRA
7. I like Option 4 there, hoping that can be fixed in a separate JIRA
8. Yeah, I moved some zookeeper client access to ReplicaManager so that all replication specific logic can be moved there.
9. Changed configs to replication.*
11. Simplified the ISR expiration. Looks better now.
12. Hmm, Utils.newThread returns Thread, but I think it is useful to use some APIs specific to ReplicaFetcherThread like getIfFollowerAndLeader(). But I see your point here. Given a choice, it is always better to use a helper method. I set the daemon property and the thread handles all Throwables. 
13. Yeah, this is a good suggestion. This also fits in generic refactoring category that can be fixed separately.
14. This is another great suggestion. Please see the included patch if you like it. 
15. Fixed it
16. Fixed it
17. Yeah, this will keep changing with the v3 code. Will be good to keep this in mind though.
                  
> Commit thread, ReplicaFetcherThread for intra-cluster replication
> -----------------------------------------------------------------
>
>                 Key: KAFKA-46
>                 URL: https://issues.apache.org/jira/browse/KAFKA-46
>             Project: Kafka
>          Issue Type: Bug
>            Reporter: Jun Rao
>            Assignee: Neha Narkhede
>         Attachments: kafka-46-draft.patch, kafka-46-v1.patch, kafka-46-v2.patch
>
>
> We need to implement the commit thread at the leader and the fetcher thread at the follower for replication the data from the leader.

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