----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/#review37265 -----------------------------------------------------------
Ship it! This is great! I don't think we should add a special purpose method that hard codes one property in AdminUtils, I think the helper code is actually there in LogConfig. I see Neha's point about adding a higher-level config to log config but I think it is worth it and not so bad as it is just a config--there is no major code coupling problem. The only real question I have which is really for Neha is whether adding these new ZK queries is a performance concern for leadership election and transfer? Should instead the controller or whoever just watch and constantly replicate these log configs all the time so that when election time comes no new fetches are required. This could perhaps be done as a follow-up improvement... core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/17537/#comment68687> Do we need this? I think we can just do LogConfig.fromProps(AdminUtils.fetchTopicConfig(...)).enableUncleanElection core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/17537/#comment68688> Is adding another zk query here a concern? core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala <https://reviews.apache.org/r/17537/#comment68689> Is adding another zk query here a concern? - Jay Kreps On March 4, 2014, 12:48 a.m., Andrew Olson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17537/ > ----------------------------------------------------------- > > (Updated March 4, 2014, 12:48 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1028 > https://issues.apache.org/jira/browse/KAFKA-1028 > > > Repository: kafka > > > Description > ------- > > KAFKA-1028: per topic configuration of preference for consistency over > availability > > > Diffs > ----- > > core/src/main/scala/kafka/admin/AdminUtils.scala > 36ddeb44490e8343a4e8056c45726ac660e4b2f9 > core/src/main/scala/kafka/common/NoReplicaOnlineException.scala > a1e12794978adf79020936c71259bbdabca8ee68 > core/src/main/scala/kafka/controller/KafkaController.scala > b58cdcd16ffb62ba5329b8b2776f2bd18440b3a0 > core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala > fa29bbe82db35551f43ac487912fba7bae1b2599 > core/src/main/scala/kafka/log/LogConfig.scala > 0b32aeeffcd9d4755ac90573448d197d3f729749 > core/src/main/scala/kafka/server/KafkaConfig.scala > 04a5d39be3c41f5cb3589d95d0bd0f4fb4d7030d > core/src/main/scala/kafka/server/ReplicaFetcherThread.scala > 73e605eb31bc71642d48b0bb8bd1632fd70b9dca > core/src/test/scala/unit/kafka/admin/AdminTest.scala > d5644ea40ec7678b975c4775546b79fcfa9f64b7 > core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala > b585f0ec0b1c402d95a3b34934dab7545dcfcb1f > core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala > PRE-CREATION > core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala > 89c207a3f56c7a7711f8cee6fb277626329882a6 > core/src/test/scala/unit/kafka/utils/TestUtils.scala > 772d2140ed926a2f9f0c99aea60daf1a9b987073 > > Diff: https://reviews.apache.org/r/17537/diff/ > > > Testing > ------- > > > Thanks, > > Andrew Olson > >
