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 https://reviews.apache.org/r/33378/diff/12-13/?file=1043780#file1043780line107 This will probably need a versionId as well (as is done in the Scala response) - i.e., when we move the broker over to use these protocol schemas. Aditya Auradkar wrote: Makes sense. Do you want me to tackle this in this patch or should it be tackled in the patch that migrates the broker to use these schemas? Joel Koshy wrote: I think it would be safer to do it in this patch itself. Do you think we actually need to check the version id? The Struct contains the schema which should be sufficient to understand the version number right? - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review96100 --- On Aug. 24, 2015, 5:33 p.m., Aditya Auradkar wrote: --- 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 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- Addressing Joel's comments Merging Chaning variable name Addressing Joel's comments Addressing Joel's comments Addressing comments Addressing joels comments Addressing joels comments Addressed Joels comments Diffs (updated) - 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- 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
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., Aditya Auradkar wrote: --- 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 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
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 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. Joel Koshy wrote: 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. I realized later that this now causes two explicit group-by's on the server-side and one on the consumer-side. So the non-lazy val may have worked better in practice. - Joel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review96009 --- On Aug. 25, 2015, 6:30 p.m., Aditya Auradkar wrote: --- 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 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
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 https://reviews.apache.org/r/33378/diff/12-13/?file=1043780#file1043780line107 This will probably need a versionId as well (as is done in the Scala response) - i.e., when we move the broker over to use these protocol schemas. Makes sense. Do you want me to tackle this in this patch or should it be tackled in the patch that migrates the broker to use these schemas? - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review96100 --- On Aug. 24, 2015, 5:33 p.m., Aditya Auradkar wrote: --- 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 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- 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
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 https://reviews.apache.org/r/33378/diff/12-13/?file=1043780#file1043780line107 This will probably need a versionId as well (as is done in the Scala response) - i.e., when we move the broker over to use these protocol schemas. Aditya Auradkar wrote: Makes sense. Do you want me to tackle this in this patch or should it be tackled in the patch that migrates the broker to use these schemas? I think it would be safer to do it in this patch itself. - Joel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review96100 --- On Aug. 24, 2015, 5:33 p.m., Aditya Auradkar wrote: --- 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 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
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 Jun Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- 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
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 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
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/FetchResponse.java (line 33) https://reviews.apache.org/r/33378/#comment151319 of FetchResponse clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java (line 107) https://reviews.apache.org/r/33378/#comment151321 This will probably need a versionId as well (as is done in the Scala response) - i.e., when we move the broker over to use these protocol schemas. - Joel Koshy 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
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 https://reviews.apache.org/r/33378/diff/12/?file=1043792#file1043792line142 any specific reason for this change? recordAndMaybeThrottle returns an int value as delay. Nicer to have delayTime return an int as well 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. 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. - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review96009 --- On Aug. 18, 2015, 8:24 p.m., Aditya Auradkar wrote: --- 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 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 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 97dcca8c96f955acb3d92b29d7faa1e031ba71d4 core/src/test/scala/unit/kafka/server/ThrottledResponseExpirationTest.scala 14a7f4538041d557c190127e3d5f85edf2a0e7c1 Diff: https://reviews.apache.org/r/33378/diff/
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 Jun Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- Addressing Joel's comments Merging Chaning variable name Addressing Joel's comments Addressing Joel's comments Addressing comments Addressing joels comments Diffs (updated) - 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
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) https://reviews.apache.org/r/33378/#comment151206 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. core/src/main/scala/kafka/server/ClientQuotaManager.scala (line 115) https://reviews.apache.org/r/33378/#comment151207 `throttleTimeMs` for consistency core/src/main/scala/kafka/server/ClientQuotaManager.scala (line 119) https://reviews.apache.org/r/33378/#comment151209 Maybe make this explicitly zero, and `delayTime` can move below as a `val` core/src/main/scala/kafka/server/ClientQuotaManager.scala (line 142) https://reviews.apache.org/r/33378/#comment151208 any specific reason for this change? - Joel Koshy On Aug. 18, 2015, 8:24 p.m., Aditya Auradkar wrote: --- 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 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 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 97dcca8c96f955acb3d92b29d7faa1e031ba71d4 core/src/test/scala/unit/kafka/server/ThrottledResponseExpirationTest.scala 14a7f4538041d557c190127e3d5f85edf2a0e7c1 Diff: https://reviews.apache.org/r/33378/diff/ Testing --- New tests added Thanks, Aditya Auradkar
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- 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 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 97dcca8c96f955acb3d92b29d7faa1e031ba71d4 core/src/test/scala/unit/kafka/server/ThrottledResponseExpirationTest.scala 14a7f4538041d557c190127e3d5f85edf2a0e7c1 Diff: https://reviews.apache.org/r/33378/diff/ Testing --- New tests added Thanks, Aditya Auradkar
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- kafka-2005; Generate html report for system tests; patched by Ashish Singh; reviewed by Jun Rao kafka-2266; Client Selector can drop idle connections without notifying NetworkClient; patched by Jason Gustafson; reviewed by Jun Rao kafka-2232; make MockProducer generic; patched by Alexander Pakulov; reviewed by Jun Rao kafka-2164; ReplicaFetcherThread: suspicious log message on reset offset; patched by Alexey Ozeritski; reviewed by Jun Rao kafka-2101; Metric metadata-age is reset on a failed update; patched by Tim Brooks; reviewed by Jun Rao kafka-2195; Add versionId to AbstractRequest.getErrorResponse and AbstractRequest.getRequest; patched by Andrii Biletskyi; reviewed by Jun Rao kafka-2270; incorrect package name in unit tests; patched by Proneet Verma; reviewed by Jun Rao kafka-2272; listeners endpoint parsing fails if the hostname has capital letter; patched by Sriharsha Chintalapani; reviewed by Jun Rao kafka-2264; SESSION_TIMEOUT_MS_CONFIG in ConsumerConfig should be int; patched by Manikumar Reddy; reviewed by Jun Rao kafka-2252; Socket connection closing is logged, but not corresponding opening of socket; patched by Gwen Shapira; reviewed by Jun Rao kafka-2262; LogSegmentSize validation should be consistent; patched by Manikumar Reddy; reviewed by Jun Rao trivial fix for stylecheck error on Jenkins kafka-2249; KafkaConfig does not preserve original Properties; patched by Gwen Shapira; reviewed by Jun Rao kafka-2265; creating a topic with large number of partitions takes a long time; patched by Manikumar Reddy; reviewed by Jun Rao kafka-2234; Partition reassignment of a nonexistent topic prevents future reassignments; patched by Manikumar Reddy; reviewed by Jun Rao trivial change to fix unit test failure introduced in kafka-2234 kafka-1758; corrupt recovery file prevents startup; patched by Manikumar Reddy; reviewed by Neha Narkhede and Jun Rao kafka-1646; Improve consumer read performance for Windows; patched by Honghai Chen; reviewed by Jay Kreps and Jun Rao kafka-2012; Broker should automatically handle corrupt index files; patched by Manikumar Reddy; reviewed by Jun Rao kafka-2290; OffsetIndex should open RandomAccessFile consistently; patched by Chris Black; reviewed by Jun Rao kafka-2235; LogCleaner offset map overflow; patched by Ivan Simoneko; reviewed by Jun Rao KAFKA-2245; Add response tests for consumer coordinator; reviewed by Joel Koshy KAFKA-2293; Fix incorrect format specification in Partition.scala; reviewed by Joel Koshy kafka-2168; New consumer poll() can block other calls like position(), commit(), and close() indefinitely; patched by Jason Gustafson; reviewed by Jay Kreps, Ewen Cheslack-Postava, Guozhang Wang and Jun Rao KAFKA-2294; javadoc compile error due to illegal p/ , build failing (jdk 8); patched by Jeff Maxwell; reviewed by Jakob Homan KAFKA-2281: avoid unnecessary value copying if logAsString is false; reviewed by Guozhang Wang KAFKA-2168: minor follow-up patch; reviewed by Guozhang Wang KAFKA-1740: merge offset manager into consumer coordinator; reviewed by Onur Karaman and Jason Gustafson kafka-2248; Use Apache Rat to enforce copyright headers; patched by Ewen Cheslack-Postava; reviewed by Gwen Shapira, Joel Joshy and Jun Rao kafka-2132; Move Log4J appender to a separate module; patched by Ashish Singh; reviewed by Gwen Shapira, Aditya Auradkar and Jun Rao KAFKA-2314: proper MirrorMaker's message handler help message; reviewed by Guozhang Wang kafka-1367; Broker topic metadata not kept in sync with ZooKeeper; patched by Ashish Singh; reviewed by Jun Rao KAFKA-2304 Supported enabling JMX in Kafka Vagrantfile patch by Stevo Slavic reviewed by Ewen Cheslack-Postava KAFKA-2306: add another metric for buffer exhausted; reviewed by Guozhang Wang KAFKA-2317: follow-up of KAFKA1367; reviewed by Guozhang Wang KAFKA-2313: javadoc fix for KafkaConsumer deserialization; reviewed by Guozhang Wang KAFKA-2298; Client Selector can drop connections on InvalidReceiveException without notifying NetworkClient; reviewed by Jason Gustafson and Joel Koshy Trivial commit - explicitly exclude build/rat-report.xml from rat check KAFKA-2308: make MemoryRecords idempotent; reviewed by Guozhang Wang KAFKA-2316: Drop java 1.6 support; patched by Sriharsha Chintalapani reviewed by Ismael Juma and Gwen Shapira KAFKA-2327; broker doesn't start if config defines advertised.host but not advertised.port Added unit tests as well. These fail without the fix, but pass
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- Addressing Joel's comments Merging Chaning variable name Addressing Joel's comments Addressing Joel's comments Addressing comments Diffs (updated) - 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 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 97dcca8c96f955acb3d92b29d7faa1e031ba71d4 core/src/test/scala/unit/kafka/server/ThrottledResponseExpirationTest.scala 14a7f4538041d557c190127e3d5f85edf2a0e7c1 Diff: https://reviews.apache.org/r/33378/diff/ Testing --- New tests added Thanks, Aditya Auradkar
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 https://reviews.apache.org/r/33378/diff/10/?file=1010009#file1010009line55 We actually will need different constructors for different versions. We want to reuse those request/response objects on the broker side. So, the broker will need to construct different version of the response based on the version of the request. You can take a look at OffsetCommitRequest as an example. Good point. - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review94167 --- On July 13, 2015, 8:36 p.m., Aditya Auradkar wrote: --- 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 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's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 once after KAFKA-2084 is committed because I will need to rebase after that. - Aditya Auradkar On July 13, 2015, 8:36 p.m., Aditya Auradkar wrote: --- 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 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's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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)
https://reviews.apache.org/r/33378/#comment149463
Good point. I think we should commit 2084 before this patch.
While rebasing, I can refactor this patch to pass the throttleTime to the
callback that sends the response. i.e.
quotaManagers.recordAndMaybeThrottle (clientId, value, callback) {
// add to delay queue
// pass in the computed throttle time to the callback.
}
If I do that, I dont need to pass the throttleTime to the responseCallback
in ReplicaManager
- Aditya Auradkar
On July 13, 2015, 8:36 p.m., Aditya Auradkar wrote:
---
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 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's comments
For now the patch will publish a zero delay and return a response
Diffs
-
clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
56281ee15cc33dfc96ff64d5b1e596047c7132a4
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java
07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd
clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java
3dc8b015afd2347a41c9a9dbc02b8e367da5f75f
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
5b38f8554898e54800abd65a7415dd0ac41fd958
core/src/main/scala/kafka/api/FetchResponse.scala
0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215
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
c16f7edd322709060e54c77eb505c44cbd77a4ec
core/src/main/scala/kafka/server/AbstractFetcherThread.scala
83fc47417dd7fe4edf030217fa7fd69d99b170b0
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
d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd
core/src/main/scala/kafka/server/OffsetManager.scala
5cca85cf727975f6d3acb2223fd186753ad761dc
core/src/main/scala/kafka/server/ReplicaFetcherThread.scala
b31b432a226ba79546dd22ef1d2acbb439c2e9a3
core/src/main/scala/kafka/server/ReplicaManager.scala
59c9bc3ac3a8afc07a6f8c88c5871304db588d17
core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala
5717165f2344823fabe8f7cfafae4bb8af2d949a
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
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/org/apache/kafka/common/requests/FetchResponse.java (line
77)
https://reviews.apache.org/r/33378/#comment148702
With this, we are bounding the constructor to the v1 protocol. This is fine
for FetchResponse for now since the broker uses the scala objects for dealing
with the send file api. However, we should add a comment to indicate this is
the constructor for v1 protocol.
clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java
(line 55)
https://reviews.apache.org/r/33378/#comment148701
We actually will need different constructors for different versions. We
want to reuse those request/response objects on the broker side. So, the broker
will need to construct different version of the response based on the version
of the request. You can take a look at OffsetCommitRequest as an example.
core/src/main/scala/kafka/api/FetchResponse.scala (lines 193 - 195)
https://reviews.apache.org/r/33378/#comment148704
No need to wrap single line statement with {}.
core/src/main/scala/kafka/api/ProducerResponse.scala (line 40)
https://reviews.apache.org/r/33378/#comment148709
Perhaps it's worth adding a commment that readFrom() only reads v1 format.
core/src/main/scala/kafka/api/ProducerResponse.scala (lines 49 - 50)
https://reviews.apache.org/r/33378/#comment148707
Coding style: no space before :.
Also, could requestVersion be just version?
core/src/main/scala/kafka/api/ProducerResponse.scala (lines 95 - 97)
https://reviews.apache.org/r/33378/#comment148706
Coding style: no need to wrap single line statement with {}.
- Jun Rao
On July 13, 2015, 8:36 p.m., Aditya Auradkar wrote:
---
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 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's comments
For now the patch will publish a zero delay and return a response
Diffs
-
clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
56281ee15cc33dfc96ff64d5b1e596047c7132a4
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java
07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd
clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java
3dc8b015afd2347a41c9a9dbc02b8e367da5f75f
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
5b38f8554898e54800abd65a7415dd0ac41fd958
core/src/main/scala/kafka/api/FetchResponse.scala
0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215
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
c16f7edd322709060e54c77eb505c44cbd77a4ec
core/src/main/scala/kafka/server/AbstractFetcherThread.scala
83fc47417dd7fe4edf030217fa7fd69d99b170b0
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
d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd
core/src/main/scala/kafka/server/OffsetManager.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/#review93979 --- This patch also needs a rebase. core/src/main/scala/kafka/server/ReplicaManager.scala (line 312) https://reviews.apache.org/r/33378/#comment148455 I'm a bit unclear on how you are planning to put in the right delay value in the response struct. i.e., in KAFKA-2084 you are computing the delay inside the callback. How will that value be accessed here? - Joel Koshy On July 13, 2015, 8:36 p.m., Aditya Auradkar wrote: --- 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 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's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 https://reviews.apache.org/r/33378/diff/9/?file=996359#file996359line135 For these, I'm wondering if we should put in the actual delay and in KAFKA-2136 just add a config to enable/disable quotas altogether. Hey Joel.. can you elaborate? The actual delay isn't being computed in this patch. - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review91319 --- On July 1, 2015, 2:44 a.m., Aditya Auradkar wrote: --- 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 Rao. 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. - Addressed Joel's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- Addressing Joel's comments Merging Chaning variable name Addressing Joel's comments Addressing Joel's comments Diffs (updated) - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 https://reviews.apache.org/r/33378/diff/8/?file=981582#file981582line40 I think we should add throttle time metrics to the old producer and consumer as well. What do you think? Aditya Auradkar wrote: I think that sounds reasonable.. I initially decided against it in my patch because I thought of this as an incentive to upgrade. Any concerns if I submit a subsequent RB for this immediately after this is committed? Joel Koshy wrote: I think it is definitely something that we will need (for users that are still on old clients). So can you either create a separate jira labeled as quotas or do that as part of this patch? Here you go. I'll work on it ASAP https://issues.apache.org/jira/browse/KAFKA-2332 - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review89429 --- On July 1, 2015, 2:44 a.m., Aditya Auradkar wrote: --- 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 Rao. 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. - Addressed Joel's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- 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's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review91319 --- On July 13, 2015, 8:36 p.m., Aditya Auradkar wrote: --- 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 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's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 https://reviews.apache.org/r/33378/diff/8/?file=981582#file981582line40 I think we should add throttle time metrics to the old producer and consumer as well. What do you think? Aditya Auradkar wrote: I think that sounds reasonable.. I initially decided against it in my patch because I thought of this as an incentive to upgrade. Any concerns if I submit a subsequent RB for this immediately after this is committed? I think it is definitely something that we will need (for users that are still on old clients). So can you either create a separate jira labeled as quotas or do that as part of this patch? - Joel --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review89429 --- On July 1, 2015, 2:44 a.m., Aditya Auradkar wrote: --- 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 Rao. 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. - Addressed Joel's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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/kafka/common/requests/ProduceResponse.java
(line 101)
https://reviews.apache.org/r/33378/#comment144632
Would prefer seeing this in this style:
```
func() {
statement;
}
```
clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java
(line 68)
https://reviews.apache.org/r/33378/#comment144638
EPSILON?
core/src/main/scala/kafka/api/FetchResponse.scala (line 169)
https://reviews.apache.org/r/33378/#comment144643
A: B
core/src/main/scala/kafka/api/FetchResponse.scala (line 170)
https://reviews.apache.org/r/33378/#comment144644
A: B
core/src/main/scala/kafka/api/FetchResponse.scala (line 179)
https://reviews.apache.org/r/33378/#comment144646
Prefer placing throttleTimeSize on the next line.
core/src/main/scala/kafka/api/FetchResponse.scala (line 229)
https://reviews.apache.org/r/33378/#comment144655
The throttleTimeSize...
(However, see following comment)
core/src/main/scala/kafka/api/FetchResponse.scala (line 230)
https://reviews.apache.org/r/33378/#comment144653
Just wondering if we should move all this to `FetchResponse` class (not
object).
i.e., add `headerSize` and `writeHeaderTo(buffer)` member methods since the
throttle-time is really part of the header. Depending on the version, those two
methods can then do the right thing. This code will then just become:
```
private val buffer = ByteBuffer.allocate(4 + fetchResponse.headerSize)
fetchResponse.writeHeaderTo(buffer)
```
core/src/main/scala/kafka/server/DelayedFetch.scala (line 135)
https://reviews.apache.org/r/33378/#comment144657
For these, I'm wondering if we should put in the actual delay and in
KAFKA-2136 just add a config to enable/disable quotas altogether.
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala (line 81)
https://reviews.apache.org/r/33378/#comment144658
delayTime: Int
- Joel Koshy
On July 1, 2015, 2:44 a.m., Aditya Auradkar wrote:
---
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 Rao.
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.
- Addressed Joel's comments
For now the patch will publish a zero delay and return a response
Diffs
-
clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java
56281ee15cc33dfc96ff64d5b1e596047c7132a4
clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java
07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd
clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java
3dc8b015afd2347a41c9a9dbc02b8e367da5f75f
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
5b38f8554898e54800abd65a7415dd0ac41fd958
core/src/main/scala/kafka/api/FetchResponse.scala
0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215
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
c16f7edd322709060e54c77eb505c44cbd77a4ec
core/src/main/scala/kafka/server/AbstractFetcherThread.scala
83fc47417dd7fe4edf030217fa7fd69d99b170b0
core/src/main/scala/kafka/server/DelayedFetch.scala
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 https://reviews.apache.org/r/33378/diff/8/?file=981582#file981582line40 I think we should add throttle time metrics to the old producer and consumer as well. What do you think? I think that sounds reasonable.. I initially decided against it in my patch because I thought of this as an incentive to upgrade. Any concerns if I submit a subsequent RB for this immediately after this is committed? - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review89429 --- On July 1, 2015, 2:44 a.m., Aditya Auradkar wrote: --- 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 Rao. 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. - Addressed Joel's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- Addressing Joel's comments Merging Chaning variable name Addressing Joel's comments Diffs (updated) - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- 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. - Addressed Joel's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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/java/org/apache/kafka/clients/consumer/internals/Fetcher.java (line 341) https://reviews.apache.org/r/33378/#comment142040 `.record(response.getThrottleTime)` (since the fetch response has already been parsed). You can also get rid of the `THROTTLE_TIME_KEY_NAME` variable. clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java (line 395) https://reviews.apache.org/r/33378/#comment142031 fetchThrottleTimeSensor clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java (line 447) https://reviews.apache.org/r/33378/#comment142035 fetch-throttle-time-avg clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java (line 452) https://reviews.apache.org/r/33378/#comment142036 fetch-throttle-time-avg clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java (line 258) https://reviews.apache.org/r/33378/#comment142041 Similar comment: this is already parsed so you can do `response.getThrottleTime` and get rid of the key variable. clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java (line 358) https://reviews.apache.org/r/33378/#comment142032 produceThrottleTimeSensor clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java (line 389) https://reviews.apache.org/r/33378/#comment142033 produce-throttle-time-avg clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java (line 391) https://reviews.apache.org/r/33378/#comment142034 produce-throttle-time-max clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java (line 170) https://reviews.apache.org/r/33378/#comment142064 requests clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java (line 187) https://reviews.apache.org/r/33378/#comment142074 Rather than iterate over all the metrics can you create the MetricName directly and get from the metrics map? You may need to hard-code the tags and metricPrefix but I think that's okay. clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java (line 104) https://reviews.apache.org/r/33378/#comment142075 Same here core/src/main/scala/kafka/server/AbstractFetcherThread.scala (line 40) https://reviews.apache.org/r/33378/#comment142072 I think we should add throttle time metrics to the old producer and consumer as well. What do you think? - Joel Koshy On June 9, 2015, 5:10 p.m., Aditya Auradkar wrote: --- 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 Rao. 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. - Addressed Joel's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- Addressing Joel's comments Merging Diffs (updated) - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- 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. - Addressed Joel's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- Addressing Joel's comments Merging Chaning variable name Diffs (updated) - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- 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. - Addressed Joel's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 never throttled. Or this may be more relevant for your other patch. I've changed the names to throttle everywhere. I'll add a replica fetcher test to my other patch - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review86428 --- On June 9, 2015, 5:10 p.m., Aditya Auradkar wrote: --- 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 Rao. 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. - Addressed Joel's comments For now the patch will publish a zero delay and return a response Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 Rao. 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 Diffs (updated) - clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java 56281ee15cc33dfc96ff64d5b1e596047c7132a4 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 07e65d4a54ba4eef5b787eba3e71cbe9f6a920bd clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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 5b38f8554898e54800abd65a7415dd0ac41fd958 core/src/main/scala/kafka/api/FetchResponse.scala 0b6b33ab6f7a732ff1322b6f48abd4c43e0d7215 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 c16f7edd322709060e54c77eb505c44cbd77a4ec core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a 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
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 https://reviews.apache.org/r/33378/diff/5/?file=956942#file956942line143 follow-up Can you elaborate? On June 5, 2015, 2:43 a.m., Joel Koshy wrote: core/src/main/scala/kafka/server/DelayedFetch.scala, line 58 https://reviews.apache.org/r/33378/diff/5/?file=956947#file956947line58 This is slightly unwieldy. Perhaps we can hold this patch especially since this will be impacted by the main patch (KAFKA-2084) Sure, the plan is to commit this after the main patch. I can make this simpler if I added a class to represent the arguments for the callback but I don't think it will add a great deal of value On June 5, 2015, 2:43 a.m., Joel Koshy wrote: core/src/main/scala/kafka/api/ProducerResponse.scala, line 40 https://reviews.apache.org/r/33378/diff/5/?file=956944#file956944line40 We should do this based on the response version as well right? I gather the readFrom is only used on the client side. A client running this code will only send V1 style requests and will always get the throttleTime in return. This isn't the case for consumers because they will send an old version of the request from the ReplicaFetcherThread (if the intra.cluster.replication.protocol is old). - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review86428 --- On May 12, 2015, 9:42 p.m., Aditya Auradkar wrote: --- 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 Rao. 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 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 b2db91ca14bbd17fef5ce85839679144fff3f689 clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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/consumer/SimpleConsumer.scala 31a2639477bf66f9a05d2b9b07794572d7ec393b core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 417960dd1ab407ebebad8fdb0e97415db3e91a2f core/src/main/scala/kafka/server/OffsetManager.scala 18680ce100f10035175cc0263ba7787ab0f6a17a core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a
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 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 never throttled. Or this may be more relevant for your other patch. clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java https://reviews.apache.org/r/33378/#comment138801 How about recordThrottleTime? and replacing quotaDelay with throttle in other places as well? clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java https://reviews.apache.org/r/33378/#comment138802 Can drop this comment clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java https://reviews.apache.org/r/33378/#comment138803 May be slightly clearer: Duration in milliseconds for which the request was throttled due to quota violation. (Zero if the request did not violate any quota.) clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java https://reviews.apache.org/r/33378/#comment138804 same clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java https://reviews.apache.org/r/33378/#comment138810 requests core/src/main/scala/kafka/api/FetchResponse.scala https://reviews.apache.org/r/33378/#comment138811 follow-up core/src/main/scala/kafka/api/FetchResponse.scala https://reviews.apache.org/r/33378/#comment138813 `if (` core/src/main/scala/kafka/api/FetchResponse.scala https://reviews.apache.org/r/33378/#comment138812 ... from a client that send a v0 FetchRequest core/src/main/scala/kafka/api/FetchResponse.scala https://reviews.apache.org/r/33378/#comment138814 `if (` core/src/main/scala/kafka/api/ProducerResponse.scala https://reviews.apache.org/r/33378/#comment138815 We should do this based on the response version as well right? core/src/main/scala/kafka/api/ProducerResponse.scala https://reviews.apache.org/r/33378/#comment138816 `if (` core/src/main/scala/kafka/server/DelayedFetch.scala https://reviews.apache.org/r/33378/#comment138818 This is slightly unwieldy. Perhaps we can hold this patch especially since this will be impacted by the main patch (KAFKA-2084) core/src/main/scala/kafka/server/DelayedProduce.scala https://reviews.apache.org/r/33378/#comment138819 similar comment here core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala https://reviews.apache.org/r/33378/#comment138823 Why do we need this as part of this patch? - Joel Koshy On May 12, 2015, 9:42 p.m., Aditya Auradkar wrote: --- 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 Rao. 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 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 b2db91ca14bbd17fef5ce85839679144fff3f689 clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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
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 https://reviews.apache.org/r/33378/#comment134427 Will readFromLocalLog() read data into memory before the cilent's quota is checked? - Dong Lin On May 11, 2015, 9:51 p.m., Aditya Auradkar wrote: --- 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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description --- Fixing bug 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 b2db91ca14bbd17fef5ce85839679144fff3f689 clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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/consumer/SimpleConsumer.scala 31a2639477bf66f9a05d2b9b07794572d7ec393b core/src/main/scala/kafka/server/AbstractFetcherThread.scala a439046e118b6efcc3a5a9d9e8acb79f85e40398 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 417960dd1ab407ebebad8fdb0e97415db3e91a2f core/src/main/scala/kafka/server/OffsetManager.scala 18680ce100f10035175cc0263ba7787ab0f6a17a core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala f3ab3f4ff8eb1aa6b2ab87ba75f72eceb6649620 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- Minor bug fix Diffs (updated) - 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 b2db91ca14bbd17fef5ce85839679144fff3f689 clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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/consumer/SimpleConsumer.scala 31a2639477bf66f9a05d2b9b07794572d7ec393b core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 417960dd1ab407ebebad8fdb0e97415db3e91a2f core/src/main/scala/kafka/server/OffsetManager.scala 18680ce100f10035175cc0263ba7787ab0f6a17a core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala f3ab3f4ff8eb1aa6b2ab87ba75f72eceb6649620 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
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 https://reviews.apache.org/r/33378/diff/4/?file=955709#file955709line428 Will readFromLocalLog() read data into memory before the cilent's quota is checked? It shouldn't. I believe only FileMessageSet is passed around which only contains start and end pointers of file segments. The actual read is done later. - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review83439 --- On May 12, 2015, 9:40 p.m., Aditya Auradkar wrote: --- 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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description --- Minor bug fix 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 b2db91ca14bbd17fef5ce85839679144fff3f689 clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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/consumer/SimpleConsumer.scala 31a2639477bf66f9a05d2b9b07794572d7ec393b core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 417960dd1ab407ebebad8fdb0e97415db3e91a2f core/src/main/scala/kafka/server/OffsetManager.scala 18680ce100f10035175cc0263ba7787ab0f6a17a core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala f3ab3f4ff8eb1aa6b2ab87ba75f72eceb6649620 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- 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 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 b2db91ca14bbd17fef5ce85839679144fff3f689 clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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/consumer/SimpleConsumer.scala 31a2639477bf66f9a05d2b9b07794572d7ec393b core/src/main/scala/kafka/server/AbstractFetcherThread.scala 83fc47417dd7fe4edf030217fa7fd69d99b170b0 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 417960dd1ab407ebebad8fdb0e97415db3e91a2f core/src/main/scala/kafka/server/OffsetManager.scala 18680ce100f10035175cc0263ba7787ab0f6a17a core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala f3ab3f4ff8eb1aa6b2ab87ba75f72eceb6649620 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
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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- Fixing bug Diffs (updated) - 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 b2db91ca14bbd17fef5ce85839679144fff3f689 clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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/consumer/SimpleConsumer.scala 31a2639477bf66f9a05d2b9b07794572d7ec393b core/src/main/scala/kafka/server/AbstractFetcherThread.scala a439046e118b6efcc3a5a9d9e8acb79f85e40398 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 417960dd1ab407ebebad8fdb0e97415db3e91a2f core/src/main/scala/kafka/server/OffsetManager.scala 18680ce100f10035175cc0263ba7787ab0f6a17a core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala f3ab3f4ff8eb1aa6b2ab87ba75f72eceb6649620 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
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 https://reviews.apache.org/r/33378/#comment134302 Should delayTimeSize be deducted from expectedBytesToWrite? - Dong Lin On May 11, 2015, 9:51 p.m., Aditya Auradkar wrote: --- 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 Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description --- Fixing bug 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 b2db91ca14bbd17fef5ce85839679144fff3f689 clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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/consumer/SimpleConsumer.scala 31a2639477bf66f9a05d2b9b07794572d7ec393b core/src/main/scala/kafka/server/AbstractFetcherThread.scala a439046e118b6efcc3a5a9d9e8acb79f85e40398 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 417960dd1ab407ebebad8fdb0e97415db3e91a2f core/src/main/scala/kafka/server/OffsetManager.scala 18680ce100f10035175cc0263ba7787ab0f6a17a core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala f3ab3f4ff8eb1aa6b2ab87ba75f72eceb6649620 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
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. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- Changes are: - protocol changes to the fetch reuqest 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 Addressing Jun's comments Diffs (updated) - 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 b2db91ca14bbd17fef5ce85839679144fff3f689 clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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/consumer/SimpleConsumer.scala 31a2639477bf66f9a05d2b9b07794572d7ec393b core/src/main/scala/kafka/server/AbstractFetcherThread.scala a439046e118b6efcc3a5a9d9e8acb79f85e40398 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 417960dd1ab407ebebad8fdb0e97415db3e91a2f core/src/main/scala/kafka/server/OffsetManager.scala 18680ce100f10035175cc0263ba7787ab0f6a17a core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala f3ab3f4ff8eb1aa6b2ab87ba75f72eceb6649620 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
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 https://reviews.apache.org/r/33378/diff/1/?file=937083#file937083line152 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). Good point. I'm passing in the protocol version to use on the fetchRequest to the AbstractFetcherThread. Also, not reading the response based on the request version. - Aditya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33378/#review82389 --- On May 7, 2015, 1:36 a.m., Aditya Auradkar wrote: --- 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, Joel Koshy and Jun Rao. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description --- Changes are: - protocol changes to the fetch reuqest 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 Addressing Jun's comments Formatting changes 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 b2db91ca14bbd17fef5ce85839679144fff3f689 clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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/consumer/SimpleConsumer.scala 31a2639477bf66f9a05d2b9b07794572d7ec393b core/src/main/scala/kafka/server/AbstractFetcherThread.scala a439046e118b6efcc3a5a9d9e8acb79f85e40398 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 417960dd1ab407ebebad8fdb0e97415db3e91a2f core/src/main/scala/kafka/server/OffsetManager.scala 18680ce100f10035175cc0263ba7787ab0f6a17a core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala f3ab3f4ff8eb1aa6b2ab87ba75f72eceb6649620
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. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- Changes are: - protocol changes to the fetch reuqest 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 Addressing Jun's comments Formatting changes Diffs (updated) - 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 b2db91ca14bbd17fef5ce85839679144fff3f689 clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f 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/consumer/SimpleConsumer.scala 31a2639477bf66f9a05d2b9b07794572d7ec393b core/src/main/scala/kafka/server/AbstractFetcherThread.scala a439046e118b6efcc3a5a9d9e8acb79f85e40398 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 417960dd1ab407ebebad8fdb0e97415db3e91a2f core/src/main/scala/kafka/server/OffsetManager.scala 18680ce100f10035175cc0263ba7787ab0f6a17a core/src/main/scala/kafka/server/ReplicaFetcherThread.scala b31b432a226ba79546dd22ef1d2acbb439c2e9a3 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 5717165f2344823fabe8f7cfafae4bb8af2d949a core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala f3ab3f4ff8eb1aa6b2ab87ba75f72eceb6649620 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
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/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
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. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description --- Changes are: - protocol changes to the fetch reuqest 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 (updated) --- New tests added Thanks, Aditya Auradkar
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. Bugs: KAFKA-2136 https://issues.apache.org/jira/browse/KAFKA-2136 Repository: kafka Description (updated) --- 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
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/browse/KAFKA-2136 Repository: kafka Description --- Changes are: - protocol changes to the fetch reuqest 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 --- Thanks, Aditya Auradkar
