> On Aug. 21, 2015, 12:13 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/api/FetchResponse.scala, line 175 > > <https://reviews.apache.org/r/33378/diff/12/?file=1043787#file1043787line175> > > > > Since (in the event of multiple calls) this grouping would be repeated, > > should we just have `responseSize` take the `FetchResponse` object and have > > that look up the `lazy val dataGroupedByTopic`? That said, I think the > > original version should have had `sizeInBytes` as a `lazy val` as well. > > Aditya Auradkar wrote: > FetchResponse.responseSize is called from KafkaApis in order to figure > out what value to record. We cannot pass in a FetchResponse object because > the object doesn't exist yet because the throttle time is not available. > > Should I change the signature to accept a dataGroupedByTopic instead of a > TopicPartition -> FetchResponsePartitionData map.
Got it - yes we could do that. There is also the pre-existing issue of `sizeInBytes` breaking the laziness of `dataGroupedByTopic` which we can address. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review96009 ----------------------------------------------------------- On Aug. 21, 2015, 11:30 p.m., Aditya Auradkar wrote: > > ----------------------------------------------------------- > 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 Jun Rao. > > > Bugs: KAFKA-2136 > https://issues.apache.org/jira/browse/KAFKA-2136 > > > Repository: kafka > > > Description > ------- > > Changes are > - Addressing Joel's comments > - 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. > - Addressed Joel and Juns comments > > > Diffs > ----- > > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java > 9dc669728e6b052f5c6686fcf1b5696a50538ab4 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > 0baf16e55046a2f49f6431e01d52c323c95eddf0 > clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java > 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f > clients/src/main/java/org/apache/kafka/common/requests/FetchRequest.java > df073a0e76cc5cc731861b9604d0e19a928970e0 > clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java > eb8951fba48c335095cc43fc3672de1c733e07ff > clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java > 715504b32950666e9aa5a260fa99d5f897b2007a > clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java > febfc70dabc23671fd8a85cf5c5b274dff1e10fb > > clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java > a7c83cac47d41138d47d7590a3787432d675c1b0 > > clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java > 8b1805d3d2bcb9fe2bacb37d870c3236aa9532c4 > > clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java > 8b2aca85fa738180e5420985fddc39a4bf9681ea > core/src/main/scala/kafka/api/FetchRequest.scala > 5b38f8554898e54800abd65a7415dd0ac41fd958 > core/src/main/scala/kafka/api/FetchResponse.scala > b9efec2efbd3ea0ee12b911f453c47e66ad34894 > core/src/main/scala/kafka/api/ProducerRequest.scala > c866180d3680da03e48d374415f10220f6ca68c4 > core/src/main/scala/kafka/api/ProducerResponse.scala > 5d1fac4cb8943f5bfaa487f8e9d9d2856efbd330 > core/src/main/scala/kafka/consumer/SimpleConsumer.scala > 7ebc0405d1f309bed9943e7116051d1d8276f200 > core/src/main/scala/kafka/server/AbstractFetcherThread.scala > f84306143c43049e3aa44e42beaffe7eb2783163 > core/src/main/scala/kafka/server/ClientQuotaManager.scala > 9f8473f1c64d10c04cf8cc91967688e29e54ae2e > core/src/main/scala/kafka/server/KafkaApis.scala > 67f0cad802f901f255825aa2158545d7f5e7cc3d > core/src/main/scala/kafka/server/ReplicaFetcherThread.scala > fae22d2af8daccd528ac24614290f46ae8f6c797 > core/src/main/scala/kafka/server/ReplicaManager.scala > d829e180c3943a90861a12ec184f9b4e4bbafe7d > core/src/main/scala/kafka/server/ThrottledResponse.scala > 1f80d5480ccf7c411a02dd90296a7046ede0fae2 > core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala > b4c2a228c3c9872e5817ac58d3022e4903e317b7 > core/src/test/scala/unit/kafka/server/ClientQuotaManagerTest.scala > caf98e8f2e09d39ab8234b9f4b9478686865e908 > core/src/test/scala/unit/kafka/server/ThrottledResponseExpirationTest.scala > c4b5803917e700965677d53624f1460c1a52bf52 > > Diff: https://reviews.apache.org/r/33378/diff/ > > > Testing > ------- > > New tests added > > > Thanks, > > Aditya Auradkar > >
