> On March 14, 2014, 8:13 p.m., Jay Kreps wrote: > > 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... > > > >
Thanks for the feedback. The LogConfig.fromProps(...) approach works, but it looks like I'd need to update the implementation to provide default values for each of the props.getProperty(...) calls (it currently assumes that all supported properties are included in the provided Properties). Should I go ahead and make that change? - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/#review37265 ----------------------------------------------------------- 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 > >