You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jun Rao <ju...@gmail.com> on 2014/05/02 03:09:41 UTC

Re: Review Request 20718: Patch for KAFKA-1384

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



core/src/main/scala/kafka/server/BrokerStates.scala
<https://reviews.apache.org/r/20718/#comment75723>

    In scala, we have been using case classes as enum. See PartitionState.
    
    Also, could PendingShutdownFromController be named PendingControlledShutdown?



core/src/main/scala/kafka/server/KafkaServer.scala
<https://reviews.apache.org/r/20718/#comment75718>

    No need to prepend with the brokerId since the caller will know the broker that it's poking.



core/src/main/scala/kafka/server/KafkaServer.scala
<https://reviews.apache.org/r/20718/#comment75719>

    Will that override the state to broker even though it's set to controller?



core/src/main/scala/kafka/server/KafkaServerStartable.scala
<https://reviews.apache.org/r/20718/#comment75720>

    What's the intention of this method?


- Jun Rao


On April 26, 2014, 7:09 a.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20718/
> -----------------------------------------------------------
> 
> (Updated April 26, 2014, 7:09 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1384
>     https://issues.apache.org/jira/browse/KAFKA-1384
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1384: Logging kafka state metric
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/controller/KafkaController.scala 933de9dd324c7086efe6aa610335ef370d9e9c12 
>   core/src/main/scala/kafka/log/Log.scala f20c232e2daa63a10d91b965af52801af656477c 
>   core/src/main/scala/kafka/log/LogManager.scala ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
>   core/src/main/scala/kafka/server/BrokerStates.scala PRE-CREATION 
>   core/src/main/scala/kafka/server/KafkaServer.scala c208f83bed7fb91f07fae42f2b66892e6d46fecc 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala acda52b801714bcc182edc0ced925f0e4b493fc1 
> 
> Diff: https://reviews.apache.org/r/20718/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


Re: Review Request 20718: Patch for KAFKA-1384

Posted by Timothy Chen <tn...@apache.org>.

> On May 2, 2014, 1:09 a.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaServer.scala, lines 108-109
> > <https://reviews.apache.org/r/20718/diff/2/?file=569132#file569132line108>
> >
> >     Will that override the state to broker even though it's set to controller?

The controller only sets the state when it is elected, and from the server startup steps it will set as broker state first, and once the controller starts up and the elect happens then it will be set to controller state.


> On May 2, 2014, 1:09 a.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaServerStartable.scala, lines 55-58
> > <https://reviews.apache.org/r/20718/diff/2/?file=569133#file569133line55>
> >
> >     What's the intention of this method?

This is to allow custom state that is not defined in the enums. One example is in our custom kafka server startable we can inject more states that is specific to different use cases.


- Timothy


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


On April 26, 2014, 7:09 a.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20718/
> -----------------------------------------------------------
> 
> (Updated April 26, 2014, 7:09 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1384
>     https://issues.apache.org/jira/browse/KAFKA-1384
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1384: Logging kafka state metric
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/controller/KafkaController.scala 933de9dd324c7086efe6aa610335ef370d9e9c12 
>   core/src/main/scala/kafka/log/Log.scala f20c232e2daa63a10d91b965af52801af656477c 
>   core/src/main/scala/kafka/log/LogManager.scala ac67f081e6219fd2181479e7a2bb88ea6044e6cc 
>   core/src/main/scala/kafka/server/BrokerStates.scala PRE-CREATION 
>   core/src/main/scala/kafka/server/KafkaServer.scala c208f83bed7fb91f07fae42f2b66892e6d46fecc 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala acda52b801714bcc182edc0ced925f0e4b493fc1 
> 
> Diff: https://reviews.apache.org/r/20718/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>