----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23266/#review47414 -----------------------------------------------------------
Three very minor issues: 1. It would be good to use a while loop when checking the condition in forceUpdate() (http://stackoverflow.com/questions/2536692/a-simple-scenario-using-wait-and-notify-in-java). In this case what you have actually works because the forceUpdate() method itself is called in a loop, but it would be better if forceUpdate() guaranteed its own postcondition. 2. It would be good to think through the case where a metadata request takes less than 1ms, as the avg. roundtrip I see is about .4 ms. This should not lead to waiting for maxWait if possible. One approach to this would be to use a logical version number. Another would be to have forceUpdate(long time) ensure that the metadata is at least as fresh as the given time. I think either would work. 3. Maybe awaitUpdate(int desiredVerison, long now) would be a better name? - Jay Kreps On July 7, 2014, 5:55 p.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23266/ > ----------------------------------------------------------- > > (Updated July 7, 2014, 5:55 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1515 > https://issues.apache.org/jira/browse/KAFKA-1515 > > > Repository: kafka > > > Description > ------- > > 1. Move the waiting logic out of Metadata into KafkaProducer (KafkaConsumer > would not wait on fetch metadata, so will not share this code); and wake-up > sender upon waiting metadata 2. Set the refresh timestamp to now instead of 0 > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/NetworkClient.java > d21f9225539b070f9b50b7a76601d80b83daf7ef > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > d85ca30001dc3d6122a890c34092551654315458 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java > 8890aa2e3ce5fffc159b3c8528138226e8c8cfd3 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > 37b9d1a462d42b811fffd2af3c418e3a9179f00f > clients/src/test/java/org/apache/kafka/clients/producer/MetadataTest.java > 0d7d04ca5d71d4db22da54ed2153f7c0e10cdf78 > > Diff: https://reviews.apache.org/r/23266/diff/ > > > Testing > ------- > > > Thanks, > > Guozhang Wang > >
