You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Guozhang Wang <gu...@linkedin.com> on 2013/12/19 04:01:10 UTC

Review Request 16360: Patch for KAFKA-1188

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/
-----------------------------------------------------------

Review request for kafka.


Bugs: KAFKA-1188
    https://issues.apache.org/jira/browse/KAFKA-1188


Repository: kafka


Description
-------

KAFKA-1188.v1


Diffs
-----

  core/src/main/scala/kafka/cluster/Partition.scala 5c9307d71641ccc6c09a54b69d5aa2b4bc2a4cde 
  core/src/main/scala/kafka/server/ReplicaManager.scala 242c18d47828b7c5e6b1fc219a0f1199fb1f9512 

Diff: https://reviews.apache.org/r/16360/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 16360: Patch for KAFKA-1188

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/#review30768
-----------------------------------------------------------



core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/16360/#comment58913>

    Originally the leader broker needs to be searched twice, once in ReplicaManager and once in Partition for checking if the leader is available, and the bug comes from the check in Partition does not stop the transition. So here I wanted to merge them into one check in Partition and also save the searching in ReplicaManager. I agree that the return type is not very neat here. Will change to having the check on ReplicaManager.



core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/16360/#comment58914>

    Reason same as above. Will fix accordingly



core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/16360/#comment58915>

    Done.



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment58916>

    Thanks. Done.



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment58917>

    This logging for all the partitions.


- Guozhang Wang


