Re: Review Request 35867: Patch for KAFKA-1901
On Aug. 20, 2015, 2:03 a.m., Joel Koshy wrote: build.gradle, line 389 https://reviews.apache.org/r/35867/diff/5/?file=1035356#file1035356line389 This gives an error in detached mode (i.e., not on any branch). Updated code to handle detached mode. thanks for the review. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/#review95902 --- On Aug. 20, 2015, 7:08 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/ --- (Updated Aug. 20, 2015, 7:08 a.m.) Review request for kafka. Bugs: KAFKA-1901 https://issues.apache.org/jira/browse/KAFKA-1901 Repository: kafka Description --- Addresing Joel's comments, rebase Diffs - build.gradle 17fc223907f6b55f2d730be81ddb8d8e07a2c7ad clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 3749880b765f74af117d6c44705daf170095a1b7 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java c4621e22c32c1a1fb23726d7f56004845def96ef clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c core/src/main/scala/kafka/server/KafkaServer.scala 0e7ba3ede78dbc995c404e0387a6be687703836a core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343 Diff: https://reviews.apache.org/r/35867/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35867: Patch for KAFKA-1901
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/ --- (Updated Aug. 20, 2015, 7:08 a.m.) Review request for kafka. Bugs: KAFKA-1901 https://issues.apache.org/jira/browse/KAFKA-1901 Repository: kafka Description (updated) --- Addresing Joel's comments, rebase Diffs (updated) - build.gradle 17fc223907f6b55f2d730be81ddb8d8e07a2c7ad clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 3749880b765f74af117d6c44705daf170095a1b7 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java c4621e22c32c1a1fb23726d7f56004845def96ef clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c core/src/main/scala/kafka/server/KafkaServer.scala 0e7ba3ede78dbc995c404e0387a6be687703836a core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343 Diff: https://reviews.apache.org/r/35867/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 36578: Patch for KAFKA-2338
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36578/#review95943 --- core/src/main/scala/kafka/admin/TopicCommand.scala (line 89) https://reviews.apache.org/r/36578/#comment151144 The warning message can be : WARNING: %s has been increased beyond the default max value of %d, update producer and consumer settings as well Also do want to include patch for second point given in JIRA decription.. - Manikumar Reddy O On July 21, 2015, 4:21 p.m., Edward Ribeiro wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36578/ --- (Updated July 21, 2015, 4:21 p.m.) Review request for kafka. Bugs: KAFKA-2338 https://issues.apache.org/jira/browse/KAFKA-2338 Repository: kafka Description --- KAFKA-2338 Warn users if they change max.message.bytes that they also need to update broker and consumer settings Diffs - core/src/main/scala/kafka/admin/TopicCommand.scala 4e28bf1c08414e8e96e6ca639b927d51bfeb4616 Diff: https://reviews.apache.org/r/36578/diff/ Testing --- Thanks, Edward Ribeiro
Re: Review Request 36578: Patch for KAFKA-2338
On Aug. 20, 2015, 11:07 a.m., Edward Ribeiro wrote: Also do want to include patch for second point given in JIRA decription.. just read previous reviews..ignore my comment. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36578/#review95943 --- On July 21, 2015, 4:21 p.m., Edward Ribeiro wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36578/ --- (Updated July 21, 2015, 4:21 p.m.) Review request for kafka. Bugs: KAFKA-2338 https://issues.apache.org/jira/browse/KAFKA-2338 Repository: kafka Description --- KAFKA-2338 Warn users if they change max.message.bytes that they also need to update broker and consumer settings Diffs - core/src/main/scala/kafka/admin/TopicCommand.scala 4e28bf1c08414e8e96e6ca639b927d51bfeb4616 Diff: https://reviews.apache.org/r/36578/diff/ Testing --- Thanks, Edward Ribeiro
Re: Review Request 34805: Patch for KAFKA-2213
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34805/ --- (Updated Aug. 20, 2015, 11:37 a.m.) Review request for kafka. Bugs: KAFKA-2213 https://issues.apache.org/jira/browse/KAFKA-2213 Repository: kafka Description (updated) --- Rebase Diffs (updated) - clients/src/main/java/org/apache/kafka/common/record/Compressor.java e570b29d5ffba5d3754c46670b708f7d511086f3 clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 5f1b45c2970e4de53bd04595afe8486b5ec49e5d core/src/main/scala/kafka/log/LogCleaner.scala b36ea0dd7f954c2a0eb3270535fc9a8df7488d59 core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 70beb5f71204ae9386484f6a559e0e7138497483 Diff: https://reviews.apache.org/r/34805/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35880: Patch for KAFKA-2295
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35880/ --- (Updated Aug. 20, 2015, 12:17 p.m.) Review request for kafka. Bugs: KAFKA-2295 https://issues.apache.org/jira/browse/KAFKA-2295 Repository: kafka Description (updated) --- Code Rebase Diffs (updated) - clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 156ec14c9c099448cc1d8347191eef4c02591faa clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527 clients/src/main/java/org/apache/kafka/common/utils/Utils.java c58b74144ba01f04ec17d8ca7bd3d231fd6fb3a8 clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java db1b0ee9113215b5ad7fda0f93915f3bdd34ac55 core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09 Diff: https://reviews.apache.org/r/35880/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35867: Patch for KAFKA-1901
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/ --- (Updated Aug. 9, 2015, 9:37 a.m.) Review request for kafka. Bugs: KAFKA-1901 https://issues.apache.org/jira/browse/KAFKA-1901 Repository: kafka Description --- Addresing Joel's comments Diffs (updated) - build.gradle 1b67e628c2fca897177c12b6afad9a8700fffd1f clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java ed99e9bdf7c4ea7a6d4555d4488cf8ed0b80641b clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7 clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c core/src/main/scala/kafka/server/KafkaServer.scala 84d4730ac634f9a5bf12a656e422fea03ad72da8 core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343 Diff: https://reviews.apache.org/r/35867/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35867: Patch for KAFKA-1901
On July 21, 2015, 1:58 p.m., Ismael Juma wrote: clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java, line 27 https://reviews.apache.org/r/35867/diff/4/?file=1011317#file1011317line27 Why isn't this unknown like `version`? yes we can set unknown here. Joel suggested to use as the unknown commit ID. Joel, can we change this? - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/#review92404 --- On Aug. 9, 2015, 9:37 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/ --- (Updated Aug. 9, 2015, 9:37 a.m.) Review request for kafka. Bugs: KAFKA-1901 https://issues.apache.org/jira/browse/KAFKA-1901 Repository: kafka Description --- Addresing Joel's comments Diffs - build.gradle 1b67e628c2fca897177c12b6afad9a8700fffd1f clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java ed99e9bdf7c4ea7a6d4555d4488cf8ed0b80641b clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7 clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c core/src/main/scala/kafka/server/KafkaServer.scala 84d4730ac634f9a5bf12a656e422fea03ad72da8 core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343 Diff: https://reviews.apache.org/r/35867/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34641: Patch for KAFKA-2214
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/ --- (Updated Aug. 5, 2015, 3:19 p.m.) Review request for kafka. Bugs: KAFKA-2214 https://issues.apache.org/jira/browse/KAFKA-2214 Repository: kafka Description (updated) --- Addresing ismail juma's comments Diffs (updated) - core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala ea345895a52977c25bff57e95e12b8662331d7fe Diff: https://reviews.apache.org/r/34641/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34641: Patch for KAFKA-2214
On July 21, 2015, 1:50 p.m., Ismael Juma wrote: Thanks for the suggestions. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/#review92403 --- On Aug. 5, 2015, 3:19 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/ --- (Updated Aug. 5, 2015, 3:19 p.m.) Review request for kafka. Bugs: KAFKA-2214 https://issues.apache.org/jira/browse/KAFKA-2214 Repository: kafka Description --- Addresing ismail juma's comments Diffs - core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala ea345895a52977c25bff57e95e12b8662331d7fe Diff: https://reviews.apache.org/r/34641/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34641: Patch for KAFKA-2214
On July 17, 2015, 5:03 a.m., Gwen Shapira wrote: Just realized that verifyAssigment runs on a Map of Partition, Status and originally printed status for each partition (some can succeed, fail or be in-progress). We need to return a single error code. In the existing code, we return 0 if all partitions were successfull, 1 if one or more failed, 2 if one of more is in progress. If some are in-progress and some are failed, it seems like the return value is either 1 or 2, at random (since it depends on order of foreach on a Map). I'm not sure thats expected :) Perhaps we can come up with something better defined? Actually return value is not random. If some are in-progress and some are failed, we are returning failed error code. Re-asginment failed status is given more preference than in-progress error status. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/#review92024 --- On Aug. 5, 2015, 3:19 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/ --- (Updated Aug. 5, 2015, 3:19 p.m.) Review request for kafka. Bugs: KAFKA-2214 https://issues.apache.org/jira/browse/KAFKA-2214 Repository: kafka Description --- Addresing ismail juma's comments Diffs - core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala ea345895a52977c25bff57e95e12b8662331d7fe Diff: https://reviews.apache.org/r/34641/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34641: Patch for KAFKA-2214
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/ --- (Updated July 14, 2015, 10:03 a.m.) Review request for kafka. Bugs: KAFKA-2214 https://issues.apache.org/jira/browse/KAFKA-2214 Repository: kafka Description --- Addressing Gwen's comments Diffs (updated) - core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala ea345895a52977c25bff57e95e12b8662331d7fe Diff: https://reviews.apache.org/r/34641/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34641: Patch for KAFKA-2214
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/ --- (Updated July 14, 2015, 10:13 a.m.) Review request for kafka. Bugs: KAFKA-2214 https://issues.apache.org/jira/browse/KAFKA-2214 Repository: kafka Description --- Addressing Gwen's comments Diffs (updated) - core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala ea345895a52977c25bff57e95e12b8662331d7fe Diff: https://reviews.apache.org/r/34641/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34641: Patch for KAFKA-2214
On May 26, 2015, 7:08 a.m., Michael Noll wrote: core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala, line 81 https://reviews.apache.org/r/34641/diff/1/?file=971195#file971195line81 Should we also consider reassignments that are in-progress as errors? The reasoning is that you'd like to do the following in (say) Ansible: Trigger reassignment of partitions, wait until all are completed, and only then continue with the next action. That being said, there are other ways to achieve this in tools like Ansible. For instance, you can trigger reassignents, then repeatedly call `--verify` in a loop and capture its STDOUT, looking for is still in progress and failed. However, this is arguably more error prone because the log messages can change between Kafka versions (and oftentimes such changes are not prominently advertised, so you only notice this once your deployment script breaks). Manikumar Reddy O wrote: Yes, we can consider in-progress as errors. Other option could be returning a different error code. Let us wait for others suggestions/concerns. Gwen Shapira wrote: I wouldn't consider in-progress as error, since in-progress is pretty much the expected behavior here (reassigning takes a while and the tool exits immediately). As you mentioned, the ansible script should include triggering re-assignment and then polling periodically until its completed. (like you would for HDFS rebalance, Oozie job, MR job, etc) Perhaps just commenting on --verify output that this is a public API and should never change would be ok? Gwen Shapira wrote: Sorry, I got confused. You meant - triggering a new re-assignment while the old one is still in progress. I agree this should be an error. (but I still think the ansible script should use --verify, otherwise you risk triggering a new re-assignment accidentally) difeerent exitCode is given for in-progress reassignments. exitCode = 0 for ReassignmentCompleted exitCode = 1 for ReassignmentInProgress exitCode = 2 for ReassignmentFailed - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/#review85154 --- On July 14, 2015, 10:03 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/ --- (Updated July 14, 2015, 10:03 a.m.) Review request for kafka. Bugs: KAFKA-2214 https://issues.apache.org/jira/browse/KAFKA-2214 Repository: kafka Description --- Addressing Gwen's comments Diffs - core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala ea345895a52977c25bff57e95e12b8662331d7fe Diff: https://reviews.apache.org/r/34641/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35867: Patch for KAFKA-1901
On July 14, 2015, 1:59 a.m., Joel Koshy wrote: core/src/main/scala/kafka/common/AppInfo.scala, line 27 https://reviews.apache.org/r/35867/diff/3/?file=1004947#file1004947line27 Per the comment in the previous diff, I think this can go now right? i.e., kafka server depends on clients so if you browse mbeans you will see two app-infos registered (under `kafka.server` and `kafka.common`) which is weird. The server will also expose app-info via the clients package since it already uses kafka metrics and the associated jmx reporter. Old AppInfo registration is removed. Now on Kafka Server, we will only see one app-info mbean. On July 14, 2015, 1:59 a.m., Joel Koshy wrote: clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java, line 27 https://reviews.apache.org/r/35867/diff/3/?file=1004944#file1004944line27 (For consistency) should we make this 40-char wide as is a standard commit id? Or we can just go with a eight-char or 16-char wide id for both this and the actual commit id. Ok..I am going with 16 char id. On July 14, 2015, 1:59 a.m., Joel Koshy wrote: clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java, line 246 https://reviews.apache.org/r/35867/diff/3/?file=1004943#file1004943line246 In https://issues.apache.org/jira/browse/KAFKA-1901?focusedCommentId=14294803page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14294803 I was wondering if we could have a commit fingerprint - i.e., the long value of the most-significant eight bytes of the commit modulo 10k or something like that. This would make is convenient to register as a measurable `KafkaMetric` that people can then use in their deployment monitoring. i.e., instantly look at a graph and say whether all brokers/clients are running the same version or not. Implemented a simple finger print (commitId.hashcode()) mechanism. I felt commit.hashCode() should be sufficient for our requirement. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/#review91574 --- On July 14, 2015, 12:32 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/ --- (Updated July 14, 2015, 12:32 p.m.) Review request for kafka. Bugs: KAFKA-1901 https://issues.apache.org/jira/browse/KAFKA-1901 Repository: kafka Description --- Addresing Joel's comments Diffs - build.gradle d86f1a8b25197d53f11e16c54a6854487e175649 clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java b4e8f7f0dceefaf94a3495f39f5783cce5ceb25f clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7 clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 6b9590c418aedd2727544c5dd23c017b4b72467a clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 07b1b60d3a9cb1a399a2fe95b87229f64f539f3b clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 544e120594de78c43581a980b1e4087b4fb98ccb core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343 Diff: https://reviews.apache.org/r/35867/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35867: Patch for KAFKA-1901
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/ --- (Updated July 14, 2015, 12:32 p.m.) Review request for kafka. Bugs: KAFKA-1901 https://issues.apache.org/jira/browse/KAFKA-1901 Repository: kafka Description (updated) --- Addresing Joel's comments Diffs (updated) - build.gradle d86f1a8b25197d53f11e16c54a6854487e175649 clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java b4e8f7f0dceefaf94a3495f39f5783cce5ceb25f clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7 clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 6b9590c418aedd2727544c5dd23c017b4b72467a clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 07b1b60d3a9cb1a399a2fe95b87229f64f539f3b clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 544e120594de78c43581a980b1e4087b4fb98ccb core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343 Diff: https://reviews.apache.org/r/35867/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34641: Patch for KAFKA-2214
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/ --- (Updated July 13, 2015, 3:43 p.m.) Review request for kafka. Bugs: KAFKA-2214 https://issues.apache.org/jira/browse/KAFKA-2214 Repository: kafka Description (updated) --- Addressing Gwen's comments Diffs (updated) - core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala ea345895a52977c25bff57e95e12b8662331d7fe Diff: https://reviews.apache.org/r/34641/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34403: Patch for KAFKA-2198
On July 10, 2015, 4:46 p.m., Gwen Shapira wrote: core/src/main/scala/kafka/admin/TopicCommand.scala, lines 72-73 https://reviews.apache.org/r/34403/diff/4/?file=1008271#file1008271line72 This is a bit unclean. I think its more idiomatic when the catch block includes the System.exit(1). Also, I'm afraid that printing the entire stack trace is intimidating to non-developers who use the CLI. Perhaps the stack trace should go under log.error(...)? Manikumar Reddy O wrote: Calling System.exit(1) in catch block results unexecuted finally block. http://stackoverflow.com/questions/1410951/how-does-javas-system-exit-work-with-try-catch-finally-blocks http://stackoverflow.com/questions/15264076/regarding-excuting-finally-block-in-system-exit-case-also-by-adding-shutdownhook?lq=1 log.error() used for printing stackTrace. Gwen Shapira wrote: Good point. The problem is that the code here is very explicit about what we do when an exception occured, but doesn't show what we do when an exception doesn't occure. Putting if ... else ... at this point duplicates the try ... catch... logic right above it. How about modifying these lines to System.exit(exitCode) in the finally clause and setting the value of exitCode in the try and catch clauses? This will also allow us to support multiple exit codes cleanly in the future. Thank for the suggestion. Uploaded a new patch with relevant chnages. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34403/#review91310 --- On July 13, 2015, 1:57 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34403/ --- (Updated July 13, 2015, 1:57 p.m.) Review request for kafka. Bugs: KAFKA-2198 https://issues.apache.org/jira/browse/KAFKA-2198 Repository: kafka Description --- Addressing Gwen's comments Diffs - core/src/main/scala/kafka/admin/TopicCommand.scala a2ecb9620d647bf8f957a1f00f52896438e804a7 Diff: https://reviews.apache.org/r/34403/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35867: Patch for KAFKA-1901
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/ --- (Updated July 10, 2015, 11:15 a.m.) Review request for kafka. Bugs: KAFKA-1901 https://issues.apache.org/jira/browse/KAFKA-1901 Repository: kafka Description (updated) --- patch after rebase Diffs (updated) - build.gradle d86f1a8b25197d53f11e16c54a6854487e175649 clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 7aa076084c894bb8f47b9df2c086475b06f47060 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7 clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 6b9590c418aedd2727544c5dd23c017b4b72467a clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 07b1b60d3a9cb1a399a2fe95b87229f64f539f3b clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 544e120594de78c43581a980b1e4087b4fb98ccb core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c core/src/main/scala/kafka/server/KafkaServer.scala 18917bc4464b9403b16d85d20c3fd4c24893d1d3 Diff: https://reviews.apache.org/r/35867/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35454: Patch for KAFKA-2159
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35454/ --- (Updated July 10, 2015, 3:47 p.m.) Review request for kafka. Bugs: KAFKA-2159 https://issues.apache.org/jira/browse/KAFKA-2159 Repository: kafka Description (updated) --- rebase Diffs (updated) - core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala 6c2df4c4f9d26d4091f480b64c063574837fd941 Diff: https://reviews.apache.org/r/35454/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35880: Patch for KAFKA-2295
On July 6, 2015, 3:50 p.m., Guozhang Wang wrote: clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java, line 57 https://reviews.apache.org/r/35880/diff/2/?file=999580#file999580line57 Is this intentional? yes. Now ClassNotFoundException will be thrown from Utils.newInstance() as KafkaException. On July 6, 2015, 3:50 p.m., Guozhang Wang wrote: clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java, line 266 https://reviews.apache.org/r/35880/diff/2/?file=999578#file999578line266 Can we use Utils.newInstance here as well? Here we only need Class Object. On July 6, 2015, 3:50 p.m., Guozhang Wang wrote: core/src/main/scala/kafka/utils/CoreUtils.scala, line 221 https://reviews.apache.org/r/35880/diff/2/?file=999581#file999581line221 Can we use scala's Utils.createObject here? Are you sugessting to use Utils.newInstance()? We dont have scala's Utils.scala. Am I missing something? - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35880/#review90488 --- On July 6, 2015, 6:05 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35880/ --- (Updated July 6, 2015, 6:05 a.m.) Review request for kafka. Bugs: KAFKA-2295 https://issues.apache.org/jira/browse/KAFKA-2295 Repository: kafka Description --- Addessing Guozhang's comments Diffs - clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527 clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68 clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java db1b0ee9113215b5ad7fda0f93915f3bdd34ac55 core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09 Diff: https://reviews.apache.org/r/35880/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34805: Patch for KAFKA-2213
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34805/#review91298 --- Joel, need your inputs for proceeding further on this. - Manikumar Reddy O On July 10, 2015, 2:50 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34805/ --- (Updated July 10, 2015, 2:50 p.m.) Review request for kafka. Bugs: KAFKA-2213 https://issues.apache.org/jira/browse/KAFKA-2213 Repository: kafka Description --- rebase Diffs - clients/src/main/java/org/apache/kafka/common/record/Compressor.java e570b29d5ffba5d3754c46670b708f7d511086f3 clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 5f1b45c2970e4de53bd04595afe8486b5ec49e5d core/src/main/scala/kafka/log/LogCleaner.scala b36ea0dd7f954c2a0eb3270535fc9a8df7488d59 core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 381e9aa6fe611887cd144b4731f9ac351d12bbf1 Diff: https://reviews.apache.org/r/34805/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34805: Patch for KAFKA-2213
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34805/ --- (Updated July 10, 2015, 2:50 p.m.) Review request for kafka. Bugs: KAFKA-2213 https://issues.apache.org/jira/browse/KAFKA-2213 Repository: kafka Description (updated) --- rebase Diffs (updated) - clients/src/main/java/org/apache/kafka/common/record/Compressor.java e570b29d5ffba5d3754c46670b708f7d511086f3 clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java 5f1b45c2970e4de53bd04595afe8486b5ec49e5d core/src/main/scala/kafka/log/LogCleaner.scala b36ea0dd7f954c2a0eb3270535fc9a8df7488d59 core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 381e9aa6fe611887cd144b4731f9ac351d12bbf1 Diff: https://reviews.apache.org/r/34805/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34403: Patch for KAFKA-2198
On July 10, 2015, 4:46 p.m., Gwen Shapira wrote: core/src/main/scala/kafka/admin/TopicCommand.scala, lines 72-73 https://reviews.apache.org/r/34403/diff/4/?file=1008271#file1008271line72 This is a bit unclean. I think its more idiomatic when the catch block includes the System.exit(1). Also, I'm afraid that printing the entire stack trace is intimidating to non-developers who use the CLI. Perhaps the stack trace should go under log.error(...)? Calling System.exit(1) in catch block results unexecuted finally block. http://stackoverflow.com/questions/1410951/how-does-javas-system-exit-work-with-try-catch-finally-blocks http://stackoverflow.com/questions/15264076/regarding-excuting-finally-block-in-system-exit-case-also-by-adding-shutdownhook?lq=1 log.error() used for printing stackTrace. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34403/#review91310 --- On July 10, 2015, 5:44 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34403/ --- (Updated July 10, 2015, 5:44 p.m.) Review request for kafka. Bugs: KAFKA-2198 https://issues.apache.org/jira/browse/KAFKA-2198 Repository: kafka Description --- Addressing Gwen's comments Diffs - core/src/main/scala/kafka/admin/TopicCommand.scala a2ecb9620d647bf8f957a1f00f52896438e804a7 Diff: https://reviews.apache.org/r/34403/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34641: Patch for KAFKA-2214
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/ --- (Updated July 10, 2015, 4:28 p.m.) Review request for kafka. Bugs: KAFKA-2214 https://issues.apache.org/jira/browse/KAFKA-2214 Repository: kafka Description (updated) --- rebase Diffs (updated) - core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala ea345895a52977c25bff57e95e12b8662331d7fe Diff: https://reviews.apache.org/r/34641/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34403: Patch for KAFKA-2198
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34403/ --- (Updated July 10, 2015, 4:34 p.m.) Review request for kafka. Bugs: KAFKA-2198 https://issues.apache.org/jira/browse/KAFKA-2198 Repository: kafka Description (updated) --- rebase Diffs (updated) - core/src/main/scala/kafka/admin/TopicCommand.scala a2ecb9620d647bf8f957a1f00f52896438e804a7 Diff: https://reviews.apache.org/r/34403/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35880: Patch for KAFKA-2295
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35880/ --- (Updated July 6, 2015, 6:05 a.m.) Review request for kafka. Bugs: KAFKA-2295 https://issues.apache.org/jira/browse/KAFKA-2295 Repository: kafka Description (updated) --- Addessing Guozhang's comments Diffs (updated) - clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527 clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68 clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java db1b0ee9113215b5ad7fda0f93915f3bdd34ac55 core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09 Diff: https://reviews.apache.org/r/35880/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35880: Patch for KAFKA-2295
On July 3, 2015, 1:46 a.m., Guozhang Wang wrote: clients/src/main/java/org/apache/kafka/common/utils/Utils.java, line 512 https://reviews.apache.org/r/35880/diff/1/?file=992171#file992171line512 I think thread.getContextClassLoader() will never return null: http://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html#getContextClassLoader() I agree. javadoc says Returns: the context ClassLoader for this Thread, or null indicating the system class loader (or, failing that, the bootstrap class loader). I added this code for completeness. On July 3, 2015, 1:46 a.m., Guozhang Wang wrote: clients/src/main/java/org/apache/kafka/common/utils/Utils.java, line 496 https://reviews.apache.org/r/35880/diff/1/?file=992171#file992171line496 How about add a createObject function in Utils.java like coreUtils.scala and let other java code calls it? we already have Utils.newInstance() method. If required, we can add createObject() method. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35880/#review90305 --- On June 25, 2015, 4:25 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35880/ --- (Updated June 25, 2015, 4:25 p.m.) Review request for kafka. Bugs: KAFKA-2295 https://issues.apache.org/jira/browse/KAFKA-2295 Repository: kafka Description --- Implemented the approach given in JIRA description. This is to support dynamically loaded envirornments Diffs - clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527 clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68 core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09 Diff: https://reviews.apache.org/r/35880/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35867: Patch for KAFKA-1901
On June 25, 2015, 7:01 p.m., Joel Koshy wrote: build.gradle, line 57 https://reviews.apache.org/r/35867/diff/1/?file=991942#file991942line57 Why was this change made? This is related to followup patch of KAFKA-2199. Will remove after KAFKA-2199 checkin. On June 25, 2015, 7:01 p.m., Joel Koshy wrote: build.gradle, line 373 https://reviews.apache.org/r/35867/diff/1/?file=991942#file991942line373 Isn't it sufficient to just get the hash from the filesystem (https://gist.github.com/JonasGroeger/7620911) instead of git log? Why do we need both approaches? yes, it should be sufficient. I just felt using git cmd is more cleaner than reading from filesystem. BTW, these gradle tasks are borrowed from gradle project itself. https://github.com/gradle/gradle/blob/master/gradle/buildReceipt.gradle On June 25, 2015, 7:01 p.m., Joel Koshy wrote: core/src/main/scala/kafka/common/AppInfo.scala, line 17 https://reviews.apache.org/r/35867/diff/1/?file=991949#file991949line17 Do we need this AppInfo anymore? KafkaServer now initializes JmxReporter - so if you browse your mbeans you will see two AppInfo mbeans - one under kafka.common and another under kafka That said, I commented further above that AppInfo should probably be directly instantiated and registered separately from JmxReporter. Curently AppInfo is also used for old producer and consumer. We can remove the AppInfo usage from KafkaServer. But, I think we can retain this till we migrate kafka server to new kafka metrics. On June 25, 2015, 7:01 p.m., Joel Koshy wrote: clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java, line 58 https://reviews.apache.org/r/35867/diff/1/?file=991945#file991945line58 Are you concerned about collisions with multiple clients in the same VM? yes, this will help us to uniquely identify/register/de-register these Mbeans On June 25, 2015, 7:01 p.m., Joel Koshy wrote: clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java, line 146 https://reviews.apache.org/r/35867/diff/1/?file=991945#file991945line146 Actually, why are we doing this in JmxReporter? Can we just instantiate an AppInfo mbean directly in the producer/consumer/server? Yes, we can instantiate directly. I dont want to duplicate the code. JmxReporter is getting instantiated in producer,consumer and server and also metrics.close() will deregister these mbeans. I can move these utility methods (registerAppInfo, unregisterAppInfo) to Utils.java and call directly from KafkaProducer,Consumer and Server. Let me know your preference. i will change accordingly. On June 25, 2015, 7:01 p.m., Joel Koshy wrote: build.gradle, line 386 https://reviews.apache.org/r/35867/diff/1/?file=991942#file991942line386 I was originally interested in this because it would be a quick way to determine what version someone is running/testing with. However, it is obviously not foolproof since you could have local changes that are staged/unstaged but not committed. This is probably good enough, but can you think about whether it is possible to easily add a tainted boolean field? i.e., if there are any additional source files that are untracked or staged but not committed? It should be posible with some git commands. But do we really need this? most of us will be running stable release or some trunk point. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/#review89379 --- On June 25, 2015, 10:11 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/ --- (Updated June 25, 2015, 10:11 a.m.) Review request for kafka. Bugs: KAFKA-1901 https://issues.apache.org/jira/browse/KAFKA-1901 Repository: kafka Description --- generated a properties file (kafka-version.properties) with version, git-commitId in it. This props file is used to create AppInfo MBean. Diffs - build.gradle 30d1cf2f1ff9ed3f86a060da8099bb0774b4cf91 clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 9be8fbc648369ad9db1a7eea94bc1b9edbfdbfd7 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 5671a3fbeea8cb9a9ffeeb41aa1b132b92c0cae8 clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 6b9590c418aedd2727544c5dd23c017b4b72467a clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 07b1b60d3a9cb1a399a2fe95b87229f64f539f3b clients
Re: Review Request 35867: Patch for KAFKA-1901
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/ --- (Updated June 26, 2015, 7:49 a.m.) Review request for kafka. Bugs: KAFKA-1901 https://issues.apache.org/jira/browse/KAFKA-1901 Repository: kafka Description (updated) --- Addresing Joel's comments Diffs (updated) - build.gradle 30d1cf2f1ff9ed3f86a060da8099bb0774b4cf91 clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 9be8fbc648369ad9db1a7eea94bc1b9edbfdbfd7 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 5671a3fbeea8cb9a9ffeeb41aa1b132b92c0cae8 clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 6b9590c418aedd2727544c5dd23c017b4b72467a clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 07b1b60d3a9cb1a399a2fe95b87229f64f539f3b clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 544e120594de78c43581a980b1e4087b4fb98ccb core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c core/src/main/scala/kafka/server/KafkaServer.scala 52dc728bb1ab4b05e94dc528da1006040e2f28c9 Diff: https://reviews.apache.org/r/35867/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 35867: Patch for KAFKA-1901
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/ --- Review request for kafka. Bugs: KAFKA-1901 https://issues.apache.org/jira/browse/KAFKA-1901 Repository: kafka Description --- generated a properties file (kafka-version.properties) with version, git-commitId in it. This props file is used to create AppInfo MBean. Diffs - build.gradle 30d1cf2f1ff9ed3f86a060da8099bb0774b4cf91 clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 9be8fbc648369ad9db1a7eea94bc1b9edbfdbfd7 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 5671a3fbeea8cb9a9ffeeb41aa1b132b92c0cae8 clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 6b9590c418aedd2727544c5dd23c017b4b72467a clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 07b1b60d3a9cb1a399a2fe95b87229f64f539f3b clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 544e120594de78c43581a980b1e4087b4fb98ccb core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c Diff: https://reviews.apache.org/r/35867/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35867: Patch for KAFKA-1901
On June 25, 2015, 3:33 p.m., Jason Rosenberg wrote: clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java, line 58 https://reviews.apache.org/r/35867/diff/1/?file=991945#file991945line58 what's the reason for this change? This change is for new java clients. Currently new java clients uses kafka metrics and support only JMX reporting. In this change, I am passing clientId to JmxReporter. This will help use to create separate MBean for each client. And each MBean will have version and commitId attributes. Example MBean: kafka.producer:type=AppInfo,client-id=producer1 kafka.producer:type=AppInfo,client-id=producer2 kafka.consumer:type=AppInfo,client-id=consumer1 kafka.consumer:type=AppInfo,client-id=conusmer2 - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/#review89364 --- On June 25, 2015, 10:11 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35867/ --- (Updated June 25, 2015, 10:11 a.m.) Review request for kafka. Bugs: KAFKA-1901 https://issues.apache.org/jira/browse/KAFKA-1901 Repository: kafka Description --- generated a properties file (kafka-version.properties) with version, git-commitId in it. This props file is used to create AppInfo MBean. Diffs - build.gradle 30d1cf2f1ff9ed3f86a060da8099bb0774b4cf91 clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 9be8fbc648369ad9db1a7eea94bc1b9edbfdbfd7 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 5671a3fbeea8cb9a9ffeeb41aa1b132b92c0cae8 clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 6b9590c418aedd2727544c5dd23c017b4b72467a clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 07b1b60d3a9cb1a399a2fe95b87229f64f539f3b clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 544e120594de78c43581a980b1e4087b4fb98ccb core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c Diff: https://reviews.apache.org/r/35867/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35880: Patch for KAFKA-2295
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35880/ --- (Updated June 25, 2015, 4:25 p.m.) Review request for kafka. Bugs: KAFKA-2295 https://issues.apache.org/jira/browse/KAFKA-2295 Repository: kafka Description (updated) --- Implemented the approach given in JIRA description. This is to support dynamically loaded envirornments Diffs - clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 4170bcc7def5b50d8aa20e8e84089c35b705b527 clients/src/main/java/org/apache/kafka/common/utils/Utils.java af9993cf9b3991f1e61e1201c94e19bc1bf76a68 core/src/main/scala/kafka/utils/CoreUtils.scala 168a18d380c200ee566eccb6988dd1ae85ed5b09 Diff: https://reviews.apache.org/r/35880/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35503: Patch for KAFKA-2012
On June 19, 2015, 12:11 a.m., Jun Rao wrote: core/src/main/scala/kafka/log/Log.scala, lines 225-227 https://reviews.apache.org/r/35503/diff/1/?file=985641#file985641line225 Do we still need this now that we have verified the index consistency earlier? Yes, we can remove these statements On June 19, 2015, 12:11 a.m., Jun Rao wrote: core/src/main/scala/kafka/log/Log.scala, line 178 https://reviews.apache.org/r/35503/diff/1/?file=985641#file985641line178 Is this correct? The mmapping of the index file happens in the initialization of OffsetIndex. It seems that we need to re-create the LogSegment after the index file is deleted. index file gets recreated in OffsetIndex.mmap() method (val newlyCreated = file.createNewFile()). So we no need to re-create the LogSegment. Pl check. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35503/#review88470 --- On June 19, 2015, 1:27 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35503/ --- (Updated June 19, 2015, 1:27 p.m.) Review request for kafka. Bugs: KAFKA-2012 https://issues.apache.org/jira/browse/KAFKA-2012 Repository: kafka Description --- Address Jun's comments Diffs - core/src/main/scala/kafka/log/Log.scala 84e7b8fe9dd014884b60c4fbe13c835cf02a40e4 core/src/test/scala/unit/kafka/log/LogTest.scala a8e57c2348e694a1e0d5407ae8ae105fb1f1bf59 Diff: https://reviews.apache.org/r/35503/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35503: Patch for KAFKA-2012
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35503/ --- (Updated June 19, 2015, 1:27 p.m.) Review request for kafka. Bugs: KAFKA-2012 https://issues.apache.org/jira/browse/KAFKA-2012 Repository: kafka Description (updated) --- Address Jun's comments Diffs (updated) - core/src/main/scala/kafka/log/Log.scala 84e7b8fe9dd014884b60c4fbe13c835cf02a40e4 core/src/test/scala/unit/kafka/log/LogTest.scala a8e57c2348e694a1e0d5407ae8ae105fb1f1bf59 Diff: https://reviews.apache.org/r/35503/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35503: Patch for KAFKA-2012
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35503/ --- (Updated June 19, 2015, 3:42 p.m.) Review request for kafka. Bugs: KAFKA-2012 https://issues.apache.org/jira/browse/KAFKA-2012 Repository: kafka Description --- Address Jun's comments Diffs (updated) - core/src/main/scala/kafka/log/Log.scala 6b9274d5f1fff434440e00d9f0e27d32b673076a core/src/test/scala/unit/kafka/log/LogTest.scala a8e57c2348e694a1e0d5407ae8ae105fb1f1bf59 Diff: https://reviews.apache.org/r/35503/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 35610: Patch for KAFKA-2265
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35610/ --- Review request for kafka. Bugs: KAFKA-2265 https://issues.apache.org/jira/browse/KAFKA-2265 Repository: kafka Description --- Remove redundant zookeeper calls, to improvetopic creation time Diffs - core/src/main/scala/kafka/controller/PartitionStateMachine.scala 92fd92d135b24931de970347eae49ad24dd1f5c4 Diff: https://reviews.apache.org/r/35610/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 35454: Patch for KAFKA-2159
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35454/ --- (Updated June 17, 2015, 6:19 a.m.) Review request for kafka. Bugs: KAFKA-2159 https://issues.apache.org/jira/browse/KAFKA-2159 Repository: kafka Description (updated) --- Addessing Joel's comments, enabled offsets.topic.compression.codec config usage Diffs (updated) - core/src/main/scala/kafka/server/KafkaServer.scala b320ce9f6a12c0ee392e91beb82e8804d167f9f4 core/src/main/scala/kafka/server/OffsetManager.scala 5cca85cf727975f6d3acb2223fd186753ad761dc Diff: https://reviews.apache.org/r/35454/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34805: Patch for KAFKA-2213
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34805/ --- (Updated June 17, 2015, 10:38 a.m.) Review request for kafka. Bugs: KAFKA-2213 https://issues.apache.org/jira/browse/KAFKA-2213 Repository: kafka Description (updated) --- Addessing Joel's comments Diffs (updated) - clients/src/main/java/org/apache/kafka/common/record/Compressor.java e570b29d5ffba5d3754c46670b708f7d511086f3 clients/src/main/java/org/apache/kafka/common/record/MemoryRecords.java b2db2403868b1e7361b8514cfed2e76ef785edee core/src/main/scala/kafka/log/LogCleaner.scala c9ade7208798fbd92d4ff49e183fe5f8925c82a9 core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 471ddff9bff1bdfa277c071e59e5c6b749b9c74f Diff: https://reviews.apache.org/r/34805/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 35503: Patch for KAFKA-2012
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35503/ --- Review request for kafka. Bugs: KAFKA-2012 https://issues.apache.org/jira/browse/KAFKA-2012 Repository: kafka Description --- handle corrupt index files during server startup Diffs - core/src/main/scala/kafka/log/Log.scala 84e7b8fe9dd014884b60c4fbe13c835cf02a40e4 core/src/test/scala/unit/kafka/log/LogTest.scala 8e095d652851f05365e1d3bbe3e9e1c3345b7a40 Diff: https://reviews.apache.org/r/35503/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 35454: Patch for KAFKA-2159
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35454/ --- Review request for kafka. Bugs: KAFKA-2159 https://issues.apache.org/jira/browse/KAFKA-2159 Repository: kafka Description --- Renamed offsets.retention.minutes to offsets.topic.retention.minutes, offsetsTopicSegmentBytes parameter passed in KafkaServer.createOffsetManager() Diffs - core/src/main/scala/kafka/server/KafkaConfig.scala 2d75186a110075e0c322db4b9f7a8c964a7a3e88 core/src/main/scala/kafka/server/KafkaServer.scala b320ce9f6a12c0ee392e91beb82e8804d167f9f4 Diff: https://reviews.apache.org/r/35454/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 35437: Patch for KAFKA-2202
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35437/ --- Review request for kafka. Bugs: KAFKA-2202 https://issues.apache.org/jira/browse/KAFKA-2202 Repository: kafka Description --- while computing stats, consumerTimeoutMs is considered only during ConsumerTimeout exception senarios; This should solve KAFKA-1828 also Diffs - core/src/main/scala/kafka/tools/ConsumerPerformance.scala 903318d15893af08104a97499798c9ad0ba98013 Diff: https://reviews.apache.org/r/35437/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 35421: Patch for KAFKA-2026
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35421/ --- Review request for kafka. Bugs: KAFKA-2026 https://issues.apache.org/jira/browse/KAFKA-2026 Repository: kafka Description --- Logging of unused options values taken from this.originals Diffs - clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java c4fa058692f50abb4f47bd344119d805c60123f5 Diff: https://reviews.apache.org/r/35421/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 35418: Patch for KAFKA-2264
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35418/ --- Review request for kafka. Bugs: KAFKA-2264 https://issues.apache.org/jira/browse/KAFKA-2264 Repository: kafka Description --- SESSION_TIMEOUT_MS_CONFIG in ConsumerConfig should be int Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java 1e905240e7459a4a0a0573ae5d8ac19217cae197 clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java d1d1ec178f60dc47d408f52a89e52886c1a093a2 clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java c1496a0851526f3c7d3905ce4bdff2129c83a6c1 clients/src/test/java/org/apache/kafka/clients/consumer/internals/CoordinatorTest.java b06c4a73e2b4e9472cd772c8bc32bf4a29f431bb Diff: https://reviews.apache.org/r/35418/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 35419: Patch for KAFKA-2262
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35419/ --- Review request for kafka. Bugs: KAFKA-2262 https://issues.apache.org/jira/browse/KAFKA-2262 Repository: kafka Description --- LogSegmentSize validation should be consistent Diffs - core/src/main/scala/kafka/log/LogConfig.scala a907da09e1ccede3b446459225e407cd1ae6d8b3 Diff: https://reviews.apache.org/r/35419/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 35424: Patch for KAFKA-2234
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35424/ --- Review request for kafka. Bugs: KAFKA-2234 https://issues.apache.org/jira/browse/KAFKA-2234 Repository: kafka Description --- Partition reassignment of a nonexistent topic prevents future reassignments; Added empty validPartitions check before creating ZK node Diffs - core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala 912b718ec9ad58b1c2d42337ed85bbb1b88162d6 Diff: https://reviews.apache.org/r/35424/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 34805: Patch for KAFKA-2213
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34805/ --- Review request for kafka. Bugs: KAFKA-2213 https://issues.apache.org/jira/browse/KAFKA-2213 Repository: kafka Description --- Write the compacted messages using the configured broker compression type. Diffs - core/src/main/scala/kafka/log/Log.scala 84e7b8fe9dd014884b60c4fbe13c835cf02a40e4 core/src/main/scala/kafka/log/LogCleaner.scala c9ade7208798fbd92d4ff49e183fe5f8925c82a9 core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 471ddff9bff1bdfa277c071e59e5c6b749b9c74f Diff: https://reviews.apache.org/r/34805/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 33049: Patch for KAFKA-2084
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review85545 --- core/src/main/scala/kafka/server/ClientQuotaMetrics.scala https://reviews.apache.org/r/33049/#comment137140 Are we using clientID as uniqueKey?. But as of now, clientID is not mandatory and it need not be unique acorss different producer and consumers. - Manikumar Reddy O On May 26, 2015, 6:53 p.m., Aditya Auradkar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated May 26, 2015, 6:53 p.m.) Review request for kafka, Joel Koshy and Jun Rao. Bugs: KAFKA-2084 https://issues.apache.org/jira/browse/KAFKA-2084 Repository: kafka Description --- Updated patch for quotas. This patch does the following: 1. Add per-client metrics for both producer and consumers 2. Add configuration for quotas 3. Compute delay times in the metrics package and return the delay times in QuotaViolationException 4. Add a DelayQueue in KafkaApi's that can be used to throttle any type of request. Implemented request throttling for produce and fetch requests. 5. Added unit and integration test cases. 6. This doesn't include a system test. There is a separate ticket for that Diffs - clients/src/main/java/org/apache/kafka/common/metrics/MetricConfig.java dfa1b0a11042ad9d127226f0e0cec8b1d42b8441 clients/src/main/java/org/apache/kafka/common/metrics/Quota.java d82bb0c055e631425bc1ebbc7d387baac76aeeaa clients/src/main/java/org/apache/kafka/common/metrics/QuotaViolationException.java a451e5385c9eca76b38b425e8ac856b2715fcffe clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java ca823fd4639523018311b814fde69b6177e73b97 clients/src/test/java/org/apache/kafka/common/utils/MockTime.java core/src/main/scala/kafka/server/ClientQuotaMetrics.scala PRE-CREATION core/src/main/scala/kafka/server/KafkaApis.scala 387e387998fc3a6c9cb585dab02b5f77b0381fbf core/src/main/scala/kafka/server/KafkaConfig.scala 9efa15ca5567b295ab412ee9eea7c03eb4cdc18b core/src/main/scala/kafka/server/KafkaServer.scala e66710d2368334ece66f70d55f57b3f888262620 core/src/main/scala/kafka/server/ReplicaManager.scala 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 core/src/main/scala/kafka/server/ThrottledRequest.scala PRE-CREATION core/src/main/scala/kafka/utils/ShutdownableThread.scala fc226c863095b7761290292cd8755cd7ad0f155c core/src/test/scala/integration/kafka/api/QuotasTest.scala PRE-CREATION core/src/test/scala/unit/kafka/server/ClientQuotaMetricsTest.scala PRE-CREATION core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala 8014a5a6c362785539f24eb03d77278434614fe6 core/src/test/scala/unit/kafka/server/ThrottledRequestExpirationTest.scala PRE-CREATION Diff: https://reviews.apache.org/r/33049/diff/ Testing --- Thanks, Aditya Auradkar
Re: Review Request 34641: Patch for KAFKA-2214
On May 26, 2015, 7:08 a.m., Michael Noll wrote: core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala, line 81 https://reviews.apache.org/r/34641/diff/1/?file=971195#file971195line81 Should we also consider reassignments that are in-progress as errors? The reasoning is that you'd like to do the following in (say) Ansible: Trigger reassignment of partitions, wait until all are completed, and only then continue with the next action. That being said, there are other ways to achieve this in tools like Ansible. For instance, you can trigger reassignents, then repeatedly call `--verify` in a loop and capture its STDOUT, looking for is still in progress and failed. However, this is arguably more error prone because the log messages can change between Kafka versions (and oftentimes such changes are not prominently advertised, so you only notice this once your deployment script breaks). Yes, we can consider in-progress as errors. Other option could be returning a different error code. Let us wait for others suggestions/concerns. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/#review85154 --- On May 24, 2015, 11:58 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34641/ --- (Updated May 24, 2015, 11:58 a.m.) Review request for kafka. Bugs: KAFKA-2214 https://issues.apache.org/jira/browse/KAFKA-2214 Repository: kafka Description --- kafka-reassign-partitions.sh: return non-zero error status on failures Diffs - core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala acaa6112db979dc775af6385c58d2e52786dfba9 Diff: https://reviews.apache.org/r/34641/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 34403: Patch for KAFKA-2198
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34403/ --- Review request for kafka. Bugs: KAFKA-2198 https://issues.apache.org/jira/browse/KAFKA-2198 Repository: kafka Description --- kafka-topics.sh: return non-zero error status on failures Diffs - core/src/main/scala/kafka/admin/TopicCommand.scala 8e6f18633b25bf1beee3f813b28ef7aa7d779d7b Diff: https://reviews.apache.org/r/34403/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34403: Patch for KAFKA-2198
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34403/ --- (Updated May 19, 2015, 12:59 p.m.) Review request for kafka. Bugs: KAFKA-2198 https://issues.apache.org/jira/browse/KAFKA-2198 Repository: kafka Description --- kafka-topics.sh: return non-zero error status on failures Diffs (updated) - core/src/main/scala/kafka/admin/TopicCommand.scala 8e6f18633b25bf1beee3f813b28ef7aa7d779d7b Diff: https://reviews.apache.org/r/34403/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 34403: Patch for KAFKA-2198
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34403/ --- (Updated May 19, 2015, 1:14 p.m.) Review request for kafka. Bugs: KAFKA-2198 https://issues.apache.org/jira/browse/KAFKA-2198 Repository: kafka Description --- kafka-topics.sh: return non-zero error status on failures Diffs (updated) - core/src/main/scala/kafka/admin/TopicCommand.scala 8e6f18633b25bf1beee3f813b28ef7aa7d779d7b Diff: https://reviews.apache.org/r/34403/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 24214: Patch for KAFKA-1374
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated May 18, 2015, 5:29 p.m.) Review request for kafka. Bugs: KAFKA-1374 https://issues.apache.org/jira/browse/KAFKA-1374 Repository: kafka Description (updated) --- Addressing Joel's comments Diffs (updated) - core/src/main/scala/kafka/log/LogCleaner.scala abea8b251895a5cc0788c6e25b112a2935a3f631 core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 9dfe914991aaf82162e5e300c587c794555d5fd0 core/src/main/scala/kafka/message/MessageSet.scala 28b56e68cfdbbf107dd7cbd248ffa8fa6bbcd13f core/src/test/scala/kafka/tools/TestLogCleaning.scala 844589427cb9337acd89a5239a98b811ee58118e core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 3b5aa9dc3b7ac5893c1d281ae1326be0e9ed8aad core/src/test/scala/unit/kafka/log/LogTest.scala 76d3bfd378f32fd2b216b3ebdec86e2070491924 Diff: https://reviews.apache.org/r/24214/diff/ Testing --- /*TestLogCleaning stress test output for compressed messages/ Producing 10 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-6014466306002699464.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-177538909590644701.txt 10 rows of data produced, 13165 rows of data consumed (86.8% reduction). De-duplicating and validating output files... Validated 9005 values, 0 mismatches. Producing 100 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-3298578695475992991.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-7192293977610206930.txt 100 rows of data produced, 119926 rows of data consumed (88.0% reduction). De-duplicating and validating output files... Validated 89947 values, 0 mismatches. Producing 1000 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-3336255463347572934.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-9149188270705707725.txt 1000 rows of data produced, 1645281 rows of data consumed (83.5% reduction). De-duplicating and validating output files... Validated 899853 values, 0 mismatches. /*TestLogCleaning stress test output for non-compressed messages*/ Producing 10 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-5174543709786189363.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-514345501144701.txt 10 rows of data produced, 22775 rows of data consumed (77.2% reduction). De-duplicating and validating output files... Validated 17874 values, 0 mismatches. Producing 100 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-7814446915546169271.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-5172557663160447626.txt 100 rows of data produced, 129230 rows of data consumed (87.1% reduction). De-duplicating and validating output files... Validated 89947 values, 0 mismatches. Producing 1000 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-6092986571905399164.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-63626021421841220.txt 1000 rows of data produced, 1136608 rows of data consumed (88.6% reduction). De-duplicating and validating output files... Validated 899853 values, 0 mismatches. Thanks, Manikumar Reddy O
Re: Review Request 24214: Patch for KAFKA-1374
On May 12, 2015, 2:01 p.m., Joel Koshy wrote: core/src/main/scala/kafka/log/LogCleaner.scala, line 409 https://reviews.apache.org/r/24214/diff/9/?file=824405#file824405line409 I would suggest one of two options over this (i.e., instead of two helper methods) - Inline both here and get rid of those - Have a single private helper (e.g., collectRetainedMessages) removed the helper methods On May 12, 2015, 2:01 p.m., Joel Koshy wrote: core/src/main/scala/kafka/log/LogCleaner.scala, line 479 https://reviews.apache.org/r/24214/diff/9/?file=824405#file824405line479 We should now compress with the compression codec of the topic (KAFKA-1499) will do as separate JIRA On May 12, 2015, 2:01 p.m., Joel Koshy wrote: core/src/main/scala/kafka/log/LogCleaner.scala, line 498 https://reviews.apache.org/r/24214/diff/9/?file=824405#file824405line498 We should instead do a trivial refactor in ByteBufferMessageSet to compress messages in a preallocated buffer. It would be preferable to avoid having this compression logic in different places. moved the compresssMessages() method to ByteBufferMessageSet class. Pl let me know your thoughts.. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/#review83392 --- On May 18, 2015, 5:29 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated May 18, 2015, 5:29 p.m.) Review request for kafka. Bugs: KAFKA-1374 https://issues.apache.org/jira/browse/KAFKA-1374 Repository: kafka Description --- Addressing Joel's comments Diffs - core/src/main/scala/kafka/log/LogCleaner.scala abea8b251895a5cc0788c6e25b112a2935a3f631 core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 9dfe914991aaf82162e5e300c587c794555d5fd0 core/src/main/scala/kafka/message/MessageSet.scala 28b56e68cfdbbf107dd7cbd248ffa8fa6bbcd13f core/src/test/scala/kafka/tools/TestLogCleaning.scala 844589427cb9337acd89a5239a98b811ee58118e core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 3b5aa9dc3b7ac5893c1d281ae1326be0e9ed8aad core/src/test/scala/unit/kafka/log/LogTest.scala 76d3bfd378f32fd2b216b3ebdec86e2070491924 Diff: https://reviews.apache.org/r/24214/diff/ Testing --- /*TestLogCleaning stress test output for compressed messages/ Producing 10 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-6014466306002699464.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-177538909590644701.txt 10 rows of data produced, 13165 rows of data consumed (86.8% reduction). De-duplicating and validating output files... Validated 9005 values, 0 mismatches. Producing 100 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-3298578695475992991.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-7192293977610206930.txt 100 rows of data produced, 119926 rows of data consumed (88.0% reduction). De-duplicating and validating output files... Validated 89947 values, 0 mismatches. Producing 1000 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-3336255463347572934.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-9149188270705707725.txt 1000 rows of data produced, 1645281 rows of data consumed (83.5% reduction). De-duplicating and validating output files... Validated 899853 values, 0 mismatches. /*TestLogCleaning stress test output for non-compressed messages*/ Producing 10 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-5174543709786189363.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-514345501144701.txt 10 rows of data produced, 22775 rows of data consumed (77.2% reduction). De-duplicating and validating output files... Validated 17874 values, 0 mismatches. Producing 100 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-7814446915546169271.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-5172557663160447626.txt 100 rows of data produced, 129230 rows of data consumed (87.1% reduction). De-duplicating and validating output files... Validated 89947 values, 0 mismatches. Producing 1000 messages... Logging produce requests to /tmp
Re: Review Request 30801: Patch for KAFKA-1758
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30801/ --- (Updated May 9, 2015, 7:02 a.m.) Review request for kafka. Bugs: KAFKA-1758 https://issues.apache.org/jira/browse/KAFKA-1758 Repository: kafka Description (updated) --- Addressing Neha's comments Diffs (updated) - core/src/main/scala/kafka/log/LogManager.scala e781ebac2677ebb22e0c1fef0cf7e5ad57c74ea4 Diff: https://reviews.apache.org/r/30801/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 30801: Patch for KAFKA-1758
On April 26, 2015, 6:53 p.m., Neha Narkhede wrote: core/src/main/scala/kafka/log/LogManager.scala, line 133 https://reviews.apache.org/r/30801/diff/1/?file=858786#file858786line133 Is there a reason we are limiting this to only NumberFormatException? Seems like a fix that applies to all errors. Also, worth changing the error message to a more generic statement about the problem and the fix (resetting the recovery checkpoint to 0). Yes, we can catch other exceptions also. updated the log message. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30801/#review81622 --- On May 9, 2015, 7:02 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30801/ --- (Updated May 9, 2015, 7:02 a.m.) Review request for kafka. Bugs: KAFKA-1758 https://issues.apache.org/jira/browse/KAFKA-1758 Repository: kafka Description --- Addressing Neha's comments Diffs - core/src/main/scala/kafka/log/LogManager.scala e781ebac2677ebb22e0c1fef0cf7e5ad57c74ea4 Diff: https://reviews.apache.org/r/30801/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 33334: Patch for KAFKA-2131
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4/ --- Review request for kafka. Bugs: KAFKA-2131 https://issues.apache.org/jira/browse/KAFKA-2131 Repository: kafka Description --- Update new producer javadocs with correct documentation Diffs - clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java b91e2c52ed0acb1faa85915097d97bafa28c413a clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java ca1c7fedbde7f53d64426da3a1aa3aeeafd2e9ad Diff: https://reviews.apache.org/r/4/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 30321: Patch for kafka-1902
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30321/#review69813 --- Ship it! Ship It! - Manikumar Reddy O On Jan. 27, 2015, 4:16 p.m., Jun Rao wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30321/ --- (Updated Jan. 27, 2015, 4:16 p.m.) Review request for kafka. Bugs: kafka-1902 https://issues.apache.org/jira/browse/kafka-1902 Repository: kafka Description --- fix metric name by adding tags as scope Diffs - core/src/main/scala/kafka/metrics/KafkaMetricsGroup.scala e9e49180f6de45f98e79374f519f6097b4fc8637 Diff: https://reviews.apache.org/r/30321/diff/ Testing --- Thanks, Jun Rao
Re: Review Request 30078: Patch for KAFKA-1885
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30078/#review69532 --- Ship it! Ship It! - Manikumar Reddy O On Jan. 24, 2015, 5:13 a.m., Jaikiran Pai wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30078/ --- (Updated Jan. 24, 2015, 5:13 a.m.) Review request for kafka. Bugs: KAFKA-1885 https://issues.apache.org/jira/browse/KAFKA-1885 Repository: kafka Description --- KAFKA-1885 Upgrade junit dependency in core to 4.6 version to allow running individual test methods via gradle command line Diffs - build.gradle 1cbab29ce83e20dae0561b51eed6fdb86d522f28 Diff: https://reviews.apache.org/r/30078/diff/ Testing --- Tested that existing support to run an entire individual test case from the command line works as advertised: ```./gradlew -Dtest.single=ProducerFailureHandlingTest core:test kafka.api.test.ProducerFailureHandlingTest testTooLargeRecordWithAckZero PASSED kafka.api.test.ProducerFailureHandlingTest testTooLargeRecordWithAckOne PASSED kafka.api.test.ProducerFailureHandlingTest testNonExistentTopic PASSED kafka.api.test.ProducerFailureHandlingTest testWrongBrokerList PASSED kafka.api.test.ProducerFailureHandlingTest testNoResponse PASSED kafka.api.test.ProducerFailureHandlingTest testInvalidPartition PASSED kafka.api.test.ProducerFailureHandlingTest testSendAfterClosed PASSED kafka.api.test.ProducerFailureHandlingTest testBrokerFailure PASSED kafka.api.test.ProducerFailureHandlingTest testCannotSendToInternalTopic PASSED kafka.api.test.ProducerFailureHandlingTest testNotEnoughReplicas PASSED kafka.api.test.ProducerFailureHandlingTest testNotEnoughReplicasAfterBrokerShutdown PASSED BUILD SUCCESSFUL ``` Also tested that with this change it is now possible to run individual test methods as follows: ``` ./gradlew clients:test --tests org.apache.kafka.clients.producer.MetadataTest.testMetadataUpdateWaitTime org.apache.kafka.clients.producer.MetadataTest testMetadataUpdateWaitTime PASSED ``` ``` ./gradlew core:test --tests kafka.api.test.ProducerFailureHandlingTest.testCannotSendToInternalTopic kafka.api.test.ProducerFailureHandlingTest testCannotSendToInternalTopic PASSED ``` Thanks, Jaikiran Pai
Re: Review Request 30078: Patch for KAFKA-1885
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30078/#review69533 --- Ship it! Non-Binding +1 - Manikumar Reddy O On Jan. 24, 2015, 5:13 a.m., Jaikiran Pai wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30078/ --- (Updated Jan. 24, 2015, 5:13 a.m.) Review request for kafka. Bugs: KAFKA-1885 https://issues.apache.org/jira/browse/KAFKA-1885 Repository: kafka Description --- KAFKA-1885 Upgrade junit dependency in core to 4.6 version to allow running individual test methods via gradle command line Diffs - build.gradle 1cbab29ce83e20dae0561b51eed6fdb86d522f28 Diff: https://reviews.apache.org/r/30078/diff/ Testing --- Tested that existing support to run an entire individual test case from the command line works as advertised: ```./gradlew -Dtest.single=ProducerFailureHandlingTest core:test kafka.api.test.ProducerFailureHandlingTest testTooLargeRecordWithAckZero PASSED kafka.api.test.ProducerFailureHandlingTest testTooLargeRecordWithAckOne PASSED kafka.api.test.ProducerFailureHandlingTest testNonExistentTopic PASSED kafka.api.test.ProducerFailureHandlingTest testWrongBrokerList PASSED kafka.api.test.ProducerFailureHandlingTest testNoResponse PASSED kafka.api.test.ProducerFailureHandlingTest testInvalidPartition PASSED kafka.api.test.ProducerFailureHandlingTest testSendAfterClosed PASSED kafka.api.test.ProducerFailureHandlingTest testBrokerFailure PASSED kafka.api.test.ProducerFailureHandlingTest testCannotSendToInternalTopic PASSED kafka.api.test.ProducerFailureHandlingTest testNotEnoughReplicas PASSED kafka.api.test.ProducerFailureHandlingTest testNotEnoughReplicasAfterBrokerShutdown PASSED BUILD SUCCESSFUL ``` Also tested that with this change it is now possible to run individual test methods as follows: ``` ./gradlew clients:test --tests org.apache.kafka.clients.producer.MetadataTest.testMetadataUpdateWaitTime org.apache.kafka.clients.producer.MetadataTest testMetadataUpdateWaitTime PASSED ``` ``` ./gradlew core:test --tests kafka.api.test.ProducerFailureHandlingTest.testCannotSendToInternalTopic kafka.api.test.ProducerFailureHandlingTest testCannotSendToInternalTopic PASSED ``` Thanks, Jaikiran Pai
Review Request 30128: Patch for KAFKA-1861
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30128/ --- Review request for kafka. Bugs: KAFKA-1861 https://issues.apache.org/jira/browse/KAFKA-1861 Repository: kafka Description --- include clients test jar in maven artifacts Diffs - build.gradle 1cbab29ce83e20dae0561b51eed6fdb86d522f28 Diff: https://reviews.apache.org/r/30128/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 30073: Patch for KAFKA-1109
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30073/ --- Review request for kafka. Bugs: KAFKA-1109 https://issues.apache.org/jira/browse/KAFKA-1109 Repository: kafka Description --- Corrected kafka-run-class.sh script to override KAFKA_GC_LOG_OPTS environment property Diffs - bin/kafka-run-class.sh 22a9865b5939450a9d7f4ea2eee5eba2c1ec758c Diff: https://reviews.apache.org/r/30073/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 30022: Patch for KAFKA-1761
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30022/ --- (Updated Jan. 19, 2015, 6:23 a.m.) Review request for kafka. Bugs: KAFKA-1761 https://issues.apache.org/jira/browse/KAFKA-1761 Repository: kafka Description --- correct the default values of config properties Diffs (updated) - config/server.properties b0e4496a8ca736b6abe965a430e8ce87b0e8287f core/src/main/scala/kafka/server/KafkaConfig.scala 88689df718364f5a9bef143d4cb7e807a9251786 Diff: https://reviews.apache.org/r/30022/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 30022: Patch for KAFKA-1761
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30022/ --- Review request for kafka. Bugs: KAFKA-1761 https://issues.apache.org/jira/browse/KAFKA-1761 Repository: kafka Description --- correct the default values of config properties Diffs - config/server.properties b0e4496a8ca736b6abe965a430e8ce87b0e8287f core/src/main/scala/kafka/log/LogCleaner.scala f8e7cd5fabce78c248a9027c4bb374a792508675 core/src/main/scala/kafka/server/KafkaConfig.scala 88689df718364f5a9bef143d4cb7e807a9251786 core/src/main/scala/kafka/tools/TestLogCleaning.scala af496f7c547a5ac7a4096a6af325dad0d8feec6f core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 07acd460b1259e0a3f4069b8b8dcd8123ef5810e Diff: https://reviews.apache.org/r/30022/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 24214: Patch for KAFKA-1374
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Jan. 17, 2015, 6:53 p.m.) Review request for kafka. Bugs: KAFKA-1374 https://issues.apache.org/jira/browse/KAFKA-1374 Repository: kafka Description --- Updating the rebased code Diffs - core/src/main/scala/kafka/log/LogCleaner.scala f8e7cd5fabce78c248a9027c4bb374a792508675 core/src/main/scala/kafka/tools/TestLogCleaning.scala af496f7c547a5ac7a4096a6af325dad0d8feec6f core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 07acd460b1259e0a3f4069b8b8dcd8123ef5810e Diff: https://reviews.apache.org/r/24214/diff/ Testing (updated) --- /*TestLogCleaning stress test output for compressed messages/ Producing 10 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-6014466306002699464.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-177538909590644701.txt 10 rows of data produced, 13165 rows of data consumed (86.8% reduction). De-duplicating and validating output files... Validated 9005 values, 0 mismatches. Producing 100 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-3298578695475992991.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-7192293977610206930.txt 100 rows of data produced, 119926 rows of data consumed (88.0% reduction). De-duplicating and validating output files... Validated 89947 values, 0 mismatches. Producing 1000 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-3336255463347572934.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-9149188270705707725.txt 1000 rows of data produced, 1645281 rows of data consumed (83.5% reduction). De-duplicating and validating output files... Validated 899853 values, 0 mismatches. /*TestLogCleaning stress test output for non-compressed messages*/ Producing 10 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-5174543709786189363.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-514345501144701.txt 10 rows of data produced, 22775 rows of data consumed (77.2% reduction). De-duplicating and validating output files... Validated 17874 values, 0 mismatches. Producing 100 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-7814446915546169271.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-5172557663160447626.txt 100 rows of data produced, 129230 rows of data consumed (87.1% reduction). De-duplicating and validating output files... Validated 89947 values, 0 mismatches. Producing 1000 messages... Logging produce requests to /tmp/kafka-log-cleaner-produced-6092986571905399164.txt Sleeping for 120 seconds... Consuming messages... Logging consumed messages to /tmp/kafka-log-cleaner-consumed-63626021421841220.txt 1000 rows of data produced, 1136608 rows of data consumed (88.6% reduction). De-duplicating and validating output files... Validated 899853 values, 0 mismatches. Thanks, Manikumar Reddy O
Re: Review Request 24214: Patch for KAFKA-1374
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24214/ --- (Updated Jan. 17, 2015, 6:51 p.m.) Review request for kafka. Bugs: KAFKA-1374 https://issues.apache.org/jira/browse/KAFKA-1374 Repository: kafka Description (updated) --- Updating the rebased code Diffs (updated) - core/src/main/scala/kafka/log/LogCleaner.scala f8e7cd5fabce78c248a9027c4bb374a792508675 core/src/main/scala/kafka/tools/TestLogCleaning.scala af496f7c547a5ac7a4096a6af325dad0d8feec6f core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala 07acd460b1259e0a3f4069b8b8dcd8123ef5810e Diff: https://reviews.apache.org/r/24214/diff/ Testing (updated) --- /safe/KAFKA/docs/TestLogCleaning.txt Thanks, Manikumar Reddy O
Re: Review Request 29523: Patch for KAFKA-1723
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29523/ --- (Updated Jan. 14, 2015, 8:02 a.m.) Review request for kafka. Bugs: KAFKA-1723 https://issues.apache.org/jira/browse/KAFKA-1723 Repository: kafka Description (updated) --- corrected javadocs to compile in java 8 Diffs (updated) - clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java c79149a715fcbf40afb06f0c9f9e550143c81bdd clients/src/main/java/org/apache/kafka/common/MetricName.java 4e810d56b753b7eeb662b99af5cdf36bcfba7ea7 Diff: https://reviews.apache.org/r/29523/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 29523: Patch for KAFKA-1723
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29523/ --- (Updated Jan. 9, 2015, 8:56 a.m.) Review request for kafka. Bugs: KAFKA-1723 https://issues.apache.org/jira/browse/KAFKA-1723 Repository: kafka Description (updated) --- Standard JMX MBean Naming is implemented;Addresing Jun's comments Diffs (updated) - clients/src/main/java/org/apache/kafka/clients/consumer/Consumer.java 1bce50185273dbdbc131fbc9c7f5f3e9c346517a clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 7f8a41c4bf437711685a8271a4d3c83a176dd957 clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 8cab16c0a0bdb671fea1fc2fc2694247f66cc971 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 3053f2745c8e5f6e3b75826d3749656f150878db clients/src/main/java/org/apache/kafka/clients/producer/Producer.java 5baa6062bd9ba8a7d38058856ed2d831fae491f0 clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java aa91e1444a49c55870b9a7a32086fa2b04471fba clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java c15485d1af304ef53691d478f113f332fe67af77 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 84a7a07269c51ccc22ebb4ff9797292d07ba778e clients/src/main/java/org/apache/kafka/common/Metric.java b023e8e7c486adf21ed9a554085ab8ad7f3ee038 clients/src/main/java/org/apache/kafka/common/metrics/CompoundStat.java 29185a6a90d0035d650c7e56ce612a0878cb115c clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 3c312011a7ff7e79c277a89048e7e62ebd6078db clients/src/main/java/org/apache/kafka/common/metrics/KafkaMetric.java a7458b50cb16fbb2b31b857d5b359e65258bbf08 clients/src/main/java/org/apache/kafka/common/metrics/MetricName.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java 49be4019ac03835701c49646920766228ac7ffe9 clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 25c1faf2887ea02708c1f5b5f822f5299ed86bd6 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentile.java 7365ceb39072a6a1ecf533f5a20830ed1f2cfc72 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java c70d577ada8c099533d4f4ed2e86d37e0a6e6676 clients/src/main/java/org/apache/kafka/common/network/Selector.java 4dd2cdf773f7eb01a93d7f994383088960303dfc clients/src/test/java/org/apache/kafka/clients/producer/BufferPoolTest.java fe3c13f319d48b89a4f26b6d78c2c3e31cc50d7e clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java 2c9932401d573549c40f16fda8c4e3e11309cb85 clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java ef2ca65cabe97b909f17b62027a1bb06827e88fe clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 2f43c49450e1a3d671bd17417dc42941f1858750 clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 19bea0f1fa1ebf15d86623015ec909b0155e4bd3 clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 5c5e3d40819e41cab7b52a0eeaee5f2e7317b7b3 clients/src/test/java/org/apache/kafka/test/MetricsBench.java 9d98c1148255455fd801043b59b98fed9d0b76b3 Diff: https://reviews.apache.org/r/29523/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 29756: Patch for KAFKA-1854
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29756/#review67400 --- kafka-patch-review.py https://reviews.apache.org/r/29756/#comment111418 Can we do authentication check at the beginning and fail-fast if the username/password is wrong. Currently it will update the review-board and fails at JIRA update. Is it possible to catch the exception/error, and show the message Invalid JIRA username/password - Manikumar Reddy O On Jan. 9, 2015, 8:09 a.m., Jaikiran Pai wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29756/ --- (Updated Jan. 9, 2015, 8:09 a.m.) Review request for kafka. Bugs: KAFKA-1854 https://issues.apache.org/jira/browse/KAFKA-1854 Repository: kafka Description --- KAFKA-1854 Allow JIRA username and password to be prompted in the absence of a jira.ini file, during patch submission Diffs - kafka-patch-review.py b7f132f9d210b8648859ab8f9c89f30ec128ab38 Diff: https://reviews.apache.org/r/29756/diff/ Testing --- Thanks, Jaikiran Pai
Re: Review Request 29523: Patch for KAFKA-1723
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29523/ --- (Updated Jan. 9, 2015, 6:15 p.m.) Review request for kafka. Bugs: KAFKA-1723 https://issues.apache.org/jira/browse/KAFKA-1723 Repository: kafka Description --- Standard JMX MBean Naming is implemented;Addresing Jun's comments Diffs (updated) - build.gradle ba52288031e2abc70e35e9297a4423dd5025950b clients/src/main/java/org/apache/kafka/clients/consumer/Consumer.java 1bce50185273dbdbc131fbc9c7f5f3e9c346517a clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 7f8a41c4bf437711685a8271a4d3c83a176dd957 clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 8cab16c0a0bdb671fea1fc2fc2694247f66cc971 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 3053f2745c8e5f6e3b75826d3749656f150878db clients/src/main/java/org/apache/kafka/clients/producer/Producer.java 5baa6062bd9ba8a7d38058856ed2d831fae491f0 clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java aa91e1444a49c55870b9a7a32086fa2b04471fba clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java c15485d1af304ef53691d478f113f332fe67af77 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 84a7a07269c51ccc22ebb4ff9797292d07ba778e clients/src/main/java/org/apache/kafka/common/Metric.java b023e8e7c486adf21ed9a554085ab8ad7f3ee038 clients/src/main/java/org/apache/kafka/common/metrics/CompoundStat.java 29185a6a90d0035d650c7e56ce612a0878cb115c clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 3c312011a7ff7e79c277a89048e7e62ebd6078db clients/src/main/java/org/apache/kafka/common/metrics/KafkaMetric.java a7458b50cb16fbb2b31b857d5b359e65258bbf08 clients/src/main/java/org/apache/kafka/common/metrics/MetricName.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java 49be4019ac03835701c49646920766228ac7ffe9 clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 25c1faf2887ea02708c1f5b5f822f5299ed86bd6 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentile.java 7365ceb39072a6a1ecf533f5a20830ed1f2cfc72 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java c70d577ada8c099533d4f4ed2e86d37e0a6e6676 clients/src/main/java/org/apache/kafka/common/network/Selector.java 4dd2cdf773f7eb01a93d7f994383088960303dfc clients/src/test/java/org/apache/kafka/clients/producer/BufferPoolTest.java fe3c13f319d48b89a4f26b6d78c2c3e31cc50d7e clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java 2c9932401d573549c40f16fda8c4e3e11309cb85 clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java ef2ca65cabe97b909f17b62027a1bb06827e88fe clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 2f43c49450e1a3d671bd17417dc42941f1858750 clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 19bea0f1fa1ebf15d86623015ec909b0155e4bd3 clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 5c5e3d40819e41cab7b52a0eeaee5f2e7317b7b3 clients/src/test/java/org/apache/kafka/test/MetricsBench.java 9d98c1148255455fd801043b59b98fed9d0b76b3 Diff: https://reviews.apache.org/r/29523/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 29523: Patch for KAFKA-1723
On Jan. 8, 2015, 9:21 p.m., Jun Rao wrote: clients/src/main/java/org/apache/kafka/common/network/Selector.java, line 419 https://reviews.apache.org/r/29523/diff/20/?file=813232#file813232line419 It's probably better to just reference a ProducerMetrics constant. Selector is a low level utility. It can be used in new producer , consumer or etc. So instead of hard-coding, metricGrpName is passed as an argument On Jan. 8, 2015, 9:21 p.m., Jun Rao wrote: clients/src/main/java/org/apache/kafka/common/network/Selector.java, line 483 https://reviews.apache.org/r/29523/diff/20/?file=813232#file813232line483 Instead of node-+node, it probably can just be node. not sure.. node is a integer value (0,1). I felt node-0, node-1 are more informative, than simple 0, 1. Jconsole MBean Tree view looks like this kafka.producer -- ProducerNodeMetrics -- node-0 -- metric-1 -- node-1 -- metric-1 Is it Ok? If required i can change this? On Jan. 8, 2015, 9:21 p.m., Jun Rao wrote: clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, line 336 https://reviews.apache.org/r/29523/diff/20/?file=813222#file813222line336 Should this be passed in? Or perhaps we can define this as a constant and reference it here. Sender is internal to producer.. So the group name is hard-coded. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29523/#review67246 --- On Jan. 9, 2015, 6:15 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29523/ --- (Updated Jan. 9, 2015, 6:15 p.m.) Review request for kafka. Bugs: KAFKA-1723 https://issues.apache.org/jira/browse/KAFKA-1723 Repository: kafka Description --- Standard JMX MBean Naming is implemented;Addresing Jun's comments Diffs - build.gradle ba52288031e2abc70e35e9297a4423dd5025950b clients/src/main/java/org/apache/kafka/clients/consumer/Consumer.java 1bce50185273dbdbc131fbc9c7f5f3e9c346517a clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 7f8a41c4bf437711685a8271a4d3c83a176dd957 clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 8cab16c0a0bdb671fea1fc2fc2694247f66cc971 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 3053f2745c8e5f6e3b75826d3749656f150878db clients/src/main/java/org/apache/kafka/clients/producer/Producer.java 5baa6062bd9ba8a7d38058856ed2d831fae491f0 clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java aa91e1444a49c55870b9a7a32086fa2b04471fba clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java c15485d1af304ef53691d478f113f332fe67af77 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 84a7a07269c51ccc22ebb4ff9797292d07ba778e clients/src/main/java/org/apache/kafka/common/Metric.java b023e8e7c486adf21ed9a554085ab8ad7f3ee038 clients/src/main/java/org/apache/kafka/common/metrics/CompoundStat.java 29185a6a90d0035d650c7e56ce612a0878cb115c clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 3c312011a7ff7e79c277a89048e7e62ebd6078db clients/src/main/java/org/apache/kafka/common/metrics/KafkaMetric.java a7458b50cb16fbb2b31b857d5b359e65258bbf08 clients/src/main/java/org/apache/kafka/common/metrics/MetricName.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java 49be4019ac03835701c49646920766228ac7ffe9 clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 25c1faf2887ea02708c1f5b5f822f5299ed86bd6 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentile.java 7365ceb39072a6a1ecf533f5a20830ed1f2cfc72 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java c70d577ada8c099533d4f4ed2e86d37e0a6e6676 clients/src/main/java/org/apache/kafka/common/network/Selector.java 4dd2cdf773f7eb01a93d7f994383088960303dfc clients/src/test/java/org/apache/kafka/clients/producer/BufferPoolTest.java fe3c13f319d48b89a4f26b6d78c2c3e31cc50d7e clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java 2c9932401d573549c40f16fda8c4e3e11309cb85 clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java ef2ca65cabe97b909f17b62027a1bb06827e88fe clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 2f43c49450e1a3d671bd17417dc42941f1858750 clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java
Re: Review Request 29523: Patch for KAFKA-1723
On Jan. 8, 2015, 10:28 p.m., Jay Kreps wrote: clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java, line 115 https://reviews.apache.org/r/29523/diff/20/?file=813229#file813229line115 This won't be enough to distinguish the metric, right? Absent the group/tags info... I have implemented toString() method in MetricName. So this will print complete info about the metric name (name , group, description , tags) - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29523/#review67314 --- On Jan. 9, 2015, 6:15 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29523/ --- (Updated Jan. 9, 2015, 6:15 p.m.) Review request for kafka. Bugs: KAFKA-1723 https://issues.apache.org/jira/browse/KAFKA-1723 Repository: kafka Description --- Standard JMX MBean Naming is implemented;Addresing Jun's comments Diffs - build.gradle ba52288031e2abc70e35e9297a4423dd5025950b clients/src/main/java/org/apache/kafka/clients/consumer/Consumer.java 1bce50185273dbdbc131fbc9c7f5f3e9c346517a clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 7f8a41c4bf437711685a8271a4d3c83a176dd957 clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 8cab16c0a0bdb671fea1fc2fc2694247f66cc971 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 3053f2745c8e5f6e3b75826d3749656f150878db clients/src/main/java/org/apache/kafka/clients/producer/Producer.java 5baa6062bd9ba8a7d38058856ed2d831fae491f0 clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java aa91e1444a49c55870b9a7a32086fa2b04471fba clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java c15485d1af304ef53691d478f113f332fe67af77 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 84a7a07269c51ccc22ebb4ff9797292d07ba778e clients/src/main/java/org/apache/kafka/common/Metric.java b023e8e7c486adf21ed9a554085ab8ad7f3ee038 clients/src/main/java/org/apache/kafka/common/metrics/CompoundStat.java 29185a6a90d0035d650c7e56ce612a0878cb115c clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 3c312011a7ff7e79c277a89048e7e62ebd6078db clients/src/main/java/org/apache/kafka/common/metrics/KafkaMetric.java a7458b50cb16fbb2b31b857d5b359e65258bbf08 clients/src/main/java/org/apache/kafka/common/metrics/MetricName.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java 49be4019ac03835701c49646920766228ac7ffe9 clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 25c1faf2887ea02708c1f5b5f822f5299ed86bd6 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentile.java 7365ceb39072a6a1ecf533f5a20830ed1f2cfc72 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java c70d577ada8c099533d4f4ed2e86d37e0a6e6676 clients/src/main/java/org/apache/kafka/common/network/Selector.java 4dd2cdf773f7eb01a93d7f994383088960303dfc clients/src/test/java/org/apache/kafka/clients/producer/BufferPoolTest.java fe3c13f319d48b89a4f26b6d78c2c3e31cc50d7e clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java 2c9932401d573549c40f16fda8c4e3e11309cb85 clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java ef2ca65cabe97b909f17b62027a1bb06827e88fe clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 2f43c49450e1a3d671bd17417dc42941f1858750 clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 19bea0f1fa1ebf15d86623015ec909b0155e4bd3 clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 5c5e3d40819e41cab7b52a0eeaee5f2e7317b7b3 clients/src/test/java/org/apache/kafka/test/MetricsBench.java 9d98c1148255455fd801043b59b98fed9d0b76b3 Diff: https://reviews.apache.org/r/29523/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 29756: Patch for KAFKA-1854
On Jan. 9, 2015, 11:07 a.m., Manikumar Reddy O wrote: kafka-patch-review.py, line 96 https://reviews.apache.org/r/29756/diff/3/?file=814216#file814216line96 Did you tested the failure senario? I did not get error message. I think we will get exception only after invoking jira.issue(). Jaikiran Pai wrote: I did test a failure case, yes. Gave an incorrect username and password and that failed during login and did not proceed further. Here's the relevant output (notice that I intentionally use JIRA user name foo): ` Configuring reviewboard url to https://reviews.apache.org Updating your remote branches to pull the latest changes Verifying JIRA connection configurations JIRA user :foo JIRA password : Failed to login to the JIRA instance class 'jira.exceptions.JIRAError' HTTP 401: html head titleUnauthorized (401)/title ` Jaikiran Pai wrote: By the way, if you are testing this, make sure you don't have a jira.ini in its usual place. Else the (correct) credentials in there will be used as usual and you won't be able to replicate a failing scenario. Manikumar Reddy O wrote: Configuring reviewboard url to https://reviews.apache.org Updating your remote branches to pull the latest changes Verifying JIRA connection configurations JIRA user :foo JIRA password : Creating diff against origin/0.8.2 and uploading patch to JIRA KAFKA-1723 I am not getting exception, it going to next line of execution. It is failing at jira.issue(). Version : jira-python 0.16 Jaikiran Pai wrote: That's interesting. Let me see what might be different. Jaikiran Pai wrote: Turns out the version of jira python that I had was 0.31 and behaved differently. I downgraded to 0.16 and was able to replicate what you are seeing. I've updated the patch to fix this and it should now correctly fail if wrong JIRA credentials are specified, even before it attempts to publish to reviewboard. Let me know if this now works for you too. Thanks for testing this! Latest patch is working with jira-python 0.16. Some members may be using older versions. We need to find correct API which works across all the jira-python versions. (or) we need to add minimum version requirement. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29756/#review67414 --- On Jan. 9, 2015, 12:47 p.m., Jaikiran Pai wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29756/ --- (Updated Jan. 9, 2015, 12:47 p.m.) Review request for kafka. Bugs: KAFKA-1854 https://issues.apache.org/jira/browse/KAFKA-1854 Repository: kafka Description --- KAFKA-1854 Allow JIRA username and password to be prompted in the absence of a jira.ini file, during patch submission Diffs - kafka-patch-review.py b7f132f9d210b8648859ab8f9c89f30ec128ab38 Diff: https://reviews.apache.org/r/29756/diff/ Testing --- Thanks, Jaikiran Pai
Re: Review Request 29523: Patch for KAFKA-1723
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29523/ --- (Updated Jan. 9, 2015, 6:12 p.m.) Review request for kafka. Bugs: KAFKA-1723 https://issues.apache.org/jira/browse/KAFKA-1723 Repository: kafka Description --- Standard JMX MBean Naming is implemented;Addresing Jun's comments Diffs (updated) - build.gradle ba52288031e2abc70e35e9297a4423dd5025950b clients/src/main/java/org/apache/kafka/clients/consumer/Consumer.java 1bce50185273dbdbc131fbc9c7f5f3e9c346517a clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 7f8a41c4bf437711685a8271a4d3c83a176dd957 clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 8cab16c0a0bdb671fea1fc2fc2694247f66cc971 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 3053f2745c8e5f6e3b75826d3749656f150878db clients/src/main/java/org/apache/kafka/clients/producer/Producer.java 5baa6062bd9ba8a7d38058856ed2d831fae491f0 clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java aa91e1444a49c55870b9a7a32086fa2b04471fba clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java c15485d1af304ef53691d478f113f332fe67af77 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 84a7a07269c51ccc22ebb4ff9797292d07ba778e clients/src/main/java/org/apache/kafka/common/Metric.java b023e8e7c486adf21ed9a554085ab8ad7f3ee038 clients/src/main/java/org/apache/kafka/common/metrics/CompoundStat.java 29185a6a90d0035d650c7e56ce612a0878cb115c clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 3c312011a7ff7e79c277a89048e7e62ebd6078db clients/src/main/java/org/apache/kafka/common/metrics/KafkaMetric.java a7458b50cb16fbb2b31b857d5b359e65258bbf08 clients/src/main/java/org/apache/kafka/common/metrics/MetricName.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java 49be4019ac03835701c49646920766228ac7ffe9 clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 25c1faf2887ea02708c1f5b5f822f5299ed86bd6 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentile.java 7365ceb39072a6a1ecf533f5a20830ed1f2cfc72 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java c70d577ada8c099533d4f4ed2e86d37e0a6e6676 clients/src/main/java/org/apache/kafka/common/network/Selector.java 4dd2cdf773f7eb01a93d7f994383088960303dfc clients/src/test/java/org/apache/kafka/clients/producer/BufferPoolTest.java fe3c13f319d48b89a4f26b6d78c2c3e31cc50d7e clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java 2c9932401d573549c40f16fda8c4e3e11309cb85 clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java ef2ca65cabe97b909f17b62027a1bb06827e88fe clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 2f43c49450e1a3d671bd17417dc42941f1858750 clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 19bea0f1fa1ebf15d86623015ec909b0155e4bd3 clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 5c5e3d40819e41cab7b52a0eeaee5f2e7317b7b3 clients/src/test/java/org/apache/kafka/test/MetricsBench.java 9d98c1148255455fd801043b59b98fed9d0b76b3 Diff: https://reviews.apache.org/r/29523/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 29756: Patch for KAFKA-1854
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29756/#review67414 --- kafka-patch-review.py https://reviews.apache.org/r/29756/#comment111442 Did you tested the failure senario? I did not get error message. I think we will get exception only after invoking jira.issue(). - Manikumar Reddy O On Jan. 9, 2015, 10:13 a.m., Jaikiran Pai wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29756/ --- (Updated Jan. 9, 2015, 10:13 a.m.) Review request for kafka. Bugs: KAFKA-1854 https://issues.apache.org/jira/browse/KAFKA-1854 Repository: kafka Description --- KAFKA-1854 Allow JIRA username and password to be prompted in the absence of a jira.ini file, during patch submission Diffs - kafka-patch-review.py b7f132f9d210b8648859ab8f9c89f30ec128ab38 Diff: https://reviews.apache.org/r/29756/diff/ Testing --- Thanks, Jaikiran Pai
Re: Review Request 29756: Patch for KAFKA-1854
On Jan. 9, 2015, 11:07 a.m., Manikumar Reddy O wrote: kafka-patch-review.py, line 96 https://reviews.apache.org/r/29756/diff/3/?file=814216#file814216line96 Did you tested the failure senario? I did not get error message. I think we will get exception only after invoking jira.issue(). Jaikiran Pai wrote: I did test a failure case, yes. Gave an incorrect username and password and that failed during login and did not proceed further. Here's the relevant output (notice that I intentionally use JIRA user name foo): ` Configuring reviewboard url to https://reviews.apache.org Updating your remote branches to pull the latest changes Verifying JIRA connection configurations JIRA user :foo JIRA password : Failed to login to the JIRA instance class 'jira.exceptions.JIRAError' HTTP 401: html head titleUnauthorized (401)/title ` Jaikiran Pai wrote: By the way, if you are testing this, make sure you don't have a jira.ini in its usual place. Else the (correct) credentials in there will be used as usual and you won't be able to replicate a failing scenario. Configuring reviewboard url to https://reviews.apache.org Updating your remote branches to pull the latest changes Verifying JIRA connection configurations JIRA user :foo JIRA password : Creating diff against origin/0.8.2 and uploading patch to JIRA KAFKA-1723 I am not getting exception, it going to next line of execution. It is failing at jira.issue(). Version : jira-python 0.16 - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29756/#review67414 --- On Jan. 9, 2015, 10:13 a.m., Jaikiran Pai wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29756/ --- (Updated Jan. 9, 2015, 10:13 a.m.) Review request for kafka. Bugs: KAFKA-1854 https://issues.apache.org/jira/browse/KAFKA-1854 Repository: kafka Description --- KAFKA-1854 Allow JIRA username and password to be prompted in the absence of a jira.ini file, during patch submission Diffs - kafka-patch-review.py b7f132f9d210b8648859ab8f9c89f30ec128ab38 Diff: https://reviews.apache.org/r/29756/diff/ Testing --- Thanks, Jaikiran Pai
Re: Review Request 29738: Patch for kafka-1797
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29738/#review67374 --- Ship it! Ship It! - Manikumar Reddy O On Jan. 9, 2015, 1:22 a.m., Jun Rao wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29738/ --- (Updated Jan. 9, 2015, 1:22 a.m.) Review request for kafka. Bugs: kafka-1797 https://issues.apache.org/jira/browse/kafka-1797 Repository: kafka Description --- add null support in string serializer and deserializer Diffs - clients/src/main/java/org/apache/kafka/common/serialization/StringDeserializer.java a3b3700a1e0716643761d7032bd32bce839d3065 clients/src/main/java/org/apache/kafka/common/serialization/StringSerializer.java 02db47f8736988343dd293fc3da03751f78a3b5c clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java d550a3137c066abb5e2984ac6245574832929ff8 Diff: https://reviews.apache.org/r/29738/diff/ Testing --- Thanks, Jun Rao
Re: Review Request 29724: Patch for KAFKA-1566
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29724/#review67376 --- Ship it! Ship It! - Manikumar Reddy O On Jan. 8, 2015, 8:47 p.m., Sriharsha Chintalapani wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29724/ --- (Updated Jan. 8, 2015, 8:47 p.m.) Review request for kafka. Bugs: KAFKA-1566 https://issues.apache.org/jira/browse/KAFKA-1566 Repository: kafka Description --- KAFKA-1566. Kafka environment configuration (kafka-env.sh). Diffs - bin/kafka-run-class.sh ce3a4d06a27f66a33d729a15207b2d226b100c6a bin/windows/kafka-run-class.bat 9df3d2b45236b4f06d55a89c84afcf0ab9f5d0f2 config/kafka-env.cmd PRE-CREATION config/kafka-env.sh PRE-CREATION Diff: https://reviews.apache.org/r/29724/diff/ Testing --- Thanks, Sriharsha Chintalapani
Re: Review Request 29590: 1. Removed defaults for serializer/deserializer. 2. Converted type cast exception to serialization exception in the producer. 3. Added string ser/deser. 4. Moved the isKey fl
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29590/#review67194 --- clients/src/main/java/org/apache/kafka/common/serialization/StringSerializer.java https://reviews.apache.org/r/29590/#comment111066 we should support null values right?. This is required for compaction.. - Manikumar Reddy O On Jan. 5, 2015, 7:47 p.m., Jun Rao wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29590/ --- (Updated Jan. 5, 2015, 7:47 p.m.) Review request for kafka. Bugs: kafka-1797 https://issues.apache.org/jira/browse/kafka-1797 Repository: kafka Description --- addressing Jay's comments Diffs - clients/src/main/java/org/apache/kafka/clients/consumer/ByteArrayDeserializer.java 514cbd2c27a8d1ce13489d315f7880dfade7ffde clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java 1d64f08762b0c33fcaebde0f41039b327060215a clients/src/main/java/org/apache/kafka/clients/consumer/Deserializer.java c774a199db71fbc00776cd1256af57b2d9e55a66 clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java fe9066388f4b7910512d85ef088a1b96749735ac clients/src/main/java/org/apache/kafka/clients/producer/ByteArraySerializer.java 9005b74a328c997663232fe3a0999b25d2267efe clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d859fc588a276eb36bcfd621ae6d7978ad0decdd clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java 9cdc13d6cbb372b350acf90a21538b8ba495d2e8 clients/src/main/java/org/apache/kafka/clients/producer/Serializer.java de87f9c1caeadd176195be75d0db43fc0a518380 clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 3d4ab7228926f50309c07f0672f33416ce4fa37f clients/src/main/java/org/apache/kafka/common/errors/DeserializationException.java a5433398fb9788e260a4250da32e4be607f3f207 clients/src/main/java/org/apache/kafka/common/serialization/StringDeserializer.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/serialization/StringSerializer.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/serialization/SerializationTest.java PRE-CREATION core/src/main/scala/kafka/producer/KafkaLog4jAppender.scala e194942492324092f811b86f9c1f28f79b366cfd core/src/main/scala/kafka/tools/ConsoleProducer.scala 397d80da08c925757649b7d104d8360f56c604c3 core/src/main/scala/kafka/tools/MirrorMaker.scala 2126f6e55c5ec6a418165d340cc9a4f445af5045 core/src/main/scala/kafka/tools/ProducerPerformance.scala f2dc4ed2f04f0e9656e10b02db5ed1d39c4a4d39 core/src/main/scala/kafka/tools/ReplayLogProducer.scala f541987b2876a438c43ea9088ae8fed708ba82a3 core/src/main/scala/kafka/tools/TestEndToEndLatency.scala 2ebc7bf643ea91bd93ba37b8e64e8a5a9bb37ece core/src/main/scala/kafka/tools/TestLogCleaning.scala b81010ec0fa9835bfe48ce6aad0c491cdc67e7ef core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala 1505fd4464dc9ac71cce52d9b64406a21e5e45d2 core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 6196060edf9f1650720ec916f88933953a1daa2c core/src/test/scala/unit/kafka/utils/TestUtils.scala 94d0028d8c4907e747aa8a74a13d719b974c97bf Diff: https://reviews.apache.org/r/29590/diff/ Testing --- Thanks, Jun Rao
Re: Review Request 29523: Patch for KAFKA-1723
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29523/ --- (Updated Jan. 8, 2015, 4:34 p.m.) Review request for kafka. Bugs: KAFKA-1723 https://issues.apache.org/jira/browse/KAFKA-1723 Repository: kafka Description --- Standard JMX MBean Naming is implemented;Addresing Jay's comments Diffs (updated) - clients/src/main/java/org/apache/kafka/clients/consumer/Consumer.java 1bce50185273dbdbc131fbc9c7f5f3e9c346517a clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 7f8a41c4bf437711685a8271a4d3c83a176dd957 clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 8cab16c0a0bdb671fea1fc2fc2694247f66cc971 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 3053f2745c8e5f6e3b75826d3749656f150878db clients/src/main/java/org/apache/kafka/clients/producer/Producer.java 5baa6062bd9ba8a7d38058856ed2d831fae491f0 clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java aa91e1444a49c55870b9a7a32086fa2b04471fba clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java c15485d1af304ef53691d478f113f332fe67af77 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 84a7a07269c51ccc22ebb4ff9797292d07ba778e clients/src/main/java/org/apache/kafka/common/Metric.java b023e8e7c486adf21ed9a554085ab8ad7f3ee038 clients/src/main/java/org/apache/kafka/common/metrics/CompoundStat.java 29185a6a90d0035d650c7e56ce612a0878cb115c clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 3c312011a7ff7e79c277a89048e7e62ebd6078db clients/src/main/java/org/apache/kafka/common/metrics/KafkaMetric.java a7458b50cb16fbb2b31b857d5b359e65258bbf08 clients/src/main/java/org/apache/kafka/common/metrics/MetricName.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java 49be4019ac03835701c49646920766228ac7ffe9 clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 25c1faf2887ea02708c1f5b5f822f5299ed86bd6 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentile.java 7365ceb39072a6a1ecf533f5a20830ed1f2cfc72 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java c70d577ada8c099533d4f4ed2e86d37e0a6e6676 clients/src/main/java/org/apache/kafka/common/network/Selector.java 4dd2cdf773f7eb01a93d7f994383088960303dfc clients/src/test/java/org/apache/kafka/clients/producer/BufferPoolTest.java fe3c13f319d48b89a4f26b6d78c2c3e31cc50d7e clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java 2c9932401d573549c40f16fda8c4e3e11309cb85 clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java ef2ca65cabe97b909f17b62027a1bb06827e88fe clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 2f43c49450e1a3d671bd17417dc42941f1858750 clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 19bea0f1fa1ebf15d86623015ec909b0155e4bd3 clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 5c5e3d40819e41cab7b52a0eeaee5f2e7317b7b3 clients/src/test/java/org/apache/kafka/test/MetricsBench.java 9d98c1148255455fd801043b59b98fed9d0b76b3 Diff: https://reviews.apache.org/r/29523/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 29523: Patch for KAFKA-1723
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29523/ --- (Updated Jan. 8, 2015, 4:13 p.m.) Review request for kafka. Bugs: KAFKA-1723 https://issues.apache.org/jira/browse/KAFKA-1723 Repository: kafka Description (updated) --- Standard JMX MBean Naming is implemented;Addresing Jay's comments Diffs (updated) - clients/src/main/java/org/apache/kafka/clients/consumer/Consumer.java 1bce50185273dbdbc131fbc9c7f5f3e9c346517a clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 7f8a41c4bf437711685a8271a4d3c83a176dd957 clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java 8cab16c0a0bdb671fea1fc2fc2694247f66cc971 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 3053f2745c8e5f6e3b75826d3749656f150878db clients/src/main/java/org/apache/kafka/clients/producer/Producer.java 5baa6062bd9ba8a7d38058856ed2d831fae491f0 clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java aa91e1444a49c55870b9a7a32086fa2b04471fba clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java c15485d1af304ef53691d478f113f332fe67af77 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 84a7a07269c51ccc22ebb4ff9797292d07ba778e clients/src/main/java/org/apache/kafka/common/Metric.java b023e8e7c486adf21ed9a554085ab8ad7f3ee038 clients/src/main/java/org/apache/kafka/common/metrics/CompoundStat.java 29185a6a90d0035d650c7e56ce612a0878cb115c clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 3c312011a7ff7e79c277a89048e7e62ebd6078db clients/src/main/java/org/apache/kafka/common/metrics/KafkaMetric.java a7458b50cb16fbb2b31b857d5b359e65258bbf08 clients/src/main/java/org/apache/kafka/common/metrics/MetricName.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java 49be4019ac03835701c49646920766228ac7ffe9 clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 25c1faf2887ea02708c1f5b5f822f5299ed86bd6 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentile.java 7365ceb39072a6a1ecf533f5a20830ed1f2cfc72 clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java c70d577ada8c099533d4f4ed2e86d37e0a6e6676 clients/src/main/java/org/apache/kafka/common/network/Selector.java 4dd2cdf773f7eb01a93d7f994383088960303dfc clients/src/test/java/org/apache/kafka/clients/producer/BufferPoolTest.java fe3c13f319d48b89a4f26b6d78c2c3e31cc50d7e clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java 2c9932401d573549c40f16fda8c4e3e11309cb85 clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java ef2ca65cabe97b909f17b62027a1bb06827e88fe clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 2f43c49450e1a3d671bd17417dc42941f1858750 clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 19bea0f1fa1ebf15d86623015ec909b0155e4bd3 clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 5c5e3d40819e41cab7b52a0eeaee5f2e7317b7b3 clients/src/test/java/org/apache/kafka/test/MetricsBench.java 9d98c1148255455fd801043b59b98fed9d0b76b3 Diff: https://reviews.apache.org/r/29523/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 29523: Patch for KAFKA-1723
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29523/ --- Review request for kafka. Bugs: KAFKA-1723 https://issues.apache.org/jira/browse/KAFKA-1723 Repository: kafka Description --- Standard JMX MBean Naming is implemented Diffs - clients/src/main/java/org/apache/kafka/clients/NetworkClient.java 525b95e98010cd2053eacd8c321d079bcac2f910 clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java d859fc588a276eb36bcfd621ae6d7978ad0decdd clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java aa91e1444a49c55870b9a7a32086fa2b04471fba clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java c15485d1af304ef53691d478f113f332fe67af77 clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 84a7a07269c51ccc22ebb4ff9797292d07ba778e clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 3c312011a7ff7e79c277a89048e7e62ebd6078db clients/src/main/java/org/apache/kafka/common/metrics/KafkaMetric.java a7458b50cb16fbb2b31b857d5b359e65258bbf08 clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java 49be4019ac03835701c49646920766228ac7ffe9 clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 25c1faf2887ea02708c1f5b5f822f5299ed86bd6 clients/src/main/java/org/apache/kafka/common/network/Selector.java 4dd2cdf773f7eb01a93d7f994383088960303dfc clients/src/test/java/org/apache/kafka/clients/producer/BufferPoolTest.java fe3c13f319d48b89a4f26b6d78c2c3e31cc50d7e clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java 2c9932401d573549c40f16fda8c4e3e11309cb85 clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java ef2ca65cabe97b909f17b62027a1bb06827e88fe clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 2f43c49450e1a3d671bd17417dc42941f1858750 clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 19bea0f1fa1ebf15d86623015ec909b0155e4bd3 clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 5c5e3d40819e41cab7b52a0eeaee5f2e7317b7b3 clients/src/test/java/org/apache/kafka/test/MetricsBench.java 9d98c1148255455fd801043b59b98fed9d0b76b3 Diff: https://reviews.apache.org/r/29523/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 24704: Patch for KAFKA-1499
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24704/ --- (Updated Dec. 26, 2014, 4:09 p.m.) Review request for kafka. Bugs: KAFKA-1499 https://issues.apache.org/jira/browse/KAFKA-1499 Repository: kafka Description (updated) --- Support given for Broker-side compression, Addersing Joel's comments Diffs (updated) - core/src/main/scala/kafka/log/Log.scala 024506cd00556a0037c0b3b6b603da32968b69ab core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03 core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1 core/src/main/scala/kafka/message/CompressionCodec.scala 9439d2bc29a0c2327085f08577c6ce1b01db1489 core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e71eb16bf6d0e68bc4ebe core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef Diff: https://reviews.apache.org/r/24704/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 24704: Patch for KAFKA-1499
On Dec. 18, 2014, 4:30 a.m., Eric Olander wrote:
core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala, line 78
https://reviews.apache.org/r/24704/diff/13/?file=793051#file793051line78
A more Scala-like implementation is:
def parameters = {
for (brokerCompression -
BrokerCompression.brokerCompressionOptions;
messageCompression - CompressionType.values
) yield Array(messageCompression.name, brokerCompression)
}
If the resulting collection needs to be a java Collection you could
import the JavaConversions implicits, or just ignore me.
Thanks for the suggestion.
- Manikumar Reddy
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/#review65450
---
On Dec. 26, 2014, 4:09 p.m., Manikumar Reddy O wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24704/
---
(Updated Dec. 26, 2014, 4:09 p.m.)
Review request for kafka.
Bugs: KAFKA-1499
https://issues.apache.org/jira/browse/KAFKA-1499
Repository: kafka
Description
---
Support given for Broker-side compression, Addersing Joel's comments
Diffs
-
core/src/main/scala/kafka/log/Log.scala
024506cd00556a0037c0b3b6b603da32968b69ab
core/src/main/scala/kafka/log/LogConfig.scala
ca7a99e99f641b2694848b88bf4ae94657071d03
core/src/main/scala/kafka/message/ByteBufferMessageSet.scala
788c7864bc881b935975ab4a4e877b690e65f1f1
core/src/main/scala/kafka/message/CompressionCodec.scala
9439d2bc29a0c2327085f08577c6ce1b01db1489
core/src/main/scala/kafka/server/KafkaConfig.scala
6e26c5436feb4629d17f199011f3ebb674aa767f
core/src/main/scala/kafka/server/KafkaServer.scala
1bf7d10cef23a77e71eb16bf6d0e68bc4ebe
core/src/test/scala/kafka/log/LogConfigTest.scala
99b0df7b69c5e0a1b6251c54592c6ef63b1800fe
core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION
core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala
4e45d965bc423192ac704883ee75e9727006f89b
core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala
2377abe4933e065d037828a214c3a87e1773a8ef
Diff: https://reviews.apache.org/r/24704/diff/
Testing
---
Thanks,
Manikumar Reddy O
Re: Review Request 24704: Patch for KAFKA-1499
On Dec. 17, 2014, 1:57 p.m., Joel Koshy wrote: core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala, line 67 https://reviews.apache.org/r/24704/diff/13/?file=793051#file793051line67 `...should produce + readMessage(0).compressionCodec` i didnt find any issue here On Dec. 17, 2014, 1:57 p.m., Joel Koshy wrote: core/src/main/scala/kafka/message/BrokerCompression.scala, line 20 https://reviews.apache.org/r/24704/diff/13/?file=793046#file793046line20 Can you think of a way to avoid having to maintain this distinctly from CompressionCodec.scala? E.g., one thought is to have a separate BrokerCompressionCodec sealed trait in that class; have all the existing codecs extend from both CompressionCodec and BrokerCompressionCodec and the new ProducerCompressionCodec only extend from BrokerCompressionCodec (or see if there is an altogether different better way) Joel, i implemented your suggestion here. Merged the new BrokerCompression code to existing CompressionCodec.scala. Pl let me know if any changes are required. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24704/#review65316 --- On Dec. 26, 2014, 4:09 p.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24704/ --- (Updated Dec. 26, 2014, 4:09 p.m.) Review request for kafka. Bugs: KAFKA-1499 https://issues.apache.org/jira/browse/KAFKA-1499 Repository: kafka Description --- Support given for Broker-side compression, Addersing Joel's comments Diffs - core/src/main/scala/kafka/log/Log.scala 024506cd00556a0037c0b3b6b603da32968b69ab core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03 core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1 core/src/main/scala/kafka/message/CompressionCodec.scala 9439d2bc29a0c2327085f08577c6ce1b01db1489 core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e71eb16bf6d0e68bc4ebe core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef Diff: https://reviews.apache.org/r/24704/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 24704: Patch for KAFKA-1499
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24704/ --- (Updated Dec. 16, 2014, 5:10 p.m.) Review request for kafka. Bugs: KAFKA-1499 https://issues.apache.org/jira/browse/KAFKA-1499 Repository: kafka Description (updated) --- Support given for Broker-side compression Diffs (updated) - core/src/main/scala/kafka/log/Log.scala 4fae2f0d339b256832baa62ca4995d10546716b4 core/src/main/scala/kafka/log/LogConfig.scala ca7a99e99f641b2694848b88bf4ae94657071d03 core/src/main/scala/kafka/message/BrokerCompression.scala PRE-CREATION core/src/main/scala/kafka/message/ByteBufferMessageSet.scala 788c7864bc881b935975ab4a4e877b690e65f1f1 core/src/main/scala/kafka/server/KafkaConfig.scala 6e26c5436feb4629d17f199011f3ebb674aa767f core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e71eb16bf6d0e68bc4ebe core/src/test/scala/kafka/log/LogConfigTest.scala 99b0df7b69c5e0a1b6251c54592c6ef63b1800fe core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala PRE-CREATION core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala 4e45d965bc423192ac704883ee75e9727006f89b core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2377abe4933e065d037828a214c3a87e1773a8ef Diff: https://reviews.apache.org/r/24704/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 28536: Patch for KAFKA-1799
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28536/ --- (Updated Nov. 30, 2014, 10:29 a.m.) Review request for kafka. Bugs: KAFKA-1799 https://issues.apache.org/jira/browse/KAFKA-1799 Repository: kafka Description --- Explicit string to class conversion done in gAbstractConfig.getConfiguredInstances() Diffs (updated) - clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 8d886105341555a548ecc7b2901e7fc5d6b1ee8c Diff: https://reviews.apache.org/r/28536/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 28536: Patch for KAFKA-1799
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28536/ --- (Updated Nov. 30, 2014, 10:35 a.m.) Review request for kafka. Bugs: KAFKA-1799 https://issues.apache.org/jira/browse/KAFKA-1799 Repository: kafka Description (updated) --- Explicit string to class conversion done in gAbstractConfig.getConfiguredInstances(), Addrresing Jun's comments Diffs (updated) - clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 8d886105341555a548ecc7b2901e7fc5d6b1ee8c clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java PRE-CREATION Diff: https://reviews.apache.org/r/28536/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 28536: Patch for KAFKA-1799
On Nov. 29, 2014, 8:02 p.m., Jun Rao wrote: clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java, line 153 https://reviews.apache.org/r/28536/diff/1/?file=778386#file778386line153 We probably should throw a ConfigException instead. Used ConfigException. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28536/#review63295 --- On Nov. 30, 2014, 10:35 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28536/ --- (Updated Nov. 30, 2014, 10:35 a.m.) Review request for kafka. Bugs: KAFKA-1799 https://issues.apache.org/jira/browse/KAFKA-1799 Repository: kafka Description --- Explicit string to class conversion done in gAbstractConfig.getConfiguredInstances(), Addrresing Jun's comments Diffs - clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 8d886105341555a548ecc7b2901e7fc5d6b1ee8c clients/src/test/java/org/apache/kafka/common/config/AbstractConfigTest.java PRE-CREATION Diff: https://reviews.apache.org/r/28536/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 28536: Patch for KAFKA-1799
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28536/ --- Review request for kafka. Bugs: KAFKA-1799 https://issues.apache.org/jira/browse/KAFKA-1799 Repository: kafka Description --- Explicit string to class conversion done in gAbstractConfig.getConfiguredInstances() Diffs - clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 8d886105341555a548ecc7b2901e7fc5d6b1ee8c Diff: https://reviews.apache.org/r/28536/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 27684: Patch for KAFKA-1743
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27684/ --- (Updated Nov. 18, 2014, 5:29 a.m.) Review request for kafka. Bugs: KAFKA-1743 https://issues.apache.org/jira/browse/KAFKA-1743 Repository: kafka Description --- def commitOffsets method added to make ConsumerConnector backward compatible; Addressing Jun's comments Diffs (updated) - core/src/main/scala/kafka/consumer/ConsumerConnector.scala 07677c1c26768ef9c9032626180d0015f12cb0e0 core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala fe9d8e028cf08db844f0d72de4dd1e78f0e4258c core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 1f98db5d692adc113189ec8c75a4fad29d6b6ffe core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala bad099a904967651bc3a38b6bb9a9cdb592b832b Diff: https://reviews.apache.org/r/27684/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 27684: Patch for KAFKA-1743
On Nov. 18, 2014, 2:49 a.m., Jun Rao wrote: Thanks for the patch. Got the following compilation error. :core:compileTestScala/Users/junrao/intellij/kafka/core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala:116: overloaded method value commitOffsets with alternatives: = Unit and (isAutoCommit: Boolean)Unit cannot be applied to () zkConsumerConnector1.commitOffsets() ^ /Users/junrao/intellij/kafka/core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala:204: overloaded method value commitOffsets with alternatives: = Unit and (isAutoCommit: Boolean)Unit cannot be applied to () zkConsumerConnector1.commitOffsets() ^ two errors found FAILED Oh My Bad! missed the test classes. Pl review the latest patch. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27684/#review61863 --- On Nov. 18, 2014, 5:29 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27684/ --- (Updated Nov. 18, 2014, 5:29 a.m.) Review request for kafka. Bugs: KAFKA-1743 https://issues.apache.org/jira/browse/KAFKA-1743 Repository: kafka Description --- def commitOffsets method added to make ConsumerConnector backward compatible; Addressing Jun's comments Diffs - core/src/main/scala/kafka/consumer/ConsumerConnector.scala 07677c1c26768ef9c9032626180d0015f12cb0e0 core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala fe9d8e028cf08db844f0d72de4dd1e78f0e4258c core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 1f98db5d692adc113189ec8c75a4fad29d6b6ffe core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala bad099a904967651bc3a38b6bb9a9cdb592b832b Diff: https://reviews.apache.org/r/27684/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 27684: Patch for KAFKA-1743
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27684/ --- (Updated Nov. 16, 2014, 6:42 a.m.) Review request for kafka. Bugs: KAFKA-1743 https://issues.apache.org/jira/browse/KAFKA-1743 Repository: kafka Description (updated) --- def commitOffsets method added to make ConsumerConnector backward compatible; Addressing Jun's comments Diffs (updated) - core/src/main/scala/kafka/consumer/ConsumerConnector.scala 07677c1c26768ef9c9032626180d0015f12cb0e0 core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala f476973eeff653473a60c3ecf36e870e386536bc core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 1f98db5d692adc113189ec8c75a4fad29d6b6ffe Diff: https://reviews.apache.org/r/27684/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 27684: Patch for KAFKA-1743
On Nov. 14, 2014, 7:29 p.m., Jun Rao wrote: Thanks for the patch. For clarity, in ZookeeperConsumerConnector, instead of having the following, def commitOffsets(isAutoCommit: Boolean = true) could we break it into two separate methods, same as what's defined in ConsumerConnector? Done. Added the following APIs in ZookeeperConsumerConnector. def commitOffsets(retryOnFailure: Boolean) def commitOffsets - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27684/#review61486 --- On Nov. 16, 2014, 6:42 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27684/ --- (Updated Nov. 16, 2014, 6:42 a.m.) Review request for kafka. Bugs: KAFKA-1743 https://issues.apache.org/jira/browse/KAFKA-1743 Repository: kafka Description --- def commitOffsets method added to make ConsumerConnector backward compatible; Addressing Jun's comments Diffs - core/src/main/scala/kafka/consumer/ConsumerConnector.scala 07677c1c26768ef9c9032626180d0015f12cb0e0 core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala f476973eeff653473a60c3ecf36e870e386536bc core/src/main/scala/kafka/javaapi/consumer/ZookeeperConsumerConnector.scala 1f98db5d692adc113189ec8c75a4fad29d6b6ffe Diff: https://reviews.apache.org/r/27684/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 27684: Patch for KAFKA-1743
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27684/ --- (Updated Nov. 14, 2014, 5 p.m.) Review request for kafka. Bugs: KAFKA-1743 https://issues.apache.org/jira/browse/KAFKA-1743 Repository: kafka Description (updated) --- def commitOffsets method added to make ConsumerConnector backward compatible; Adressing Jun's comments Diffs (updated) - core/src/main/scala/kafka/consumer/ConsumerConnector.scala 07677c1c26768ef9c9032626180d0015f12cb0e0 Diff: https://reviews.apache.org/r/27684/diff/ Testing --- Thanks, Manikumar Reddy O
Re: Review Request 27684: Patch for KAFKA-1743
On Nov. 10, 2014, 7:50 p.m., Jun Rao wrote:
core/src/main/scala/kafka/consumer/ConsumerConnector.scala, lines 76-80
https://reviews.apache.org/r/27684/diff/2/?file=755292#file755292line76
We will also need to change the interface in ConsumerConnector from
def commitOffsets(retryOnFailure: Boolean = true)
back to
def commitOffsets
In ZookeeperConsumerconnector, we can make the following method private
def commitOffsets(retryOnFailure: Boolean = true)
Another question, will scala compiler be confused with 2 methods, one
w/o parenthsis and one with 1 parameter having a default? Could you try
compiling the code on all scala versions?
Manikumar Reddy O wrote:
Currently below classes uses the new method commitOffsets(true).
kafka/javaapi/consumer/ZookeeperConsumerConnector.scala
kafka/tools/TestEndToEndLatency.scala
If we are changing the interface, then we need to change the above
classes
also.
If we are not fixing this on trunk, then same problem will come in 0.8.3.
How to handle this?
2 methods, one w/o parenthsis and one with 1 parameter is getting
compiled on
all scala versions.
Jun Rao wrote:
Thanks for the explanation. There is actually a bit of inconsistency
introduced in this patch.
In kafka.javaapi.consumer.ZookeeperConsumerConnector, commitOffsets() is
implemented as the following.
def commitOffsets() {
underlying.commitOffsets()
}
This actually calls underlying.commitOffsets(isAutoCommit: Boolean =
true) with a default value of true. However, ConsumerConnector.commitOffset
is implemented as the following which sets isAutoCommit to false.
def commitOffsets { commitOffsets(false) }
So, we should use true in the above.
Another thing that I was thinking is that it's going to be a bit
confusing if we have the following scala apis.
def commitOffsets(retryOnFailure: Boolean = true)
def commitOffsets
So, if you do commitOffset it calls the second one and if you do
commitOffset(), you actually call the first one. However, the expectation is
probably the same method will be called in both cases. Would it be better if
we get rid of the default like the following? Then, it's clear which method
will be called.
def commitOffsets(retryOnFailure: Boolean)
def commitOffsets
Manikumar Reddy O wrote:
This JIRA is to make ConsumerConnecor compatible with 0.8.1, right?
then, we need to remove
def commitOffsets(retryOnFailure: Boolean = true) from ConsumerConnecor.
Changing the API to def commitOffsets(retryOnFailure: Boolean) will not
help us.
It still breaks the compatability.
Jun Rao wrote:
In 0.8.1, ConsumerConnector has
def commitOffsets
I was thinking of having the following two APIs in ConsumerConnector in
0.8.2. That should be backward compatible with the 0.8.1 api, right?
def commitOffsets(retryOnFailure: Boolean)
def commitOffsets
Ok. I was thinking there may be some custom implementations of
ConsumerConnector interface out side the kafka codebase. So changing the
interface will break those implementations.
I added the following APIs in ConsumerConnector.
def commitOffsets(retryOnFailure: Boolean)
def commitOffsets
- Manikumar Reddy
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27684/#review60652
---
On Nov. 14, 2014, 5 p.m., Manikumar Reddy O wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27684/
---
(Updated Nov. 14, 2014, 5 p.m.)
Review request for kafka.
Bugs: KAFKA-1743
https://issues.apache.org/jira/browse/KAFKA-1743
Repository: kafka
Description
---
def commitOffsets method added to make ConsumerConnector backward
compatible; Adressing Jun's comments
Diffs
-
core/src/main/scala/kafka/consumer/ConsumerConnector.scala
07677c1c26768ef9c9032626180d0015f12cb0e0
Diff: https://reviews.apache.org/r/27684/diff/
Testing
---
Thanks,
Manikumar Reddy O
Re: Review Request 27684: Patch for KAFKA-1743
On Nov. 10, 2014, 7:50 p.m., Jun Rao wrote:
core/src/main/scala/kafka/consumer/ConsumerConnector.scala, lines 76-80
https://reviews.apache.org/r/27684/diff/2/?file=755292#file755292line76
We will also need to change the interface in ConsumerConnector from
def commitOffsets(retryOnFailure: Boolean = true)
back to
def commitOffsets
In ZookeeperConsumerconnector, we can make the following method private
def commitOffsets(retryOnFailure: Boolean = true)
Another question, will scala compiler be confused with 2 methods, one
w/o parenthsis and one with 1 parameter having a default? Could you try
compiling the code on all scala versions?
Manikumar Reddy O wrote:
Currently below classes uses the new method commitOffsets(true).
kafka/javaapi/consumer/ZookeeperConsumerConnector.scala
kafka/tools/TestEndToEndLatency.scala
If we are changing the interface, then we need to change the above
classes
also.
If we are not fixing this on trunk, then same problem will come in 0.8.3.
How to handle this?
2 methods, one w/o parenthsis and one with 1 parameter is getting
compiled on
all scala versions.
Jun Rao wrote:
Thanks for the explanation. There is actually a bit of inconsistency
introduced in this patch.
In kafka.javaapi.consumer.ZookeeperConsumerConnector, commitOffsets() is
implemented as the following.
def commitOffsets() {
underlying.commitOffsets()
}
This actually calls underlying.commitOffsets(isAutoCommit: Boolean =
true) with a default value of true. However, ConsumerConnector.commitOffset
is implemented as the following which sets isAutoCommit to false.
def commitOffsets { commitOffsets(false) }
So, we should use true in the above.
Another thing that I was thinking is that it's going to be a bit
confusing if we have the following scala apis.
def commitOffsets(retryOnFailure: Boolean = true)
def commitOffsets
So, if you do commitOffset it calls the second one and if you do
commitOffset(), you actually call the first one. However, the expectation is
probably the same method will be called in both cases. Would it be better if
we get rid of the default like the following? Then, it's clear which method
will be called.
def commitOffsets(retryOnFailure: Boolean)
def commitOffsets
This JIRA is to make ConsumerConnecor compatible with 0.8.1, right? then, we
need to remove
def commitOffsets(retryOnFailure: Boolean = true) from ConsumerConnecor.
Changing the API to def commitOffsets(retryOnFailure: Boolean) will not help
us.
It still breaks the compatability.
- Manikumar Reddy
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27684/#review60652
---
On Nov. 8, 2014, 6:20 a.m., Manikumar Reddy O wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27684/
---
(Updated Nov. 8, 2014, 6:20 a.m.)
Review request for kafka.
Bugs: KAFKA-1743
https://issues.apache.org/jira/browse/KAFKA-1743
Repository: kafka
Description
---
def commitOffsets method added to make ConsumerConnector backward compatible
Diffs
-
core/src/main/scala/kafka/consumer/ConsumerConnector.scala
07677c1c26768ef9c9032626180d0015f12cb0e0
Diff: https://reviews.apache.org/r/27684/diff/
Testing
---
Thanks,
Manikumar Reddy O
Re: Review Request 27684: Patch for KAFKA-1743
On Nov. 10, 2014, 7:50 p.m., Jun Rao wrote: core/src/main/scala/kafka/consumer/ConsumerConnector.scala, lines 76-80 https://reviews.apache.org/r/27684/diff/2/?file=755292#file755292line76 We will also need to change the interface in ConsumerConnector from def commitOffsets(retryOnFailure: Boolean = true) back to def commitOffsets In ZookeeperConsumerconnector, we can make the following method private def commitOffsets(retryOnFailure: Boolean = true) Another question, will scala compiler be confused with 2 methods, one w/o parenthsis and one with 1 parameter having a default? Could you try compiling the code on all scala versions? Currently below classes uses the new method commitOffsets(true). kafka/javaapi/consumer/ZookeeperConsumerConnector.scala kafka/tools/TestEndToEndLatency.scala If we are changing the interface, then we need to change the above classes also. If we are not fixing this on trunk, then same problem will come in 0.8.3. How to handle this? 2 methods, one w/o parenthsis and one with 1 parameter is getting compiled on all scala versions. - Manikumar Reddy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27684/#review60652 --- On Nov. 8, 2014, 6:20 a.m., Manikumar Reddy O wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27684/ --- (Updated Nov. 8, 2014, 6:20 a.m.) Review request for kafka. Bugs: KAFKA-1743 https://issues.apache.org/jira/browse/KAFKA-1743 Repository: kafka Description --- def commitOffsets method added to make ConsumerConnector backward compatible Diffs - core/src/main/scala/kafka/consumer/ConsumerConnector.scala 07677c1c26768ef9c9032626180d0015f12cb0e0 Diff: https://reviews.apache.org/r/27684/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 27722: Patch for KAFKA-1749
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27722/ --- Review request for kafka. Bugs: KAFKA-1749 https://issues.apache.org/jira/browse/KAFKA-1749 Repository: kafka Description --- Removed testComplexCompressDecompress from MessageCompressionTest class Diffs - core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala 0bb275d0dc840e40289e488b1a00aca2e26fe6f9 Diff: https://reviews.apache.org/r/27722/diff/ Testing --- Thanks, Manikumar Reddy O
Review Request 27723: Patch for KAFKA-1739
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27723/ --- Review request for kafka. Bugs: KAFKA-1739 https://issues.apache.org/jira/browse/KAFKA-1739 Repository: kafka Description --- Removed testComplexCompressDecompress from MessageCompressionTest class Diffs - core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala 0bb275d0dc840e40289e488b1a00aca2e26fe6f9 Diff: https://reviews.apache.org/r/27723/diff/ Testing --- Thanks, Manikumar Reddy O
