[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-213468155 @jianbzhou Thanks for the catch and for suggesting a fix. I was taking a look and the best way to fix this is to replace the `TreeMap` with a `HashMap`, i.e. do `final Set tps = new HashSet<>();`. `TopicPartition` implements `equals()` and `hashCode()` so if you change the line as suggested nothing else needs to be done. Ordering is not important here, so `HashMap` is indeed the most appropriate data structure here, and the one I meant to use to begin with. The appropriate way to do code contributions is to create a Pull Request and push it for review by the community. Please find the documentation on how to do so in here: * http://storm.apache.org/contribute/Contributing-to-Storm.html * https://github.com/apache/storm/blob/master/DEVELOPER.md#contribute-code * https://github.com/apache/storm/blob/master/DEVELOPER.md#pull-requests I am in the process of submitting a patch with an updated README and a few more things. Since this is such a small fix, I can include this fix there, and mention that the credit should go to you. However, if you with to create the pull request yourself to receive the deserved credit, please go ahead and I will review it right away. Please let me know and thanks once again. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1725: Kafka Spout New Consumer API - Kaf...
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1357 STORM-1725: Kafka Spout New Consumer API - KafkaSpoutRetryExponential Backoff method should use HashMap instead of TreeMap not to throw Exception - Replace TreeMap with HashMap You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache 1.x-branch_STORM-1725 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1357.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1357 commit 701a9045f62e3a61f7021f79a765ea89c1afa69d Author: Hugo Louro Date: 2016-04-22T17:16:31Z STORM-1725: Kafka Spout New Consumer API - KafkaSpoutRetryExponentialBackoff method should use HashMap instead of TreeMap not to throw Exception - Replace TreeMap with HashMap --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1725: Kafka Spout New Consumer API - Kaf...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1357#issuecomment-213522166 This issue was found by @jianbzhou and credit should be given to him (for discussion details see https://github.com/apache/storm/pull/1131#issuecomment-213211034) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-213522367 @jianbzhou done! Please see https://github.com/apache/storm/pull/1357. Thanks once again. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1725: Kafka Spout New Consumer API - Kaf...
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1382 STORM-1725: Kafka Spout New Consumer API - KafkaSpoutRetryExponentialBackoff method should use HashMap instead of TreeMap not to throw Exception - Replace TreeMap with HashMap You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache master_STORM-1725 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1382.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1382 commit 3702df4abfcad3537847f5aa957aaf243c1860a0 Author: Hugo Louro Date: 2016-04-22T17:16:31Z STORM-1725: Kafka Spout New Consumer API - KafkaSpoutRetryExponentialBackoff method should use HashMap instead of TreeMap not to throw Exception - Replace TreeMap with HashMap --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1725: Kafka Spout New Consumer API - Kaf...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1382#issuecomment-215794086 This is a cherry-pick of the commit that went in into the 1.x-branch --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: KafkaSpout README for storm-kafka-client (Kafk...
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1383 KafkaSpout README for storm-kafka-client (Kafka new Consumer API) You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache 1.x-branch_KafkaSpoutREADME Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1383.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1383 commit a46f0159b6261ffa7ed77760244866d4681a Author: Hugo Louro Date: 2016-04-29T19:05:17Z KafkaSpout README for storm-kafka-client (Kafka new Consumer API) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: KafkaSpout README for storm-kafka-client (Kafk...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1383#issuecomment-215857762 @ptgoetz @harshach can you please review. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1761: Storm-Solr Example Throws ArrayInd...
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1394 STORM-1761: Storm-Solr Example Throws ArrayIndexOutOfBoundsException in Remote Cluster Mode Please cherry-pick this patch into 0.10.x-branch and 1.x-branch as well. You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache master_STORM-1761 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1394.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1394 commit 514b932c2f2b643e6865fd82bc6d720acecfa301 Author: Hugo Louro Date: 2016-05-03T23:32:45Z STORM-1761: Storm-Solr Example Throws ArrayIndexOutOfBoundsException in Remote Cluster Mode --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-217894464 @jianbzhou thanks for your feedback. Let me take a look at this and I will get back to you shortly. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1834: Documentation How to Generate Cert...
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1414 STORM-1834: Documentation How to Generate Certificates For Local Testing SSL Setup This patch must be cherry picked in 0.10.x-branch, 1.x-branch, and master You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache 0.10.x-branch_EAR-3561_UI_HTTPS_Docs Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1414.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1414 commit 3df00e0cd5055b60893bcdaf652884b7d29b5811 Author: Hugo Louro Date: 2016-05-13T22:46:50Z STORM-1834: Documentation How to Generate Certificates For Local Testing SSL Setup --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1838 update OffsetEntry when onPartition...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1419#issuecomment-219533584 @flisky can you please share the code, or at least the steps to reproduce the scenario where this code does not work? Another alternative is that you enable all the logging levels for us to understand what is happening. That could should be necessary, and I am not very comfortable just removing it without understanding what is going on. Without that code, I believe that the scenario where a consumer rebalance occurs, and offsets 1,2 have been hacked, but 3 is still pending, and 4, 5 have also been acked, won't work. That code is there to cover for this scenario. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: fix typo in worker.clj
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1418#issuecomment-219559024 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1838 update OffsetEntry to support EARLI...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1419#issuecomment-219788238 @flisky so what does your last comment mean? That this patch no longer applies? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1838 update OffsetEntry to support EARLI...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1419#issuecomment-220102668 @flisky can you please provide specific examples or a code snippet that I can use to test what you are saying. Can you also post the logs (perhaps on JIRA) that show the cases that are failing? What do you mean by `offset out of range`? As for your first comment, please keep in mind that the `FirstPollOffsetStrategy`, as the name indicates, it's only for the first poll, i.e., the first time a `KafkaSpout` polls from Kafka. It does not apply to consumer rebalance. Quoting the [javadoc](https://goo.gl/PyR7Cm) of "The offset used by the Kafka spout in the first poll to Kafka broker." Therefore, the `LATEST/EARLIEST` strategies only apply in the first poll, not on every consumer rebalance. Furthermore, if a consumer rebalance occurs, but the same partition is assigned to this spout, and some offsets have already been emitted and acked, we wan to keep those, and only process the ones that haven't yet been acked because they have have failed and waiting to be retried (according to exponential backoff) or may be on the "wire". What do you mean by -2 , -1? Can you please give specific examples. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1848: Make KafkaMessageId and Partition ...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1428#issuecomment-220105450 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-220347788 @jianbzhou thanks once again for your feedback. Can you please share the results of your tests? I am working on adding more tests to this, as well as fix some of these corner cases, possibly using some of your suggested solutions, if they are the most suitable. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1850: State Checkpointing Documentation ...
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1433#discussion_r63924300 --- Diff: docs/State-checkpointing.md --- @@ -94,6 +94,9 @@ is saved and then the checkpoint tuple is forwarded to the next component. Each streams before it saves its state so that the state represents a consistent state across the topology. Once the checkpoint spout receives ACK from all the bolts, the state commit is complete and the transaction is recorded as committed by the checkpoint spout. +The state checkpointing does not currently checkpoint the state of the spout. Yet, once the states of all bolts are checkpointed and once the checkpoint tuples is acked, the tuples emitted by the spout are acked. +It also implies that `topology.state.checkpoint.interval.ms` is lower than `topology.message.timeout.secs`. --- End diff -- once the *STATE* of all the bolts are checkpointed, and once the checkpoint tuples *ARE* acked by the spout are *ALSO* acked. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1850: State Checkpointing Documentation ...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1433#issuecomment-220402065 +1 once some minor grammar suggestions have been addressed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1845: use UTF-8 instead of default encod...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1422#issuecomment-220407078 @vesense what is the motivation behind this change? Prior to your proposed change, the charset used is the one that is defined by the java system property `file.encoding`. If this property is absent the file encoding used will be `UTF-8` in JDK8. My understanding is that your proposed change will eliminate the chance to customize an arbitrary charset using `file.encoding`. Is this the goal? If the goal is to set `UTF-8` by default, perhaps once should simply set in a configuration file the property `file.encoding=UTF-8` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1861 Fix storm script bug to not fork ja...
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1441#discussion_r64300845 --- Diff: bin/storm.py --- @@ -241,7 +240,6 @@ def jar(jarfile, klass, *args): extrajars=[tmpjar, USER_CONF_DIR, STORM_BIN_DIR], args=args, daemon=False, --- End diff -- @revans2 can you please take a look and let us know if there is any special reason for this process to be forked? This process being forked affects the return code. If there is no reason for this process to be forked, this fix will correct the return code. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-1851] Fix default nimbus impersonation ...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1435#issuecomment-221141811 +1 @arunmahadevan perhaps you can update the docs stating what you say in your comment as it will bring awareness to the users on what is going on by default, as well as setup impersonation. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-221143192 @jianbzhou can you please email me what you have such that we can provide with a fix? Is there a way you can share the kafka setup that causes the issues that you mention ? I should upload my test cases later today, and that should help us address any possible issues. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1838 update OffsetEntry to support EARLI...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1419#issuecomment-221145807 @flisky I am looking at this now --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-221161594 @jianbzhou please email me to  --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1861 Fix storm script bug to not fork ja...
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1441#discussion_r64604305 --- Diff: bin/storm.py --- @@ -241,7 +240,6 @@ def jar(jarfile, klass, *args): extrajars=[tmpjar, USER_CONF_DIR, STORM_BIN_DIR], args=args, daemon=False, --- End diff -- +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-221928986 @jianbzhou looking at it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-1861 Fix storm script bug to not fork ja...
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1448#issuecomment-222035081 +1. @csivaguru can you please squash the commits. It will keep the log a bit cleaner --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-78070 @jianbzhou I confirm that your suggested fix for doSeekRetriableTopicPartitions is correct. I am going to include that in the next patch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-78660 @connieyang I am finishing addressing some issues brought up by the initial users of this kafka spout, as well as unit test coverage, and will push the trident API right after. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131#issuecomment-80056 @jianbzhou any updates on ``` One customer of us who also use the spout they found some other issues: 1. Work load is not distributed to all spout tasks(as per the storm topology) 2. No progress on some partitions (as per the log) 3. No commit on some partitions (as per the log) I will start looking into that tomorrow once I get the detail log file. Also if you have any clue on this please kindly advise.``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the pull request: https://github.com/apache/storm/pull/1131 @jianbzhou **Storm** guarantees that all the messages are either acked or failed. There is the property "topology.message.timeout.secs" https://github.com/apache/storm/blob/master/storm-core/src/jvm/org/apache/storm/Config.java#L1669 If Storm is configured to use acks, and the acks don't arrive in a certain amount of time, the tuple will be retired. You don't have to worry about the scenario you described, and basically implement the timeout yourself. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1352: STORM-1723 (1.x) Introduce ClusterMetricsConsumer
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1352 +1. I am also in favor of making cluster summary modular. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1419: STORM-1838 update OffsetEntry to support EARLIEST/LATEST ...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1419 @flisky some stuff came up. Apologies for the delay in my response. I will get back to you today or tomorrow as I am working on this now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1131: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1131 @jianbzhou thanks. Looking at it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #1451: STORM-1136: Command line module to return kafka sp...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/1451#discussion_r65576093
--- Diff:
external/storm-kafka-monitor/src/main/java/org/apache/storm/kafka/monitor/KafkaOffsetLagUtil.java
---
@@ -0,0 +1,374 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.storm.kafka.monitor;
+
+import kafka.api.OffsetRequest;
+import kafka.api.PartitionOffsetRequestInfo;
+import kafka.common.TopicAndPartition;
+import kafka.javaapi.OffsetResponse;
+import kafka.javaapi.consumer.SimpleConsumer;
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.CommandLineParser;
+import org.apache.commons.cli.DefaultParser;
+import org.apache.commons.cli.HelpFormatter;
+import org.apache.commons.cli.Options;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.retry.RetryOneTime;
+import org.apache.kafka.clients.consumer.KafkaConsumer;
+import org.apache.kafka.clients.consumer.OffsetAndMetadata;
+import org.apache.kafka.common.PartitionInfo;
+import org.apache.kafka.common.TopicPartition;
+import org.json.simple.JSONValue;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+
+/**
+ * Utility class for querying offset lag for kafka spout
+ */
+public class KafkaOffsetLagUtil {
+private static final String OPTION_TOPIC_SHORT = "t";
--- End diff --
what is the difference between SHORT and LONG ? why t vs topics ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1451: STORM-1136: Command line module to return kafka sp...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/1451#discussion_r65576345
--- Diff:
external/storm-kafka-monitor/src/main/java/org/apache/storm/kafka/monitor/KafkaOffsetLagUtil.java
---
@@ -0,0 +1,374 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.storm.kafka.monitor;
+
+import kafka.api.OffsetRequest;
+import kafka.api.PartitionOffsetRequestInfo;
+import kafka.common.TopicAndPartition;
+import kafka.javaapi.OffsetResponse;
+import kafka.javaapi.consumer.SimpleConsumer;
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.CommandLineParser;
+import org.apache.commons.cli.DefaultParser;
+import org.apache.commons.cli.HelpFormatter;
+import org.apache.commons.cli.Options;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.retry.RetryOneTime;
+import org.apache.kafka.clients.consumer.KafkaConsumer;
+import org.apache.kafka.clients.consumer.OffsetAndMetadata;
+import org.apache.kafka.common.PartitionInfo;
+import org.apache.kafka.common.TopicPartition;
+import org.json.simple.JSONValue;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+
+/**
+ * Utility class for querying offset lag for kafka spout
+ */
+public class KafkaOffsetLagUtil {
+private static final String OPTION_TOPIC_SHORT = "t";
+private static final String OPTION_TOPIC_LONG = "topics";
+private static final String OPTION_OLD_CONSUMER_SHORT = "o";
+private static final String OPTION_OLD_CONSUMER_LONG = "old-spout";
+private static final String OPTION_BOOTSTRAP_BROKERS_SHORT = "b";
+private static final String OPTION_BOOTSTRAP_BROKERS_LONG =
"bootstrap-brokers";
+private static final String OPTION_GROUP_ID_SHORT = "g";
+private static final String OPTION_GROUP_ID_LONG = "groupid";
+private static final String OPTION_TOPIC_WILDCARD_SHORT = "w";
+private static final String OPTION_TOPIC_WILDCARD_LONG =
"wildcard-topic";
+private static final String OPTION_PARTITIONS_SHORT = "p";
+private static final String OPTION_PARTITIONS_LONG = "partitions";
+private static final String OPTION_LEADERS_SHORT = "l";
+private static final String OPTION_LEADERS_LONG = "leaders";
+private static final String OPTION_ZK_SERVERS_SHORT = "z";
+private static final String OPTION_ZK_SERVERS_LONG = "zk-servers";
+private static final String OPTION_ZK_COMMITTED_NODE_SHORT = "n";
+private static final String OPTION_ZK_COMMITTED_NODE_LONG = "zk-node";
+private static final String OPTION_ZK_BROKERS_ROOT_SHORT = "r";
+private static final String OPTION_ZK_BROKERS_ROOT_LONG =
"zk-brokers-root-node";
+
+public static void main (String args[]) {
+try {
+List results;
+Options options = buildOptions();
+CommandLineParser parser = new DefaultParser();
+CommandLine commandLine = parser.parse(options, args);
+if (!commandLine.hasOption(OPTION_TOPIC_LONG)) {
+printUsageAndExit(options, OPTION_TOPIC_LONG + " is
required");
+}
+if (commandLine.hasOption(OPTION_OLD_CONSUMER_LONG)) {
+OldKafkaSpoutOffsetQuery oldKafkaSpoutOffsetQuery;
+if (commandLine.hasOption(OPTION_GROUP_ID_LONG) ||
commandLine.hasOption(OPTION_BOOTSTRAP_BROKERS_LONG)) {
+printUsageAndExit(options, OPTION_GROUP_ID_LONG + " or
" + OPTION_
[GitHub] storm pull request #1491: STORM-1907: PartitionedTridentSpoutExecutor has in...
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1491 STORM-1907: PartitionedTridentSpoutExecutor has incompatible types that cause ClassCastException - Generify on Object rather than Integer because example is broken - Fix TridentWordCount example to print more meaningful output You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache master_STORM-1907 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1491.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1491 commit aae0acbe8206f66129443d78831883226f8144c9 Author: Hugo Louro Date: 2016-06-16T00:49:30Z STORM-1907: PartitionedTridentSpoutExecutor has incompatible types that cause ClassCastException - Generify on Object rather than Integer because example is broken - Fix TridentWordCount example to print more meaningful output --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1491: STORM-1907: PartitionedTridentSpoutExecutor has incompati...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1491 This PR is related to https://github.com/apache/storm/pull/1429 but I found this issue independently, in my local branch, which was a bit outdated. This patch still makes sense since it gets rid of five warnings that are caused by having ITridentSpout.BatchCoordinator as raw type. Furthermore, it also prints more the actual word count output in the example in file `TridentWordCount` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1488: STORM-1906: Window count/length of zero should be disallo...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1488 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1431: STORM-1849: HDFSFileTopology should use the third argumen...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1431 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1487: off-by-one error for UNCOMMITTED_LATEST and UNCOMMITTED_E...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1487 @nabhanelrahman I am addressing this and other edge cases in a patch that I am about to submit. I am afraid that just removing the +1 won't work for all the cases. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1474: support for wildcard topics ( storm-kafka-client )
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1474 @harshach @nabhanelrahman looking at this now. Somehow I missed this PR earlier when going throw the PRs for reviewing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #1516: STORM-1930: Kafka New Client API - Support for Top...
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1516 STORM-1930: Kafka New Client API - Support for Topic Wildcards - Interfaces for TuplesBuilder and KafkaSpoutStreams and implementations for named topics and wildcards - Test topologies for named topics and wildcards You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache Apache_master_STORM-1930_KafkaWildcards Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1516.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1516 commit 754da7f61e2aef46c64a07f4b7e8cedc9161499a Author: Hugo Louro Date: 2016-06-23T20:07:57Z STORM-1930: Kafka New Client API - Support for Topic Wildcards - Interfaces for TuplesBuilder and KafkaSpoutStreams and implementations for named topics and wildcards - Test topologies for named topics and wildcards --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1516: STORM-1930: Kafka New Client API - Support for Topic Wild...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1516 @harshach Should this patch be merged onto 1.x-branch as well. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1474: support for wildcard topics ( storm-kafka-client )
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1474 @nabhanelrahman your patch enlightened that it would be better to add the `emit` and `getOutputFields` methods to `KafkaSpoutStream`. Furthermore, there were a couple of design decisions that would be ideal to do at this point in order to reuse code, and keep it clean and modular. Since it would be too verbose to go over those discussions in here, I just created a pull request with the changes that I believe are adequate to address the wildcard topic pattern requirement. This is the pull request: https://github.com/apache/storm/pull/1516 Although I had this in my todo list, since you already worked on it, it's totally fine for you to get the credit for it. Hence feel free to assign the JIRA to you, and recommit the PR with your username. Please mention that we worked on this together, and it's all good. Thanks for your understanding. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #1556: STORM-1737: storm-kafka-client has compilation err...
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1556 STORM-1737: storm-kafka-client has compilation errors with Apache Kafka 0.10 You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache STORM-1737 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1556.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1556 commit 04f00e56beb9799dc8b8fc732271b786ced5c21e Author: Hugo Louro Date: 2016-04-29T16:18:05Z STORM-1737: storm-kafka-client has compilation errors with Apache Kafka 0.10 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #1562: TridentHiveTopology fails to run due to incompatib...
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1562 TridentHiveTopology fails to run due to incompatibility between thrift dependencies - hive-hcatalog-streaming:jar depends on thrift 0.9.3 - storm-hive:jar depends on thrift 0.9.0 - Update storm-hive thrift version to 0.9.3 You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache Apache_master_BUG-60812_TridentHiveTopolgy_Thrift Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1562.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1562 commit 5df06bf523754d2bae35c23c24a6c6ffb99e4d9f Author: Hugo Louro Date: 2016-07-13T07:04:42Z TridentHiveTopology fails to run due to incompatibility between thrift dependencies - hive-hcatalog-streaming:jar depends on thrift 0.9.3 - storm-hive:jar depends on thrift 0.9.0 - Update storm-hive thrift version to 0.9.3 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1562: TridentHiveTopology fails to run due to incompatibility b...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1562 This fix likely has to be backported to 1.x-branch --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1482: STORM-1876: Option to build storm-kafka and storm-kafka-c...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1482 +1 besides the neat pick which would be to have the new spout property names prefixed by `storm.kafka.client` property names for consistency. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #1482: STORM-1876: Option to build storm-kafka and storm-...
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/1482#discussion_r70818175 --- Diff: pom.xml --- @@ -258,6 +258,12 @@ 1.4.0-incubating 2.6.3 2.18.1 + + +0.8.2.1 +kafka_2.10 + + --- End diff -- @abhishekagarwal87 can the property names be a bit more consistent? A suggestion would be to prefix the properties used by `storm.kafka.client`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1556: STORM-1737: storm-kafka-client has compilation errors wit...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1556 @harshach @HeartSaVioR @abhishekagarwal87, thank you for your feedback. The updated ReadME, including the added Wildcard Support will be out in a few mins. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #1576: Kafka Spout New Consumer API - KafkaSpoutRetryExpo...
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1576 Kafka Spout New Consumer API - KafkaSpoutRetryExponentialBackoff Bug - KafkaSpoutRetryExponentialBackoff#nextTime(...) method is off by order of magnitude You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache Apache_master_07.19.2016_KSExpnBackoff Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1576.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1576 commit 3fa2283007c66762db2021d3c0766677f5f7c8b7 Author: Hugo Louro Date: 2016-07-19T22:03:33Z Kafka Spout New Consumer API - KafkaSpoutRetryExponentialBackoff#nextTime(...) method is off by order of magnitude --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1576: Kafka Spout New Consumer API - KafkaSpoutRetryExponential...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1576 Please merge this into 1.x-branch as well --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1576: Kafka Spout New Consumer API - KafkaSpoutRetryExponential...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1576 @harshach @priyank5485 can you please review. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1576: Kafka Spout New Consumer API - KafkaSpoutRetryExponential...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1576 @priyank5485 thanks for catch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1587: [STORM-1991] Support auto.commit.interval in Kafka Client
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1587 @darionyaphet can you please clarify why this patch is needed? In my understanding it shouldn't be necessary to have any logic for auto commit interval. If one wishes to set the auto commit interval, one can do so using the kafka properties `autocommit.interval.ms` and `autocommit.enable`. These properties will then be passed the `KafkaConsumer` [here](https://github.com/apache/storm/blob/master/external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java#L350) The [commit timer](https://github.com/apache/storm/blob/master/external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java#L111) is only used when in non auto commit mode. The 500 is only the initial delay. One can make this initial delay configurable, but it may not be of extreme importance to do so. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1587: [STORM-1991] Support auto.commit.interval in Kafka Client
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1587 @darionyaphet this kafka spout does not use zookeeper as the new Kafka API writes the offsets to a topic rather than to zookeeper. As I mentioned in the initial comment, this change is not necessary. Furthermore, all the kafka properties are supposed to go in a map, as illustrated in [this example](https://github.com/apache/storm/blob/master/external/storm-kafka-client/src/test/java/org/apache/storm/kafka/spout/test/KafkaSpoutTopologyMainNamedTopics.java#L115-L123) At last, the commitTimer is not used in auto commit mode. I would like to suggest that you close this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1131: STORM-822: Kafka Spout New Consumer API
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1131 @jianbzhou you can get obtain at most once semantics by setting maxRetries to zero. Here is the method to do so. https://github.com/apache/storm/blob/master/external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java#L206 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #1649: STORM-2052: Kafka Spout - New Client API - Perform...
GitHub user hmcl opened a pull request: https://github.com/apache/storm/pull/1649 STORM-2052: Kafka Spout - New Client API - Performance Improvements This patch should be back-ported to 1.0.x branch and 1.x-branch. Thank you. You can merge this pull request into a Git repository by running: $ git pull https://github.com/hmcl/storm-apache Apache_master_STORM-2052_KSPI Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1649.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1649 commit b98db7b0fc29d4c12fb4ce24bd2210d3cb3bc087 Author: Hugo Louro Date: 2016-08-03T05:24:52Z STORM-2052: Kafka Spout - New Client API - Performance Improvements - Improve Logging - Log heavy objects with TRACE level - Make log messages more meaningful - Added log timer to throttle messages - Delete a couple of logs commit be748bd2c0df1e6cc33706ca48c3a3d6ffc26b5f Author: Hugo Louro Date: 2016-08-08T21:36:41Z STORM-2052: Kafka Spout - New Client API - Performance Improvements - Set default config values for good performance commit a81fa6e62bb4ae0f4fc01585ec3fc51015d236e0 Author: Hugo Louro Date: 2016-08-24T14:50:13Z STORM-2052: Kafka Spout - New Client API - Performance Improvements - Update README --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #1649: STORM-2052: Kafka Spout - New Client API - Perform...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/1649#discussion_r76275357
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -221,7 +224,22 @@ private boolean commit() {
}
private boolean poll() {
-return !waitingToEmit() && numUncommittedOffsets <
kafkaSpoutConfig.getMaxUncommittedOffsets();
+final int maxUncommittedOffsets =
kafkaSpoutConfig.getMaxUncommittedOffsets();
+final boolean poll = !waitingToEmit() && numUncommittedOffsets <
maxUncommittedOffsets;
+
+if (!poll) {
+final boolean isLoggable = logTimer.isExpiredResetOnTrue();
--- End diff --
This check is here in order to throttle the number of messages that get
printed. Although logs are with debug level, a lot of messages will get printed
(one each time nextTuple is called), which will spam the logs, and limit the
ability to debug. What this code does is to print one message per second,
rather than one per call to nextTuple. By doing this, no info is lost, and
things are much cleaner.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1649: STORM-2052: Kafka Spout - New Client API - Performance Im...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1649 @harshach I can squash the commits, but I left these three commits on purpose. Each commit is a logical groping. One is docs, one is the default values, and one is the logging changes. They all have the same JIRA ticket, but the commit log messages suit the changes. The reason behind this choice is that it is easier to cherry pick only part of the changes to a given branch - if needed. I also believe that it makes it easier for an external person to follow what each patch addresses. I am OK with either approach. This squash is pretty simple. Let me know if you prefer that this is indeed squashed, and I will do so. Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #1649: STORM-2052: Kafka Spout - New Client API - Performance Im...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1649 @harshach Done! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2155: STORM-2548: Simplify KafkaSpoutConfig to avoid duplicatin...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2155 +1. @harshach @priyank5485 can you please take one final look. If you don't have any objection, I suggest that we merge this patch in the next day or so. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2150: STORM-2541: Fix storm-kafka-client manual subscription no...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2150 +1. @harshach @priyank5485 can you please take one final look. If you don't have any objection, I suggest that we merge this patch in the next day or so. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2150: STORM-2541: Fix storm-kafka-client manual subscrip...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2150#discussion_r127769417
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/TopicFilter.java
---
@@ -0,0 +1,38 @@
+/*
+ * Copyright 2017 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.storm.kafka.spout;
+
+import java.io.Serializable;
+import java.util.List;
+import org.apache.kafka.clients.consumer.KafkaConsumer;
+import org.apache.kafka.common.TopicPartition;
+
+public interface TopicFilter extends Serializable {
+
+/**
+ * Get the Kafka TopicPartitions passed by this filter.
+ * @param consumer The Kafka consumer to use to read the list of
existing partitions
+ * @return The Kafka partitions passed by this filter.
+ */
+List getFilteredTopicPartitions(KafkaConsumer
consumer);
+
+/**
+ * @return A human-readable string representing the subscribed topics.
--- End diff --
NIT: representing the topics that pass the filter
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2151: STORM-2542: Remove KafkaConsumer.subscribe API opt...
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/2151#discussion_r127773597 --- Diff: docs/storm-kafka-client.md --- @@ -240,12 +240,9 @@ streams. If you are doing this for Trident a value must be in the List returned otherwise trident can throw exceptions. -### Manual Partition Control (ADVANCED) +### Manual Partition Assigment (ADVANCED) -By default Kafka will automatically assign partitions to the current set of spouts. It handles lots of things, but in some cases you may want to manually assign the partitions. -This can cause less churn in the assignments when spouts go down and come back up, but it can result in a lot of issues if not done right. This can all be handled by subclassing -Subscription and we have a few implementations that you can look at for examples on how to do this. ManualPartitionNamedSubscription and ManualPartitionPatternSubscription. Again -please be careful when using these or implementing your own. +By default the KafkaSpout instancs will be assigned partitions by round robin assignment. If you need to customize this assignment, you can implement the `ManualPartitioner` interface. The implementation can be passed to the `ManualPartitionSubscription` constructor, and the `Subscription` can then be set in the `KafkaSpoutConfig` via the `KafkaSpoutConfig.Builder` constructor. Please take care when supplying a custom implementation, since an incorrect `ManualPartitioner` implementation could leave some partitions unread, or concurrently read by multiple spout instances. See the `RoundRobinManualPartitioner` for an example of how to implement this functionality. --- End diff -- By default the KafkaSpout instances will be assigned partitions using a round robin strategy. If you need to customize partitions assignment, you must implement the `ManualPartitioner` interface. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2147: STORM-2538: New kafka spout emits duplicate tuples
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2147 @srdo @harshach @arunmahadevan in face of the Manual Partition assignment related [fixes](https://github.com/apache/storm/pull/2151/commits), there are two options. I believe in the email list we agreed to go with option #1, but I would like to make sure. 1 - We disregard and close this PR. If we do so, we are implicitly telling the user that if they decide to use dynamic partition assignment they are at their own responsibility and it won't work with multiple parallelism. Currently the only implementation we support is `ManualPartitionSubscription` 2 - We improve this code to make it work for the dynamic case. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2147: STORM-2538: New kafka spout emits duplicate tuples
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2147 Closed in favor of #2151 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2147: STORM-2538: New kafka spout emits duplicate tuples
Github user hmcl closed the pull request at: https://github.com/apache/storm/pull/2147 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2152: [STORM-2505] OffsetManager should account for offset void...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2152 @askprasanna let's just create a new JIRA and associate it with the STORM-2505 JIRA - it's a quick thing and will take care of this clean. Another minor detail we have a script to find orphan pull requests, malformed pull requests, etc. The pull request description `[STORM-2505] OffsetManager...` does not conform with the name convention. Can you please rename it to follow the pattern `STORM-2505: OffsetManager...`. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2156: STORM-2549: Fix broken enforcement mechanism for maxUncom...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2156 @srdo I am doing one last review of this patch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2226: STORM-2642: Storm-kafka-client spout cannot be serialized...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2226 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2224: STORM-2640: Deprecate KafkaConsumer.subscribe API option,...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2224 @srdo since the dynamic partition assignment does not work anyways, and since Storm 2.0 is a major release, I wonder if we shouldn't simply remove these classes. It is not backwards compatible, but if it doesn't really work, what's the harm? I am not a big fan of, from very early releases, leaving deprecated code around, which just becomes a source of confusion and harder to maintain. At the very least we should plan a future release to remove it for good. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2224: STORM-2640: Deprecate KafkaConsumer.subscribe API option,...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2224 The commit with title "Cherry pick STORM-2542" should have the same title as in master. We should also incorporate the Java 7 compatibility changes in the commit itself. Otherwise it's really hard to track which commits really belong to the JIRA. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2224: STORM-2640: Deprecate KafkaConsumer.subscribe API option,...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2224 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2243: STORM-2658: Extract storm-kafka-client examples to...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2243#discussion_r130422627
--- Diff:
examples/storm-kafka-client-examples/src/main/java/org/apache/storm/kafka/trident/TridentKafkaClientWordCountNamedTopics.java
---
@@ -42,72 +42,68 @@
import org.apache.storm.tuple.Values;
public class TridentKafkaClientWordCountNamedTopics {
+
private static final String TOPIC_1 = "test-trident";
private static final String TOPIC_2 = "test-trident-1";
private static final String KAFKA_LOCAL_BROKER = "localhost:9092";
-private KafkaTridentSpoutOpaque
newKafkaTridentSpoutOpaque() {
-return new KafkaTridentSpoutOpaque<>(newKafkaSpoutConfig());
+private KafkaTridentSpoutOpaque
newKafkaTridentSpoutOpaque(KafkaSpoutConfig spoutConfig) {
+return new KafkaTridentSpoutOpaque<>(spoutConfig);
}
private static Func, List>
JUST_VALUE_FUNC = new JustValueFunc();
/**
- * Needs to be serializable
+ * Needs to be serializable.
*/
private static class JustValueFunc implements
Func, List>, Serializable {
+
@Override
public List apply(ConsumerRecord record) {
return new Values(record.value());
}
}
-protected KafkaSpoutConfig newKafkaSpoutConfig() {
-return KafkaSpoutConfig.builder(KAFKA_LOCAL_BROKER, TOPIC_1,
TOPIC_2)
-.setProp(ConsumerConfig.GROUP_ID_CONFIG,
"kafkaSpoutTestGroup_" + System.nanoTime())
-.setProp(ConsumerConfig.MAX_PARTITION_FETCH_BYTES_CONFIG,
200)
-.setRecordTranslator(JUST_VALUE_FUNC, new Fields("str"))
-.setRetry(newRetryService())
-.setOffsetCommitPeriodMs(10_000)
-.setFirstPollOffsetStrategy(EARLIEST)
-.setMaxUncommittedOffsets(250)
-.build();
+protected KafkaSpoutConfig newKafkaSpoutConfig(String
bootstrapServers) {
+return KafkaSpoutConfig.builder(bootstrapServers, TOPIC_1, TOPIC_2)
+.setProp(ConsumerConfig.GROUP_ID_CONFIG,
"kafkaSpoutTestGroup_" + System.nanoTime())
+.setProp(ConsumerConfig.MAX_PARTITION_FETCH_BYTES_CONFIG, 200)
+.setRecordTranslator(JUST_VALUE_FUNC, new Fields("str"))
+.setRetry(newRetryService())
+.setOffsetCommitPeriodMs(10_000)
+.setFirstPollOffsetStrategy(EARLIEST)
+.setMaxUncommittedOffsets(250)
+.build();
}
protected KafkaSpoutRetryService newRetryService() {
return new KafkaSpoutRetryExponentialBackoff(new
TimeInterval(500L, TimeUnit.MICROSECONDS),
-TimeInterval.milliSeconds(2), Integer.MAX_VALUE,
TimeInterval.seconds(10));
+TimeInterval.milliSeconds(2), Integer.MAX_VALUE,
TimeInterval.seconds(10));
}
public static void main(String[] args) throws Exception {
new TridentKafkaClientWordCountNamedTopics().run(args);
}
-protected void run(String[] args) throws AlreadyAliveException,
InvalidTopologyException, AuthorizationException, InterruptedException {
-if (args.length > 0 && Arrays.stream(args).anyMatch(option ->
option.equals("-h"))) {
-System.out.printf("Usage: java %s [%s] [%s] [%s] [%s]\n",
getClass().getName(),
-"broker_host:broker_port", "topic1", "topic2",
"topology_name");
-} else {
-final String brokerUrl = args.length > 0 ? args[0] :
KAFKA_LOCAL_BROKER;
-final String topic1 = args.length > 1 ? args[1] : TOPIC_1;
-final String topic2 = args.length > 2 ? args[2] : TOPIC_2;
-
-System.out.printf("Running with broker_url: [%s], topics: [%s,
%s]\n", brokerUrl, topic1, topic2);
-
-Config tpConf = new Config();
-tpConf.setDebug(true);
-tpConf.setMaxSpoutPending(5);
-
-// Producers
-StormSubmitter.submitTopology(topic1 + "-producer", tpConf,
KafkaProducerTopology.newTopology(brokerUrl, topic1));
-StormSubmitter.submitTopology(topic2 + "-producer", tpConf,
KafkaProducerTopology.newTopology(brokerUrl, topic2));
-// Consumer
-StormSubmitter.submitTopology("topics-consumer", tpConf,
TridentKafkaConsumerTopology.newTopology(newKafkaTridentSpoutOpaque()));
-
-// Print results to console, which also cau
[GitHub] storm pull request #2243: STORM-2658: Extract storm-kafka-client examples to...
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/2243#discussion_r130417840 --- Diff: examples/storm-kafka-client-examples/README.markdown --- @@ -0,0 +1,10 @@ +## Usage +This module contains example topologies demonstrating storm-kafka-client spout and Trident usage. + +The module is built by `mvn clean package`. This will generate the `target/storm-kafka-client-examples-VERSION.jar` file. The jar contains all dependencies and can be submitted to Storm via the Storm CLI. For example: --- End diff -- ... built running ... the Storm CLI, e.g.: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2243: STORM-2658: Extract storm-kafka-client examples to...
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/2243#discussion_r130429805 --- Diff: external/storm-kafka-client/src/test/java/org/apache/storm/kafka/spout/builders/SingleTopicKafkaSpoutConfiguration.java --- @@ -21,15 +21,10 @@ import static org.apache.storm.kafka.spout.KafkaSpoutConfig.FirstPollOffsetStrategy.EARLIEST; import org.apache.kafka.clients.consumer.ConsumerConfig; --- End diff -- I also would call the two getXyz methods in this class createXyz, as they are static factory methods. I know that the name was already like that, but since we are changing it, we should just make it more conventional. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2243: STORM-2658: Extract storm-kafka-client examples to...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2243#discussion_r130428532
--- Diff:
examples/storm-kafka-client-examples/src/main/java/org/apache/storm/kafka/spout/test/KafkaSpoutTopologyMainNamedTopics.java
---
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.storm.kafka.spout.test;
+
+import static
org.apache.storm.kafka.spout.KafkaSpoutConfig.FirstPollOffsetStrategy.EARLIEST;
+
+import org.apache.kafka.clients.consumer.ConsumerConfig;
+import org.apache.storm.Config;
+import org.apache.storm.StormSubmitter;
+import org.apache.storm.generated.StormTopology;
+import org.apache.storm.kafka.spout.ByTopicRecordTranslator;
+import org.apache.storm.kafka.spout.KafkaSpout;
+import org.apache.storm.kafka.spout.KafkaSpoutConfig;
+import org.apache.storm.kafka.spout.KafkaSpoutRetryExponentialBackoff;
+import
org.apache.storm.kafka.spout.KafkaSpoutRetryExponentialBackoff.TimeInterval;
+import org.apache.storm.kafka.spout.KafkaSpoutRetryService;
+import org.apache.storm.kafka.trident.KafkaProducerTopology;
+import org.apache.storm.topology.TopologyBuilder;
+import org.apache.storm.tuple.Fields;
+import org.apache.storm.tuple.Values;
+
+public class KafkaSpoutTopologyMainNamedTopics {
+
+private static final String TOPIC_2_STREAM = "test_2_stream";
+private static final String TOPIC_0_1_STREAM = "test_0_1_stream";
+private static final String KAFKA_LOCAL_BROKER = "localhost:9092";
+private static final String TOPIC_0 = "kafka-spout-test";
+private static final String TOPIC_1 = "kafka-spout-test-1";
+private static final String TOPIC_2 = "kafka-spout-test-2";
+
+public static void main(String[] args) throws Exception {
+new KafkaSpoutTopologyMainNamedTopics().runMain(args);
+}
+
+protected void runMain(String[] args) throws Exception {
--- End diff --
Isn't this change removing the ability to run this code in LocalCluster
mode? I think it is very useful. For example, I use it all the time to run
these simple test examples from IntelliJ.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2243: STORM-2658: Extract storm-kafka-client examples to...
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/2243#discussion_r130425918 --- Diff: external/storm-kafka-client/src/test/java/org/apache/storm/kafka/spout/builders/SingleTopicKafkaSpoutConfiguration.java --- @@ -21,15 +21,10 @@ import static org.apache.storm.kafka.spout.KafkaSpoutConfig.FirstPollOffsetStrategy.EARLIEST; import org.apache.kafka.clients.consumer.ConsumerConfig; --- End diff -- I would call package where this class lives `config.builder` instead of `builders`, which is a bit misleading since this is really a configuration class. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2243: STORM-2658: Extract storm-kafka-client examples to...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2243#discussion_r130431738
--- Diff: examples/storm-kafka-client-examples/pom.xml ---
@@ -73,19 +75,27 @@
org.apache.storm
storm-kafka-client
${project.version}
+
org.apache.kafka
${storm.kafka.artifact.id}
${storm.kafka.client.version}
+compile
+
org.apache.kafka
kafka-clients
${storm.kafka.client.version}
+compile
+
[GitHub] storm pull request #2243: STORM-2658: Extract storm-kafka-client examples to...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2243#discussion_r130431776
--- Diff: examples/storm-kafka-client-examples/pom.xml ---
@@ -73,19 +75,27 @@
org.apache.storm
storm-kafka-client
${project.version}
+
org.apache.kafka
${storm.kafka.artifact.id}
${storm.kafka.client.version}
+compile
+
[GitHub] storm pull request #2243: STORM-2658: Extract storm-kafka-client examples to...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2243#discussion_r130419415
--- Diff: examples/storm-kafka-client-examples/pom.xml ---
@@ -73,19 +75,27 @@
org.apache.storm
storm-kafka-client
${project.version}
+
[GitHub] storm pull request #2243: STORM-2658: Extract storm-kafka-client examples to...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2243#discussion_r130418867
--- Diff: examples/storm-kafka-client-examples/pom.xml ---
@@ -42,7 +42,9 @@
org.apache.storm
storm-kafka
${project.version}
+
[GitHub] storm issue #2243: STORM-2658: Extract storm-kafka-client examples to storm-...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2243 +1. Thanks @srdo --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2250: WIP: STORM-2665: Adapt Kafka's release note generation sc...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2250 @srdo Can you please add a screenshot, or a link for a sample of the output. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2249: WIP: STORM-2648/STORM-2357: Add storm-kafka-client suppor...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2249 @srdo can you please assign JIRA's to you and mark them as in progress as you work on them and/or submit a pull request. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request #2250: STORM-2665: Adapt Kafka's release note generation ...
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/2250#discussion_r131024223 --- Diff: dev-tools/release_notes.py --- @@ -0,0 +1,107 @@ +#!/usr/bin/env python + +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +#http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Usage: release_notes.py > RELEASE_NOTES.html + +Generates release notes for a Storm release by generating an HTML doc containing some introductory information about the + release with links to the Storm docs followed by a list of issues resolved in the release. The script will fail if it finds + any unresolved issues still marked with the target release. You should run this script after either resolving all issues or + moving outstanding issues to a later release. + +""" + +from jira import JIRA +import itertools, sys + +if len(sys.argv) < 2: +print >>sys.stderr, "Usage: release_notes.py " +sys.exit(1) + +version = sys.argv[1] + +JIRA_BASE_URL = 'https://issues.apache.org/jira' +MAX_RESULTS = 100 # This is constrained for cloud instances so we need to fix this value + +def get_issues(jira, query, **kwargs): +""" +Get all issues matching the JQL query from the JIRA instance. This handles expanding paginated results for you. Any additional keyword arguments are forwarded to the JIRA.search_issues call. +""" +results = [] +startAt = 0 +new_results = None +while new_results == None or len(new_results) == MAX_RESULTS: +new_results = jira.search_issues(query, startAt=startAt, maxResults=MAX_RESULTS, **kwargs) +results += new_results +startAt += len(new_results) +return results + +def issue_link(issue): +return "%s/browse/%s" % (JIRA_BASE_URL, issue.key) + + +if __name__ == "__main__": +apache = JIRA(JIRA_BASE_URL) +issues = get_issues(apache, 'project=STORM and fixVersion=%s' % version) +if not issues: +print >>sys.stderr, "Didn't find any issues for the target fix version" +sys.exit(1) + +# Some resolutions, including a lack of resolution, indicate that the bug hasn't actually been addressed and we shouldn't even be able to create a release until they are fixed +UNRESOLVED_RESOLUTIONS = [None, + "Unresolved", + "Duplicate", + "Invalid", + "Not A Problem", + "Not A Bug", + "Won't Fix", + "Incomplete", + "Cannot Reproduce", + "Later", + "Works for Me", + "Workaround", + "Information Provided" + ] +unresolved_issues = [issue for issue in issues if issue.fields.resolution in UNRESOLVED_RESOLUTIONS or issue.fields.resolution.name in UNRESOLVED_RESOLUTIONS] +if unresolved_issues: +print >>sys.stderr, "The release is not completed since unresolved issues or improperly resolved issues were found still tagged with this release as the fix version:" +for issue in unresolved_issues: +print >>sys.stderr, "Unresolved issue: %15s %20s %s" % (issue.key, issue.fields.resolution, issue_link(issue)) +print >>sys.stderr +print >>sys.stderr, "Note that for some resolutions, you should simply remove the fix version as they have not been truly fixed in this release." +sys.exit(1) + +# Get
[GitHub] storm pull request #2250: STORM-2665: Adapt Kafka's release note generation ...
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/2250#discussion_r131024194 --- Diff: dev-tools/release_notes.py --- @@ -0,0 +1,107 @@ +#!/usr/bin/env python + +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +#http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Usage: release_notes.py > RELEASE_NOTES.html + +Generates release notes for a Storm release by generating an HTML doc containing some introductory information about the + release with links to the Storm docs followed by a list of issues resolved in the release. The script will fail if it finds + any unresolved issues still marked with the target release. You should run this script after either resolving all issues or + moving outstanding issues to a later release. + +""" + +from jira import JIRA +import itertools, sys + +if len(sys.argv) < 2: +print >>sys.stderr, "Usage: release_notes.py " +sys.exit(1) + +version = sys.argv[1] + +JIRA_BASE_URL = 'https://issues.apache.org/jira' +MAX_RESULTS = 100 # This is constrained for cloud instances so we need to fix this value + +def get_issues(jira, query, **kwargs): +""" +Get all issues matching the JQL query from the JIRA instance. This handles expanding paginated results for you. Any additional keyword arguments are forwarded to the JIRA.search_issues call. +""" +results = [] +startAt = 0 +new_results = None +while new_results == None or len(new_results) == MAX_RESULTS: +new_results = jira.search_issues(query, startAt=startAt, maxResults=MAX_RESULTS, **kwargs) +results += new_results +startAt += len(new_results) +return results + +def issue_link(issue): +return "%s/browse/%s" % (JIRA_BASE_URL, issue.key) + + +if __name__ == "__main__": +apache = JIRA(JIRA_BASE_URL) +issues = get_issues(apache, 'project=STORM and fixVersion=%s' % version) +if not issues: +print >>sys.stderr, "Didn't find any issues for the target fix version" +sys.exit(1) + +# Some resolutions, including a lack of resolution, indicate that the bug hasn't actually been addressed and we shouldn't even be able to create a release until they are fixed +UNRESOLVED_RESOLUTIONS = [None, + "Unresolved", + "Duplicate", + "Invalid", + "Not A Problem", + "Not A Bug", + "Won't Fix", + "Incomplete", + "Cannot Reproduce", + "Later", + "Works for Me", + "Workaround", + "Information Provided" + ] +unresolved_issues = [issue for issue in issues if issue.fields.resolution in UNRESOLVED_RESOLUTIONS or issue.fields.resolution.name in UNRESOLVED_RESOLUTIONS] +if unresolved_issues: +print >>sys.stderr, "The release is not completed since unresolved issues or improperly resolved issues were found still tagged with this release as the fix version:" +for issue in unresolved_issues: +print >>sys.stderr, "Unresolved issue: %15s %20s %s" % (issue.key, issue.fields.resolution, issue_link(issue)) +print >>sys.stderr +print >>sys.stderr, "Note that for some resolutions, you should simply remove the fix version as they have not been truly fixed in this release." +sys.exit(1) + +# Get
[GitHub] storm issue #2250: STORM-2665: Adapt Kafka's release note generation script ...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2250 +1. Let's squash the commits and it's good to go as far as I am concerned. Thanks @srdo --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2249: WIP: STORM-2648/STORM-2357: Add storm-kafka-client suppor...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2249 @srdo reviewing it --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2271: STORM-2675: Fix storm-kafka-client Trident spout failing ...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2271 @srdo shouldn't this [PR](https://github.com/apache/storm/pull/2174) come in before this one? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2174: STORM-2554: Trident Kafka Spout Refactoring to Include Ma...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2174 @srdo thanks for your comments. Let me think about it. This change builds on the existing code and fixes some of the problems. I was also planning on revisiting this implementation. The way to go about it is either: 1. Submit this fix and then an improvement with the new design (if we agree it's the right thing to do) 2. Submit the new design (if we agree it's the right thing to do) without this fix, i.e. on top of the existing code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2276: Fix typos in Worker.java
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2276 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2174: STORM-2554: Trident Kafka Spout Refactoring to Include Ma...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2174 @srdo I am evaluating if we can do the change without breaking the API. If so we can go ahead with it. Otherwise, as you suggested, we can go with this change for 1.x-branch and then refactor for Storm 2.0. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2275: STORM-2692: Load only configs specific to the topology in...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2275 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2277: STORM-2666: Fix storm-kafka-client spout sometimes emitti...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2277 @srdo can you please update the JIRA ticket, thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2181: [STORM-2607] Offset consumer + 1
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2181 @tiodollar once you do the merge, can you please squash all the commits. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm issue #2273: 1.1.x branch
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2273 @momoxixi what's the motivation behind merging 1.1.x-branch onto master? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
