I have changed the semantics of awaitUpdate (renamed from forceUpdate) to "wait for one update on the metadata, but if the current metadata version is already larger than the known last version that was passed in, return immediately". How do you think of this change?
Guozhang ________________________________ From: Guozhang Wang Sent: Monday, July 07, 2014 1:49 PM To: Jay Kreps; kafka Subject: RE: Review Request 23266: Fix KAFKA-1515 Jay, About 1), I just remembered why I did not use a while loop inside forceUpdate, which is because every time we are interrupted but not jumping out of the loop, we need to call sender.wakeup(), which can only be done in the caller. Guozhang ________________________________ From: Jay Kreps [nore...@reviews.apache.org] on behalf of Jay Kreps [boredandr...@gmail.com] Sent: Monday, July 07, 2014 1:45 PM To: Jay Kreps; kafka; Guozhang Wang Subject: Re: Review Request 23266: Fix KAFKA-1515 This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23266/ 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 7th, 2014, 5:55 p.m. UTC, Guozhang Wang wrote: Review request for kafka. By Guozhang Wang. Updated July 7, 2014, 5:55 p.m. 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) View Diff<https://reviews.apache.org/r/23266/diff/>
