Re: Review Request 33378: Patch for KAFKA-2136

> On Aug. 21, 2015, 12:13 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/FetchResponse.scala, line 175 > > > > > > Since (in the event of multiple calls) this grouping would be repeated, > > should we ju

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review96470 --- Ship it! Ship It! - Joel Koshy On Aug. 25, 2015, 6:30 p.m., Adit

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated Aug. 25, 2015, 6:30 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated Aug. 25, 2015, 6:30 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

> On Aug. 22, 2015, 12:45 a.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java, > > line 107 > > > > > > This will probably need a versionId as well (as is done i

Re: Review Request 33378: Patch for KAFKA-2136

> On Aug. 22, 2015, 12:45 a.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java, > > line 107 > > > > > > This will probably need a versionId as well (as is done i

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review96186 --- core/src/main/scala/kafka/api/FetchResponse.scala (line 183)

Re: Review Request 33378: Patch for KAFKA-2136

> On Aug. 22, 2015, 12:45 a.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java, > > line 107 > > > > > > This will probably need a versionId as well (as is done i

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated Aug. 24, 2015, 5:33 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated Aug. 24, 2015, 5:33 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review96100 --- clients/src/main/java/org/apache/kafka/common/requests/FetchRespons

Re: Review Request 33378: Patch for KAFKA-2136

> On Aug. 21, 2015, 12:13 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/FetchResponse.scala, line 175 > > > > > > Since (in the event of multiple calls) this grouping would be repeated, > > should we ju

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated Aug. 21, 2015, 11:30 p.m.) Review request for kafka, Joel Koshy and Ju

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated Aug. 21, 2015, 11:29 p.m.) Review request for kafka, Joel Koshy and Ju

Re: Review Request 33378: Patch for KAFKA-2136

> On Aug. 21, 2015, 12:13 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/ClientQuotaManager.scala, line 142 > > > > > > any specific reason for this change? recordAndMaybeThrottle returns an int value

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review96009 --- core/src/main/scala/kafka/api/FetchResponse.scala (line 172)

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated Aug. 18, 2015, 8:24 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated Aug. 18, 2015, 8:24 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated Aug. 18, 2015, 8:20 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

> On Aug. 5, 2015, 4:44 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java, > > line 55 > > > > > > We actually will need different constructors for different versio

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review94876 --- Jun/Joel - Thanks for the comments. I'd like to address these all at

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review94822 --- core/src/main/scala/kafka/server/ReplicaManager.scala (line 312)

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review94167 --- Thanks for the patch. A few comments below. clients/src/main/java/

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review93979 --- This patch also needs a rebase. core/src/main/scala/kafka/server/R

Re: Review Request 33378: Patch for KAFKA-2136

> On July 10, 2015, 5:49 p.m., Joel Koshy wrote: > > LGTM - just a few minor comments. Also, I filed this ticket to add metrics to the old producer and consumers: https://issues.apache.org/jira/browse/KAFKA-2332 - Aditya --- This is an

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated July 13, 2015, 8:36 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated July 13, 2015, 8:34 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

> On June 25, 2015, 10:55 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/AbstractFetcherThread.scala, line 40 > > > > > > I think we should add throttle time metrics to the old producer and > > consumer a

Re: Review Request 33378: Patch for KAFKA-2136

> On July 10, 2015, 5:49 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/DelayedFetch.scala, line 135 > > > > > > For these, I'm wondering if we should put in the actual delay and in > > KAFKA-2136 just a

Re: Review Request 33378: Patch for KAFKA-2136

> On June 25, 2015, 10:55 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/AbstractFetcherThread.scala, line 40 > > > > > > I think we should add throttle time metrics to the old producer and > > consumer a

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review91319 --- LGTM - just a few minor comments. clients/src/main/java/org/apache

Re: Review Request 33378: Patch for KAFKA-2136

> On June 25, 2015, 10:55 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/AbstractFetcherThread.scala, line 40 > > > > > > I think we should add throttle time metrics to the old producer and > > consumer a

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated July 1, 2015, 2:44 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated July 1, 2015, 2:44 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review89429 --- Thanks for the updated patch. Few more comments.. clients/src/main

Re: Review Request 33378: Patch for KAFKA-2136

> On June 5, 2015, 2:43 a.m., Joel Koshy wrote: > > Overall, looks good. > > > > General comment on the naming: delay vs throttle. Personally, I prefer > > throttle - I think that is clearer from the client perspective. > > > > Should probably add a test to verify that the replica fetchers are

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated June 9, 2015, 5:10 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated June 9, 2015, 5:10 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated June 9, 2015, 5:08 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated June 9, 2015, 5:07 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated June 9, 2015, 1:37 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

> On June 5, 2015, 2:43 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/FetchResponse.scala, line 143 > > > > > > follow-up Can you elaborate? > On June 5, 2015, 2:43 a.m., Joel Koshy wrote: > > core/src/m

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review86428 --- Overall, looks good. General comment on the naming: delay vs thrott

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated May 12, 2015, 9:42 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

> On May 12, 2015, 7:39 p.m., Dong Lin wrote: > > core/src/main/scala/kafka/server/ReplicaManager.scala, line 428 > > > > > > Will readFromLocalLog() read data into memory before the cilent's quota > > is checked? It

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated May 12, 2015, 9:40 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review83439 --- core/src/main/scala/kafka/server/ReplicaManager.scala

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review83341 --- core/src/main/scala/kafka/api/FetchResponse.scala

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated May 11, 2015, 9:51 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33378: Patch for KAFKA-2136

> On May 4, 2015, 4:51 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/api/FetchResponse.scala, lines 152-153 > > > > > > This is tricky since FetchRequest is used in the follower as well. When > > doing a rolling

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated May 7, 2015, 1:36 a.m.) Review request for kafka and Joel Koshy. Bug

Re: Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- (Updated May 7, 2015, 1:32 a.m.) Review request for kafka and Joel Koshy. Bug

Re: Review Request 33378: Patch for KAFKA-2136

--- 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/sc

Re: Review Request 33378: Patch for KAFKA-2136

--- 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.

Re: Review Request 33378: Patch for KAFKA-2136

--- 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.

Review Request 33378: Patch for KAFKA-2136

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/ --- Review request for kafka. Bugs: KAFKA-2136 https://issues.apache.org/jira/b