> On Oct. 5, 2015, 6:51 p.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java, > > line 427 > > <https://reviews.apache.org/r/36858/diff/14/?file=1086222#file1086222line427> > > > > One minor thing: could you remove this comment? > > Ismael Juma wrote: > I had asked for that to be removed too (you can see it in the > discussion), but Mayuresh said that some more work needed to be done before > it could be removed (I didn't quite understand it to be honest).
We have a ticket KAFKA-2428 which needs to be handled after KIP-19. The reason for that ticket is availble on KAFKA-2120 Jira. This was the brief discussion on the ticket KAFKA-2120: The request time is for exception handling purpose, the request timeout should be the highest timeout across all the timeout. Can we just sanity check the configurations user provides? The rules should be: Request timeout > session timeout. Request timeout > fetch.max.wait.timeout request timeout won't kick in before the other timeout reached. The sanity check patch should be relatively simple. We can either do it in this patch or in a separate patch. I created KAFKA-2428 to handle this. I have explained about discrepency of long/int datatype of requestTimeout in kafkaConsumer and sanity checks on Ismael's comments above. Probably I should have listed the Jira ticket there for the sanity checks which would have made this more clear. The TODO was left as it is to remind us that there is still work needed to be done after KIP-19 for KafkaConsumer on usage of requestTimeout. - Mayuresh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36858/#review101511 ----------------------------------------------------------- On Sept. 28, 2015, 11:16 p.m., Mayuresh Gharat wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36858/ > ----------------------------------------------------------- > > (Updated Sept. 28, 2015, 11:16 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2120 > https://issues.apache.org/jira/browse/KAFKA-2120 > > > Repository: kafka > > > Description > ------- > > Solved compile error > > > Addressed Jason's comments for Kip-19 > > > Addressed Jun's comments > > > Addressed Jason's comments about the default values for requestTimeout > > > checkpoint > > > Addressed Joel's concerns. Also tried to include Jun's feedback. > > > Fixed a minor comment > > > Solved unittest issue > > > Addressed Jun's comments regarding NetworkClient > > > Addressed Jun's comments about disconnect() in Selector > > > changed logging level to debug > > > Addressed Joels comments to break out early from the loop while aborting > expired batches > > > Addressed Jun's comments > > > Addressed Jason's concern about iterating over timeout request in > getNodesWithTimedOutRequest() > > > Addressed Joel's comments > > > Changed the default timeouts for requesttimeout in KafkaConfig and also made > requestrtimeout consistent in KafkaConsumer > > > Fixed the forward referencing bug in KafkaConfig. > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/ClientRequest.java > dc8f0f115bcda893c95d17c0a57be8d14518d034 > clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.java > 7d24c6f5dd2b63b96584f3aa8922a1d048dc1ae4 > clients/src/main/java/org/apache/kafka/clients/InFlightRequests.java > 15d00d4e484bb5d51a9ae6857ed6e024a2cc1820 > clients/src/main/java/org/apache/kafka/clients/KafkaClient.java > f46c0d9b5eb73887c62a0e09c96e9d8c964c709d > clients/src/main/java/org/apache/kafka/clients/NetworkClient.java > 51a6c5dc47c4008e92b9fdd2ee359b573184e8ed > clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java > b9a2d4e2bc565f0ee72b27791afe5c894af262f1 > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java > 3ac2be840c5d313a497ab67dc024d1578d4679fd > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkClient.java > 9517d9d0cd480d5ba1d12f1fde7963e60528d2f8 > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > 804d569498396d431880641041fc9292076452cb > clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java > 06f00a99a73a288df9afa8c1d4abe3580fa968a6 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java > 4cb1e50d6c4ed55241aeaef1d3af09def5274103 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java > a152bd7697dca55609a9ec4cfe0a82c10595fbc3 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java > 06182db1c3a5da85648199b4c0c98b80ea7c6c0c > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > d2e64f7cd8bf56e433a210905b2874f71eee9ea0 > clients/src/main/java/org/apache/kafka/common/network/Selectable.java > 70e74bd6aa629c430b2850ca40c97df0b16e5d75 > clients/src/main/java/org/apache/kafka/common/network/Selector.java > 5a4909e4e5afe1b7ce2c33b9324d3947d2d91a38 > clients/src/test/java/org/apache/kafka/clients/MockClient.java > e5815f56bdf8e2d980f2bc36b831ed234c0ac781 > clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java > 69c93c3adf674b1640534c3d7410fcaafaf2232c > > clients/src/test/java/org/apache/kafka/clients/producer/internals/BufferPoolTest.java > 2c693824fa53db1e38766b8c66a0ef42ef9d0f3a > > clients/src/test/java/org/apache/kafka/clients/producer/internals/RecordAccumulatorTest.java > 5b2e4ffaeab7127648db608c179703b27b577414 > > clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java > aa44991777a855f4b7f4f7bf17107c69393ff8ff > clients/src/test/java/org/apache/kafka/common/network/SSLSelectorTest.java > df1205c935bee9a30a50816dbade64d6014b1ef2 > clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java > 3a684d98b05cadfb25c6f7f9a038ef1f6697edbf > clients/src/test/java/org/apache/kafka/test/MockSelector.java > f83fd9b794a3bd191121a22bcb40fd6ec31d83b5 > core/src/main/scala/kafka/controller/ControllerChannelManager.scala > b1cf668dbb5bc8617d320fd3a307c6c8005b78a2 > core/src/main/scala/kafka/server/KafkaConfig.scala > 1e8b2331486ffe55bfcc0919e48e12aad23b7d3c > core/src/main/scala/kafka/server/KafkaServer.scala > f3f1fa653b9c85e729cbe32b63669011ada46547 > core/src/main/scala/kafka/server/ReplicaFetcherThread.scala > 6c85e52e172bfc0278e724068a48a271bc15dc9f > core/src/main/scala/kafka/tools/ProducerPerformance.scala > 46a68e97b8bcc8821f21e4220ce9b3acedc5dafe > core/src/main/scala/kafka/utils/NetworkClientBlockingOps.scala > ad10721de844725f27a116611209992cea61b088 > core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala > 1198df02ddd7727269e84a751ba99520f6d5584a > core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala > 5b4f2db4607ae6d17696c1140f1a771ce75c80e0 > core/src/test/scala/unit/kafka/utils/TestUtils.scala > 09b8444c2add87f0f70dbb182e892977a6b5c243 > > Diff: https://reviews.apache.org/r/36858/diff/ > > > Testing > ------- > > Unit test pass > > > Thanks, > > Mayuresh Gharat > >
