kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Guozhang Wang" <guw...@linkedin.com>
Subject Re: Review Request 17248: Patch for KAFKA-1215
Date Mon, 03 Mar 2014 18:38:03 GMT

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


Some high-level comments:

1. Instead of exposing the maxRackReplica parameter throughout the calling hierarchy and also
in the API function, could we just wrap it in these places:

a. assignReplicasToBrokers, the only place we need to read this parameter. This function is
called by

createTopic

addPartitions

generateAssignment

All of these three have zkClient, so we can read the cluster and maxRackReplica information
just there.

b. createOrUpdateTopicPartitionAssignmentPathInZK, one of the two places we need to write
the value from the command line tool. Instead of specify this parameter we can treat it as
part of the general topic configs (i.e. just add this value in parseTopicConfigsToBeAdded).

c. updateAssignedReplicasForPartition, the other place we need to write the value from the
controller. Currently the logic never changes this value, what it does is just read the original
value and write again. So we can just avoid doing so by just moving this value to the general
configs, like I said in b).

2. Broker.scala is used by the producer, who do not need to know the rack information.







- Guozhang Wang


On Feb. 1, 2014, 12:20 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17248/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2014, 12:20 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1215
>     https://issues.apache.org/jira/browse/KAFKA-1215
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1226
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/admin/AdminUtils.scala a167756 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala 2637586 
>   core/src/main/scala/kafka/admin/TopicCommand.scala 842c110 
>   core/src/main/scala/kafka/client/ClientUtils.scala 1d2f81b 
>   core/src/main/scala/kafka/cluster/Broker.scala 9407ed2 
>   core/src/main/scala/kafka/controller/KafkaController.scala a0267ae 
>   core/src/main/scala/kafka/controller/PartitionStateMachine.scala ac4262a 
>   core/src/main/scala/kafka/server/KafkaApis.scala 29abc46 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 3c3aafc 
>   core/src/main/scala/kafka/server/KafkaHealthcheck.scala 9dca55c 
>   core/src/main/scala/kafka/server/KafkaServer.scala 5e34f95 
>   core/src/main/scala/kafka/utils/ZkUtils.scala b42e52b 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 115e203 
>   core/src/test/scala/unit/kafka/admin/AdminTest.scala 59de1b4 
>   core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 8df0982 
>   core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala 9347ea6 
>   core/src/test/scala/unit/kafka/integration/FetcherTest.scala 47130d3 
>   core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala 9998a11 
>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala 18e3555 
>   core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala 38e3ae7 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala d88b6c3 
>   examples/README d33f6c5 
> 
> Diff: https://reviews.apache.org/r/17248/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> rack_aware_replica_assignment_v1.patch
>   https://reviews.apache.org/media/uploaded/files/2014/01/23/394cef99-f800-4d94-bc59-fdb6c68b53f5__rack_aware_replica_assignment_v1.patch
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message