On Dec. 19, 2013, 3:01 a.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16360/
> -----------------------------------------------------------
> 
> (Updated Dec. 19, 2013, 3:01 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1188
>     https://issues.apache.org/jira/browse/KAFKA-1188
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1188.v1
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 5c9307d71641ccc6c09a54b69d5aa2b4bc2a4cde 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 242c18d47828b7c5e6b1fc219a0f1199fb1f9512 
> 
> Diff: https://reviews.apache.org/r/16360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 16360: Patch for KAFKA-1188

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/#review30767
-----------------------------------------------------------


Thanks for the patch. Some comments.


core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/16360/#comment58909>

    It's kind of weird that this method returns the leader broker. Returning a boolean seems more natural. We can find out the leader broker in the caller.



core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/16360/#comment58908>

    Wouldn't using case match be more natural than dealing with null?



core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/16360/#comment58907>

    Could we make the comment clearer? Basically, during controller failover, the controller always first sends the current partition state info before doing the leader election. So, it could happen that the leader broker is not present in the leaderAndIsrRequest.



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment58910>

    It seems that if we are shutting down this broker, we still want to remove existing fetchers. We just don't want to add new fetchers.



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment58911>

    Could we name this to sth like successfulPartitionAndOffsets?



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment58912>

    Is it better to just log the failed partition in partition.makeFollower(), instead of here? This way, we probably log less.


- Jun Rao


On Dec. 19, 2013, 3:01 a.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16360/
> -----------------------------------------------------------
> 
> (Updated Dec. 19, 2013, 3:01 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1188
>     https://issues.apache.org/jira/browse/KAFKA-1188
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1188.v1
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 5c9307d71641ccc6c09a54b69d5aa2b4bc2a4cde 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 242c18d47828b7c5e6b1fc219a0f1199fb1f9512 
> 
> Diff: https://reviews.apache.org/r/16360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 16360: Address Jun's comments

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/#review31041
-----------------------------------------------------------



core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/16360/#comment59388>

    This comment is no longer valid.



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment59387>

    The ordering of correlationId and controllerId is reversed.



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment59386>

    fetchers -> fetcher


- Jun Rao


On Dec. 24, 2013, 7:07 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16360/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2013, 7:07 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1188
>     https://issues.apache.org/jira/browse/KAFKA-1188
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1118.v2
> 
> 
> KAFKA-1188.v1
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 5c9307d71641ccc6c09a54b69d5aa2b4bc2a4cde 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 242c18d47828b7c5e6b1fc219a0f1199fb1f9512 
> 
> Diff: https://reviews.apache.org/r/16360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 16360: Fix the offset bug while adding fetchers

Posted by Guozhang Wang <gu...@linkedin.com>.

> On Jan. 7, 2014, 5:12 p.m., Neha Narkhede wrote:
> > In general, does the alternate approach of sending all commands to every broker and let the broker ignore a become follower if it is for the same leader as the current one? The reason why this would work is because all controller-broker communication is synchronous, so even if the same broker became leader in quick succession (1->2->1), the follower would see the 1->2 transition before seeing the 2->1, so it wouldn't incorrectly ignore a become follower request from the same leader.

Sending LeaderAndIsr request to followers on non-leader change has the following risk: since the follower will only increase its HW on the next fetch request after the leader has increased its HW, during this time if a LeaderAndIsr request reached the follower it may truncate the last fetched data since they are not within HW yet. Then if there is a leader change immediately the data will be lost without even notified by the controller.


- Guozhang


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/#review31299
-----------------------------------------------------------


On Jan. 16, 2014, 12:43 a.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16360/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2014, 12:43 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1188
>     https://issues.apache.org/jira/browse/KAFKA-1188
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1188.v5
> 
> 
> Dummy
> 
> 
> KAFKA-1188.v4
> 
> 
> KAFKA-1188.v3
> 
> 
> KAFKA-1118.v2
> 
> 
> KAFKA-1188.v1
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f9d10d385cee49a1e3be8c82e3ffa22ef87a8fd6 
> 
> Diff: https://reviews.apache.org/r/16360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 16360: Address Jun's comments v3

Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/#review31299
-----------------------------------------------------------


In general, does the alternate approach of sending all commands to every broker and let the broker ignore a become follower if it is for the same leader as the current one? The reason why this would work is because all controller-broker communication is synchronous, so even if the same broker became leader in quick succession (1->2->1), the follower would see the 1->2 transition before seeing the 2->1, so it wouldn't incorrectly ignore a become follower request from the same leader.

- Neha Narkhede


On Jan. 2, 2014, 6:44 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16360/
> -----------------------------------------------------------
> 
> (Updated Jan. 2, 2014, 6:44 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1188
>     https://issues.apache.org/jira/browse/KAFKA-1188
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1188.v3
> 
> 
> KAFKA-1118.v2
> 
> 
> KAFKA-1188.v1
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 5c9307d71641ccc6c09a54b69d5aa2b4bc2a4cde 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 242c18d47828b7c5e6b1fc219a0f1199fb1f9512 
> 
> Diff: https://reviews.apache.org/r/16360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 16360: Fix the offset bug while adding fetchers

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/#review32002
-----------------------------------------------------------


Looks good. Just a couple of minor comments.


core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment60751>

    Should controllerId be logged using %d?



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment60752>

    Since this foreach is not a liner, it's probably better to use {, instead of (?


- Jun Rao


On Jan. 16, 2014, 12:43 a.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16360/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2014, 12:43 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1188
>     https://issues.apache.org/jira/browse/KAFKA-1188
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1188.v5
> 
> 
> Dummy
> 
> 
> KAFKA-1188.v4
> 
> 
> KAFKA-1188.v3
> 
> 
> KAFKA-1118.v2
> 
> 
> KAFKA-1188.v1
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f9d10d385cee49a1e3be8c82e3ffa22ef87a8fd6 
> 
> Diff: https://reviews.apache.org/r/16360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 16360: Address comments from Neha, and record error messages if leader brokers are not available since this should not happen then

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/#review32147
-----------------------------------------------------------



core/src/main/scala/kafka/controller/ReplicaStateMachine.scala
<https://reviews.apache.org/r/16360/#comment60853>

    Need to change this comment accordingly.



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment60852>

    present => be present
    now => not


- Jun Rao


On Jan. 16, 2014, 11:02 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16360/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2014, 11:02 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1188
>     https://issues.apache.org/jira/browse/KAFKA-1188
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1188.v6
> 
> 
> KAFKA-1188.v5.1
> 
> 
> KAFKA-1188.v5
> 
> 
> Dummy
> 
> 
> KAFKA-1188.v4
> 
> 
> KAFKA-1188.v3
> 
> 
> KAFKA-1118.v2
> 
> 
> KAFKA-1188.v1
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 483559aa64726c51320d18b64a1b48f8fb2905a0 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f9d10d385cee49a1e3be8c82e3ffa22ef87a8fd6 
> 
> Diff: https://reviews.apache.org/r/16360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 16360: Address comments from Neha, and record error messages if leader brokers are not available since this should not happen then

Posted by Guozhang Wang <gu...@linkedin.com>.

> On Jan. 18, 2014, 2:17 a.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/ReplicaManager.scala, line 425
> > <https://reviews.apache.org/r/16360/diff/6/?file=425126#file425126line425>
> >
> >     "aborted the become-follower state change"...
> >     
> >     Also, I'm not sure this shutting down check is doing what we really want.
> >     i.e., the replica fetcher manager may be shutdown right after this, and we
> >     could still add fetchers. But that's a separate issue, and I may have
> >     misread the code.
> >     
> >

I agree with you, this check does not actually avoid adding fetchers while at the same time shutting down fetcher manager. Will file another jira for this.


> On Jan. 18, 2014, 2:17 a.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/server/ReplicaManager.scala, line 376
> > <https://reviews.apache.org/r/16360/diff/6/?file=425126#file425126line376>
> >
> >     I'm wondering if it would make the code cleaner/clearer if you move all the
> >     code below that deals with partitionsToMakeFollower to this block of the if
> >     statement.
> >

We use the current pattern because we want to do adding-fetcher and truncating log in batch, moving them into the if statement will revert to the one-by-one pattern.


- Guozhang


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/#review32242
-----------------------------------------------------------


On Jan. 16, 2014, 11:02 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16360/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2014, 11:02 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1188
>     https://issues.apache.org/jira/browse/KAFKA-1188
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1188.v6
> 
> 
> KAFKA-1188.v5.1
> 
> 
> KAFKA-1188.v5
> 
> 
> Dummy
> 
> 
> KAFKA-1188.v4
> 
> 
> KAFKA-1188.v3
> 
> 
> KAFKA-1118.v2
> 
> 
> KAFKA-1188.v1
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 483559aa64726c51320d18b64a1b48f8fb2905a0 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f9d10d385cee49a1e3be8c82e3ffa22ef87a8fd6 
> 
> Diff: https://reviews.apache.org/r/16360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 16360: Address comments from Neha, and record error messages if leader brokers are not available since this should not happen then

Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/#review32242
-----------------------------------------------------------



core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/16360/#comment60970>

    Would be better to move this comment to above the method and say what it
    returns and what it means there.
    



core/src/main/scala/kafka/controller/ReplicaStateMachine.scala
<https://reviews.apache.org/r/16360/#comment60972>

    Clearer to say: "to all the remaining (alive) replicas" of the partition.



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment60981>

    I'm wondering if it would make the code cleaner/clearer if you move all the
    code below that deals with partitionsToMakeFollower to this block of the if
    statement.
    



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment60982>

    "aborted the become-follower state change"...
    
    Also, I'm not sure this shutting down check is doing what we really want.
    i.e., the replica fetcher manager may be shutdown right after this, and we
    could still add fetchers. But that's a separate issue, and I may have
    misread the code.
    
    


- Joel Koshy


On Jan. 16, 2014, 11:02 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16360/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2014, 11:02 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1188
>     https://issues.apache.org/jira/browse/KAFKA-1188
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1188.v6
> 
> 
> KAFKA-1188.v5.1
> 
> 
> KAFKA-1188.v5
> 
> 
> Dummy
> 
> 
> KAFKA-1188.v4
> 
> 
> KAFKA-1188.v3
> 
> 
> KAFKA-1118.v2
> 
> 
> KAFKA-1188.v1
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 483559aa64726c51320d18b64a1b48f8fb2905a0 
>   core/src/main/scala/kafka/server/ReplicaManager.scala f9d10d385cee49a1e3be8c82e3ffa22ef87a8fd6 
> 
> Diff: https://reviews.apache.org/r/16360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 16360: Patch for KAFKA-1188

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/
-----------------------------------------------------------

(Updated Feb. 12, 2014, 1:48 a.m.)


Review request for kafka.


Bugs: KAFKA-1188
    https://issues.apache.org/jira/browse/KAFKA-1188


Repository: kafka


Description (updated)
-------

Rebased on delete-topics


Diffs (updated)
-----

  core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 5e016d5d2bbef7fc33fb80c9a4a90ec5d39c16d0 
  core/src/main/scala/kafka/server/ReplicaManager.scala 21bba48affb38ff6504e02535d3cc56258f45b60 

Diff: https://reviews.apache.org/r/16360/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 16360: Patch for KAFKA-1188

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/
-----------------------------------------------------------

(Updated Feb. 12, 2014, 1:58 a.m.)


Review request for kafka.


Bugs: KAFKA-1188
    https://issues.apache.org/jira/browse/KAFKA-1188


Repository: kafka


Description
-------

Rebased on delete-topics


Diffs
-----

  core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 5e016d5d2bbef7fc33fb80c9a4a90ec5d39c16d0 
  core/src/main/scala/kafka/server/ReplicaManager.scala 21bba48affb38ff6504e02535d3cc56258f45b60 

Diff: https://reviews.apache.org/r/16360/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 16360: Patch for KAFKA-1188

Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/#review34406
-----------------------------------------------------------

Ship it!


Only minor comments 


core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/16360/#comment64521>

    remove extra TopicAndPartition import



core/src/main/scala/kafka/server/ReplicaManager.scala
<https://reviews.apache.org/r/16360/#comment64495>

    could you please wrap the long line here?


- Neha Narkhede


On Feb. 12, 2014, 1:58 a.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16360/
> -----------------------------------------------------------
> 
> (Updated Feb. 12, 2014, 1:58 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1188
>     https://issues.apache.org/jira/browse/KAFKA-1188
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Rebased on delete-topics
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 5e016d5d2bbef7fc33fb80c9a4a90ec5d39c16d0 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 21bba48affb38ff6504e02535d3cc56258f45b60 
> 
> Diff: https://reviews.apache.org/r/16360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


Re: Review Request 16360: Patch for KAFKA-1188

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/
-----------------------------------------------------------

(Updated Feb. 12, 2014, 1:58 a.m.)


Review request for kafka.


Bugs: KAFKA-1188
    https://issues.apache.org/jira/browse/KAFKA-1188


Repository: kafka


Description
-------

Rebased on delete-topics


Diffs
-----

  core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 5e016d5d2bbef7fc33fb80c9a4a90ec5d39c16d0 
  core/src/main/scala/kafka/server/ReplicaManager.scala 21bba48affb38ff6504e02535d3cc56258f45b60 

Diff: https://reviews.apache.org/r/16360/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 16360: Patch for KAFKA-1188

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/
-----------------------------------------------------------

(Updated Feb. 12, 2014, 1:49 a.m.)


Review request for kafka.


Bugs: KAFKA-1188
    https://issues.apache.org/jira/browse/KAFKA-1188


Repository: kafka


Description
-------

Rebased on delete-topics


Diffs
-----

  core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 5e016d5d2bbef7fc33fb80c9a4a90ec5d39c16d0 
  core/src/main/scala/kafka/server/ReplicaManager.scala 21bba48affb38ff6504e02535d3cc56258f45b60 

Diff: https://reviews.apache.org/r/16360/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 16360: Patch for KAFKA-1188

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/
-----------------------------------------------------------

(Updated Jan. 21, 2014, 10:01 p.m.)


Review request for kafka.


Summary (updated)
-----------------

Patch for KAFKA-1188


Bugs: KAFKA-1188
    https://issues.apache.org/jira/browse/KAFKA-1188


Repository: kafka


Description (updated)
-------

KAFKA-1188.v7


KAFKA-1188.v6


KAFKA-1188.v5.1


KAFKA-1188.v5


Dummy


KAFKA-1188.v4


KAFKA-1188.v3


KAFKA-1118.v2


KAFKA-1188.v1


Diffs (updated)
-----

  core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 483559aa64726c51320d18b64a1b48f8fb2905a0 
  core/src/main/scala/kafka/server/ReplicaManager.scala f9d10d385cee49a1e3be8c82e3ffa22ef87a8fd6 

Diff: https://reviews.apache.org/r/16360/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 16360: Address comments from Neha, and record error messages if leader brokers are not available since this should not happen then

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/
-----------------------------------------------------------

(Updated Jan. 16, 2014, 11:02 p.m.)


Review request for kafka.


Summary (updated)
-----------------

Address comments from Neha, and record error messages if leader brokers are not available since this should not happen then


Bugs: KAFKA-1188
    https://issues.apache.org/jira/browse/KAFKA-1188


Repository: kafka


Description (updated)
-------

KAFKA-1188.v6


KAFKA-1188.v5.1


KAFKA-1188.v5


Dummy


KAFKA-1188.v4


KAFKA-1188.v3


KAFKA-1118.v2


KAFKA-1188.v1


Diffs (updated)
-----

  core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
  core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 483559aa64726c51320d18b64a1b48f8fb2905a0 
  core/src/main/scala/kafka/server/ReplicaManager.scala f9d10d385cee49a1e3be8c82e3ffa22ef87a8fd6 

Diff: https://reviews.apache.org/r/16360/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 16360: Address comments from Jun

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/
-----------------------------------------------------------

(Updated Jan. 16, 2014, 7:07 p.m.)


Review request for kafka.


Summary (updated)
-----------------

Address comments from Jun


Bugs: KAFKA-1188
    https://issues.apache.org/jira/browse/KAFKA-1188


Repository: kafka


Description (updated)
-------

KAFKA-1188.v5.1


KAFKA-1188.v5


Dummy


KAFKA-1188.v4


KAFKA-1188.v3


KAFKA-1118.v2


KAFKA-1188.v1


Diffs (updated)
-----

  core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
  core/src/main/scala/kafka/server/ReplicaManager.scala f9d10d385cee49a1e3be8c82e3ffa22ef87a8fd6 

Diff: https://reviews.apache.org/r/16360/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 16360: Fix the offset bug while adding fetchers

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/
-----------------------------------------------------------

(Updated Jan. 16, 2014, 12:43 a.m.)


Review request for kafka.


Summary (updated)
-----------------

Fix the offset bug while adding fetchers


Bugs: KAFKA-1188
    https://issues.apache.org/jira/browse/KAFKA-1188


Repository: kafka


Description (updated)
-------

KAFKA-1188.v5


Dummy


KAFKA-1188.v4


KAFKA-1188.v3


KAFKA-1118.v2


KAFKA-1188.v1


Diffs (updated)
-----

  core/src/main/scala/kafka/cluster/Partition.scala 1087a2e91c86e36a2494a95913a3ec2daf238287 
  core/src/main/scala/kafka/server/ReplicaManager.scala f9d10d385cee49a1e3be8c82e3ffa22ef87a8fd6 

Diff: https://reviews.apache.org/r/16360/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 16360: Address Jun's comments v3

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/
-----------------------------------------------------------

(Updated Jan. 2, 2014, 6:44 p.m.)


Review request for kafka.


Summary (updated)
-----------------

Address Jun's comments v3


Bugs: KAFKA-1188
    https://issues.apache.org/jira/browse/KAFKA-1188


Repository: kafka


Description (updated)
-------

KAFKA-1188.v3


KAFKA-1118.v2


KAFKA-1188.v1


Diffs (updated)
-----

  core/src/main/scala/kafka/cluster/Partition.scala 5c9307d71641ccc6c09a54b69d5aa2b4bc2a4cde 
  core/src/main/scala/kafka/server/ReplicaManager.scala 242c18d47828b7c5e6b1fc219a0f1199fb1f9512 

Diff: https://reviews.apache.org/r/16360/diff/


Testing
-------


Thanks,

Guozhang Wang


Re: Review Request 16360: Address Jun's comments

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16360/
-----------------------------------------------------------

(Updated Dec. 24, 2013, 7:07 p.m.)


Review request for kafka.


Summary (updated)
-----------------

Address Jun's comments


Bugs: KAFKA-1188
    https://issues.apache.org/jira/browse/KAFKA-1188


Repository: kafka


Description (updated)
-------

KAFKA-1118.v2


KAFKA-1188.v1


Diffs (updated)
-----

  core/src/main/scala/kafka/cluster/Partition.scala 5c9307d71641ccc6c09a54b69d5aa2b4bc2a4cde 
  core/src/main/scala/kafka/server/ReplicaManager.scala 242c18d47828b7c5e6b1fc219a0f1199fb1f9512 

Diff: https://reviews.apache.org/r/16360/diff/


Testing
-------


Thanks,

Guozhang Wang