----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review82389 -----------------------------------------------------------
Thanks for the patch. A couple of comments below. core/src/main/scala/kafka/api/FetchResponse.scala <https://reviews.apache.org/r/33378/#comment133128> This is tricky since FetchRequest is used in the follower as well. When doing a rolling upgrade of the broker to 0.8.3, we have to follow the following steps. 1. Configure intra.cluster.protocol to 0.8.2 and rolling upgrade each broker to 0.8.3. After this step, each broker understands version 1 of the fetch request, but still sends fetch request in version 0. 2. Configure intra.cluster.protocol to 0.8.3 and restart each broker. After this step, every broker will start sending fetch request in version 1. So, we need the logic in ReplicaFetcherThread to issue the right version of the FetchRequest according to intra.cluster.protocol. We also need to read the response according to the request version (i.e., can't just assume the response is always on the latest version). core/src/main/scala/kafka/api/FetchResponse.scala <https://reviews.apache.org/r/33378/#comment133129> To be consistent with ProduceResponse, we probably want to condition this on reqeust version? - Jun Rao On April 21, 2015, 12:02 a.m., Aditya Auradkar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33378/ > ----------------------------------------------------------- > > (Updated April 21, 2015, 12:02 a.m.) > > > Review request for kafka and Joel Koshy. > > > Bugs: KAFKA-2136 > https://issues.apache.org/jira/browse/KAFKA-2136 > > > Repository: kafka > > > Description > ------- > > Changes are: > - protocol changes to the fetch request and response to return the > throttle_time_ms to clients > - New producer/consumer metrics to expose the avg and max delay time for a > client > - Test cases > > For now the patch will publish a zero delay and return a response > > Added more tests > > > Diffs > ----- > > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java > ef9dd5238fbc771496029866ece1d85db6d7b7a5 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > 70954cadb5c7e9a4c326afcf9d9a07db230e7db2 > clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java > 9c4518e840904c371a5816bfc52be1933cba0b96 > clients/src/main/java/org/apache/kafka/common/requests/FetchRequest.java > 8686d83aa52e435c6adafbe9ff4bd1602281072a > clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java > eb8951fba48c335095cc43fc3672de1c733e07ff > clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java > fabeae3083a8ea55cdacbb9568f3847ccd85bab4 > clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java > 37ec0b79beafcf5735c386b066eb319fb697eff5 > > clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java > 419541011d652becf0cda7a5e62ce813cddb1732 > > clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java > 8b1805d3d2bcb9fe2bacb37d870c3236aa9532c4 > > clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java > e3cc1967e407b64cc734548c19e30de700b64ba8 > core/src/main/scala/kafka/api/FetchRequest.scala > b038c15186c0cbcc65b59479324052498361b717 > core/src/main/scala/kafka/api/FetchResponse.scala > 75aaf57fb76ec01660d93701a57ae953d877d81c > core/src/main/scala/kafka/api/ProducerRequest.scala > 570b2da1d865086f9830aa919a49063abbbe574d > core/src/main/scala/kafka/api/ProducerResponse.scala > 5d1fac4cb8943f5bfaa487f8e9d9d2856efbd330 > core/src/main/scala/kafka/server/DelayedFetch.scala > de6cf5bdaa0e70394162febc63b50b55ca0a92db > core/src/main/scala/kafka/server/DelayedProduce.scala > 05078b24ef28f2f4e099afa943e43f1d00359fda > core/src/main/scala/kafka/server/KafkaApis.scala > b4004aa3a1456d337199aa1245fb0ae61f6add46 > core/src/main/scala/kafka/server/OffsetManager.scala > 420e2c3535e722c503f13d093849469983f6f08d > core/src/main/scala/kafka/server/ReplicaManager.scala > 8ddd325015de4245fd2cf500d8b0e8c1fd2bc7e8 > core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala > 566b5381665bb027a06e17c4fc27414943f85220 > core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala > 9186c90de5a983a73b042fcb42987bfabae14fcf > core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala > 00d59337a99ac135e8689bd1ecd928f7b1423d79 > > Diff: https://reviews.apache.org/r/33378/diff/ > > > Testing > ------- > > New tests added > > > Thanks, > > Aditya Auradkar > >
