> On July 17, 2015, 4:26 a.m., Jiangjie Qin wrote: > > Looks good to me. > > Jiangjie Qin wrote: > Actually do we need to talk to Zookeeper every time? Can we read the data > from topic metadata cache directly? > > Gwen Shapira wrote: > Good point, Jiangjie - looks like partitionFor is called on every > ConsumerMetadataRequest handling, so some kind of caching will be nice. > > Grant Henke wrote: > We only talk to Zookeeper once at instance creation via the call to > `getOffsetsTopicPartitionCount` and setting the val > `offsetsTopicPartitionCount`. The static value is used from then on for every > call to `partitionFor`. > > Jiangjie Qin wrote: > Ah, I see, my bad. Then this patch seems not completely solve the issue, > though. Let's say offsets topic is not exist yet. What if two brokers had > different offset topic partition number configuration? After they startup and > before the offset topic get created in zookeeper, they will have different > value for offsetsTopicPartitionCount. Will that cause problem silently? > > Grant Henke wrote: > I think that sort of issue exists for many of Kafka's configurations, and > exists for this configuration without this patch too. I do not aim to solve > non-uniform configuration in this patch. > > Jiangjie Qin wrote: > I agree that configuration mismatch could cause issue for other > configurations as well. But having existing problems does not mean we should > introduce one more to them. Besides, is it a simpler solution to just read > from topic metadata cache? > > Gwen Shapira wrote: > Was there a resolution here? > > Grant, will reading number of partitions from topic metadata cache solve > the edge case that Becket raised?
This is something that I have been thinking about for a little while. I think reading from topic metadata would help the scenario Becket raised, though it is a very rare edge case and would only occur when both the offset topic does not exist yet and the brokers do not have uniform configurations. I can make the change to use the cache, but the thing is I will either need to pass it via the constructors down through ConsumerCoordinator and OffsetManager from KafkaApis or through the partitionsFor calls in each. And though its sounds mroe simple to use a cache...chaches are never simple. These are the trade offs I have been considering. - Grant ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36548/#review92020 ----------------------------------------------------------- On July 16, 2015, 6:04 p.m., Grant Henke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36548/ > ----------------------------------------------------------- > > (Updated July 16, 2015, 6:04 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2336 > https://issues.apache.org/jira/browse/KAFKA-2336 > > > Repository: kafka > > > Description > ------- > > Fix Scala style > > > Diffs > ----- > > core/src/main/scala/kafka/server/OffsetManager.scala > 47b6ce93da320a565435b4a7916a0c4371143b8a > > Diff: https://reviews.apache.org/r/36548/diff/ > > > Testing > ------- > > > Thanks, > > Grant Henke > >
