[GitHub] storm pull request #1703: [STORM-1057] Add throughput metrics to spouts/bolt...
GitHub user wangli1426 opened a pull request: https://github.com/apache/storm/pull/1703 [STORM-1057] Add throughput metrics to spouts/bolts and display them on web ui for 1.x-branch Hi @HeartSaVioR, @revans2, @harshach,@d2r,@unsleepy22, I upmerge PR apache/storm#753 to 1.x-branch. Sorry for the delay. Please review. Thanks. You can merge this pull request into a Git repository by running: $ git pull https://github.com/wangli1426/storm throughput-metrics Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1703.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 #1703 commit 67721f77f28cbb4a29fab7e3023b220095886338 Author: Li Wang Date: 2015-09-18T02:11:20Z initializing Timer in RateTracker.java lazily to avoid a bug in Clojure Maven plugin that might make compiling process never finish commit 1dad2041fb0ae8689794fec93d05df2266292dc8 Author: Li Wang Date: 2015-09-18T07:19:28Z take throughput measurements on spouts and bolts commit d74f784175cc4b8c631c52acaab3e0743f11a56d Author: Li Wang Date: 2015-09-20T01:26:11Z keep Nimbus updated to the latest throughput stats commit 066e234bcf141525c0a1ed4a04f6d3cf160f6c28 Author: wangli1426 Date: 2015-09-22T14:17:17Z display the throughput metrics on web ui commit d552a990af56ccd7e8e5211a0146030fccf570a1 Author: wangli1426 Date: 2015-10-03T15:29:37Z addressed all concerns raised by revans2 commit 22460771279c788198dbbbfa060a67db05cf449f Author: wangli1426 Date: 2015-10-17T04:57:22Z upmerged with master commit a5116b38ee4f64e5bec951b81538b1f4a1c980bd Author: wangli1426 Date: 2015-10-17T06:59:54Z updated storm.thrift for rolling update, removed unused functions in stats.clj, and updated STORM-UI-REST-API.md commit 4de33d1562bbdc2d7c7d17c605614939fe4b288e Author: wangli1426 Date: 2015-10-21T15:37:59Z Merge remote-tracking branch 'apache/master' into throughput-metrics commit 1b0598485adb298d1e008b82ecda706d5996721f Author: wangli1426 Date: 2015-10-22T02:59:06Z addressed issues raised by revans2 commit fd89a78a9acac830cab3391da4f94eb232ff68fb Author: Li Wang Date: 2016-09-22T07:21:12Z Merge branch '1.x-branch' into throughput-metrics --- 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 #753: [STORM-1057] Add throughput metrics to spouts/bolts and di...
Github user wangli1426 commented on the issue: https://github.com/apache/storm/pull/753 @harshach I managed to upmerge this PR to 1.x-branch in #1703. 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 #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r79990064 --- Diff: external/storm-eventhubs/pom.xml --- @@ -21,19 +21,19 @@ storm org.apache.storm -1.0.3-SNAPSHOT +1.0.2 --- End diff -- These versions probably shouldn't be changed --- 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 #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1702 This seems fine once the storm versions are reverted, though you should get this on the master branch first. I'd like to mention before you spend too much time on trying to make the spout run reliably, that we've had some reliability problems with the com.microsoft.eventhubs.client:eventhubs-client since the underlying AMQP library isn't being maintained. There's a new eventhub client here based on Apache Proton which is likely to provide a better experience. https://github.com/Azure/azure-event-hubs/tree/master/java --- 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 #1696: STORM-2104: More graceful handling of acked/failed...
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/1696#discussion_r79992419
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -145,6 +154,10 @@ private void initialize(Collection
partitions) {
}
retryService.retainAll(partitions);
+
+//Emitted messages for partitions that are no longer assigned
to this spout can't be acked, and they shouldn't be retried. Remove them from
emitted.
+Set partitionsSet = new HashSet(partitions);
+emitted.removeIf((msgId) ->
!partitionsSet.contains(msgId.getTopicPartition()));
--- End diff --
The messages should be getting removed from retryService in line 156. It's
my impression that onPartitionsAssigned will be getting called immediately
after onPartitionsRevoked, before the current call to poll returns (see
https://kafka.apache.org/090/javadoc/org/apache/kafka/clients/consumer/ConsumerRebalanceListener.html).
---
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 #1704: STORM-2111 [Storm SQL] support 'LIKE' and 'SIMILAR...
GitHub user HeartSaVioR opened a pull request: https://github.com/apache/storm/pull/1704 STORM-2111 [Storm SQL] support 'LIKE' and 'SIMILAR TO' Note: This patch is on top of STORM-2089 in order to reduce any merge conflict and upmerging. Reviewers may want to only take a look into 3e05056. I also addressed some ExprCompiler's bug: doesn't wrap null as type in elvis operator which raises type mismatch. Please review and comment. Thanks! You can merge this pull request into a Git repository by running: $ git pull https://github.com/HeartSaVioR/storm STORM-2111 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1704.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 #1704 --- 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 #1679: STORM-2087: storm-kafka-client - tuples not always...
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/1679#discussion_r79993708
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -266,26 +266,32 @@ private void doSeekRetriableTopicPartitions() {
if (offsetAndMeta != null) {
kafkaConsumer.seek(rtp, offsetAndMeta.offset() + 1); //
seek to the next offset that is ready to commit in next commit cycle
} else {
-kafkaConsumer.seekToEnd(toArrayList(rtp));// Seek to
last committed offset
+kafkaConsumer.seek(rtp, acked.get(rtp).committedOffset +
1);// Seek to last committed offset
--- End diff --
Looks good, 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 #1679: STORM-2087: storm-kafka-client - tuples not always being ...
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1679 This more or less seems done to me. The only thing that bugs me is Storm double acking tuples. The only case I could think of is if tuples time out and they're later acked by the acker bolt, but it seems like the executor actually handles that. As far as I can tell hitting the changed case in findNextCommitOffset should actually indicate a bug in the code, because we've received an ack for a tuple we've already committed to Kafka, which shouldn't be possible unless a tuple is double acked. I don't think that should be happening unless the spout is doing something wrong? @jfenc91 are you still seeing that case (L498) getting hit even with these changes and the other 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 pull request #1696: STORM-2104: More graceful handling of acked/failed...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/1696#discussion_r79800937
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/SerializableDeserializer.java
---
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2016 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 org.apache.kafka.common.serialization.Deserializer;
+
+/**
+ * @param The type this deserializer deserializes to.
+ */
+public interface SerializableDeserializer extends Deserializer,
Serializable {
--- End diff --
Why do we need this wrapper marking 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 pull request #1605: STORM-2014: Put logic around dropping messages int...
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/1605#discussion_r80004187
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -330,11 +328,9 @@ public void ack(Object messageId) {
public void fail(Object messageId) {
final KafkaSpoutMessageId msgId = (KafkaSpoutMessageId) messageId;
emitted.remove(msgId);
-if (msgId.numFails() < maxRetries) {
-msgId.incrementNumFails();
-retryService.schedule(msgId);
-} else { // limit to max number of retries
-LOG.debug("Reached maximum number of retries. Message [{}]
being marked as acked.", msgId);
+msgId.incrementNumFails();
+if (!retryService.schedule(msgId)) {
+LOG.debug("Retry service indicated message should not be
retried. Message [{}] being marked as acked.", msgId);
--- End diff --
@srdo I am not sure I completely follow your reasoning. The most meaningful
piece of information that we want to show to the user here is that if the max
number of retries has been reached, no more retries happen. If the user reads a
message saying that the retry service marked it not to be retried, he still
does not know the cause.
Looking at the code, the `boolean schedule(msgId)` only returns false only
if the max number of retries has been reached.
Although the retry service log messages hints ant the max retries cap being
reached, the user will have to have both logs enabled to have both spouts. I
favor the message as it was before, as it is straight to the point and provides
good insight on what is happening.
---
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 #1605: STORM-2014: Put logic around dropping messages into Retry...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1605 I am +1 overall. Please format the commit message to be easy to read and squash the two commits. This is a simple change that should have only one commit. We can merge after that. --- 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 #1696: STORM-2104: More graceful handling of acked/failed tuples...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1696 @srdo @jfenc91 I am on vacation this week (with limited access to Internet) and I will be back on Monday. Can we please holding on merging this until I can finish my review. I implemented the original patch, and would like to review these changes. 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 #1679: STORM-2087: storm-kafka-client - tuples not always being ...
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1679 @srdo @jfenc91 I am on vacation this week (with limited access to Internet) and I will be back on Monday. **Can we please holding on merging this until I can finish my review on Monday**. I implemented the original patch, and would like to review these changes.Thanks. I am still a bit confused about the duplicated acking tuples. If that is the case, that could be a possible storm guarantees problem, which should be fixed at its root cause, and not worked around in spout code. Furthermore, from what I understand, as far as the findNextOffset method behavior, the duplicate acking should be irrelevant, because once all the tuples up to a certain offset have been acked, they will be committed in the next commit call. For example let's say `1,2,3,4,5, 8,9,10` have acked, on the next commit call, `1,2,3,4,5` should be committed, but `8,9,10` won't be until `6,7` get acked. If `6,7` never get acked, say (retry forever scenario), then the desired behavior is that `8,9,10` never get committed. @jfenc91 Can you help me understand how does the duplicate acking interfere in this scenario, and how? --- 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 #1704: STORM-2111 [Storm SQL] support 'LIKE' and 'SIMILAR TO'
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1704 Tests of storm-sql-core failed because of VM crashing. UT passes on my local dev. --- 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 #1605: STORM-2014: Put logic around dropping messages int...
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/1605#discussion_r80023848
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -330,11 +328,9 @@ public void ack(Object messageId) {
public void fail(Object messageId) {
final KafkaSpoutMessageId msgId = (KafkaSpoutMessageId) messageId;
emitted.remove(msgId);
-if (msgId.numFails() < maxRetries) {
-msgId.incrementNumFails();
-retryService.schedule(msgId);
-} else { // limit to max number of retries
-LOG.debug("Reached maximum number of retries. Message [{}]
being marked as acked.", msgId);
+msgId.incrementNumFails();
+if (!retryService.schedule(msgId)) {
+LOG.debug("Retry service indicated message should not be
retried. Message [{}] being marked as acked.", msgId);
--- End diff --
@hmcl I was referring to RetryService being an interface users can
override, which doesn't mention max retries in the schedule javadoc, and this
part of the code technically not "knowing about" max retries, because users may
supply a retry service that doesn't schedule tuples for some other reason. I
think you're right though, it was probably clearer before.
---
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 #1605: STORM-2014: Put logic around dropping messages into Retry...
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1605 Squashed. I hope this commit message is clearer. --- 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 #1705: STORM-2117 Supervisor V2 with local mode extracts ...
GitHub user HeartSaVioR opened a pull request: https://github.com/apache/storm/pull/1705 STORM-2117 Supervisor V2 with local mode extracts resources directory to the wrong directory * it extracts the resources directory to topology root directory instead of temporary directory * it should be extracted to temporary directory since we will move that directory to topology root directory It didn't make unit tests failing, but I ran the tests from IntelliJ manually, IntelliJ adds some jars which contains resources directory, and this happens. @revans2 Please take a look. Thanks! You can merge this pull request into a Git repository by running: $ git pull https://github.com/HeartSaVioR/storm STORM-2117 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1705.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 #1705 commit 26dc3a6ad333a79224eaac71d5876d0ea1db5853 Author: Jungtaek Lim Date: 2016-09-22T11:46:11Z STORM-2117 Supervisor V2 with local mode extracts resources directory to the wrong directory * it extracts the resources directory to topology root directory instead of temporary directory * it should be extracted to temporary directory since we will move that directory to topology root directory --- 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 #1696: STORM-2104: More graceful handling of acked/failed...
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/1696#discussion_r80026248
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/SerializableDeserializer.java
---
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2016 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 org.apache.kafka.common.serialization.Deserializer;
+
+/**
+ * @param The type this deserializer deserializes to.
+ */
+public interface SerializableDeserializer extends Deserializer,
Serializable {
--- End diff --
I thought it was nice to have, since setKey/ValueDeserializer in the
builder implicitly requires the deserializer to be serializable. For example,
if you try to set the standard Kafka StringDeserializer via those methods,
you'll get a NotSerializableException when the topology is submitted to Storm,
since they'll be set as fields on the final KafkaSpoutConfig field in
KafkaSpout.
---
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 #1696: STORM-2104: More graceful handling of acked/failed tuples...
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1696 @hmcl Sure thing. --- 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 #1679: STORM-2087: storm-kafka-client - tuples not always being ...
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1679 @hmcl I agree that if there really is a double acking problem somewhere else, it should be fixed there. In your scenario say the spout commits 1...5 to Kafka and 4 is later acked. ackedMsgs now contains 4, and committedOffset is 5. With the previous code, that would permanently break findNextCommitOffset. findNextCommitOffset would start at 5, and the first message in ackedMsgs list is 4. The code breaks out of the loop over ackedMsgs, since the else block in L496 is hit, causing the function to return null. When null is returned to commitOffsetsForAckedTuples, that means that it will skip that topic partition when committing, which means that ackedMsgs doesn't get cleared. That means that on the next call to findNextCommitOffset the same thing happens again. The end result is that the spout never commits anything for that topic partition again. I agree that we should never be in the case where 4 is acked after committedOffset has become larger than 4, which is why I think the else block in L496 should probably log at a higher level than debug. --- 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 #1706: [STORM-2118] A few fixes for storm-sql standalone ...
GitHub user arunmahadevan opened a pull request: https://github.com/apache/storm/pull/1706 [STORM-2118] A few fixes for storm-sql standalone mode 1. Cast the result, accumulator and value types correctly 2. Support aggregate functions with more than one argument You can merge this pull request into a Git repository by running: $ git pull https://github.com/arunmahadevan/storm udf Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1706.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 #1706 commit 2da36e660dbf3e80db54a209a91d093f520ff7cd Author: Arun Mahadevan Date: 2016-09-15T10:12:25Z [STORM-2118] A few fixes for storm-sql standalone mode 1. Cast the result, accumulator and value types correctly 2. Support aggregate functions with more than one argument --- 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 #1705: STORM-2117 Supervisor V2 with local mode extracts resourc...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1705 +1 I'll pull this into #1697 too --- 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 #1700: STORM-2110: strip out empty String in worker opts
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1700 The travis failure is unrelated to this change, it is a failure in maven downloading a dependency. @knusbaum or @kishorvpatil could you please take a look at this? --- 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 #1699: STORM-2109: Treat Supervisor CPU/MEMORY Configs as Number...
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1699 The travis failure is unrelated it is yet again a maven issue downloading something that should be there. @knusbaum @kishorvpatil could you please take a look? --- 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 #1707: STORM-2113 [Storm SQL] fix 'OR' and 'AND' operator...
GitHub user HeartSaVioR opened a pull request: https://github.com/apache/storm/pull/1707 STORM-2113 [Storm SQL] fix 'OR' and 'AND' operators handle more than 2 operands NOTE: This is on top of STORM-2089 and STORM-2111 since this requires the ExprCompiler bugfix introduced on STORM-2111. After the patch 'IN' and 'NOT IN' operator works properly. It also respects 'short circuit'. You can merge this pull request into a Git repository by running: $ git pull https://github.com/HeartSaVioR/storm STORM-2113 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1707.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 #1707 commit ce73a8b9fc4ee13620d07c1f621a36a7e327a872 Author: Jungtaek Lim Date: 2016-09-13T02:40:13Z STORM-2089 Replace Consumer of ISqlTridentDataSource with SqlTridentConsumer * SqlTridentConsumer contains StateFactory and StateUpdater which is needed to store tuples to State via batch * Apply the change to storm-sql-kafka * move out JsonScheme and JsonSerializer to runtime * it will be used from other external sql modules * add javadoc to ISqlTridentDataSource commit 3e050569e06276446e2bcec42dff80bfd659e8f4 Author: Jungtaek Lim Date: 2016-09-22T06:38:44Z STORM-2111 [Storm SQL] support 'LIKE' and 'SIMILAR TO' * associate 'LIKE' and 'SIMILAR TO' to its implementation Calcite is providing * NOTE: SIMILAR TO takes SQL regex, not Java regex * add unit tests commit ee32e8538f9e8dc765257318bd083da2a1a3ecb4 Author: Jungtaek Lim Date: 2016-09-22T14:37:44Z STORM-2113 [Storm SQL] fix 'OR' and 'AND' operators handle more than 2 operands * This fix makes 'IN' and 'NOT IN' working properly --- 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 #1681: STORM-1444 Support EXPLAIN statement in StormSQL
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1681 '!storm-core' fails due to the timeout of downloading artifact. I tested it manually. --- 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 #1700: STORM-2110: strip out empty String in worker opts
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1700 +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 #1699: STORM-2109: Treat Supervisor CPU/MEMORY Configs as Number...
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1699 +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 #1699: STORM-2109: Treat Supervisor CPU/MEMORY Configs as...
Github user kishorvpatil commented on a diff in the pull request:
https://github.com/apache/storm/pull/1699#discussion_r79851428
--- Diff:
storm-core/src/jvm/org/apache/storm/daemon/supervisor/timer/SupervisorHeartbeat.java
---
@@ -72,10 +72,10 @@ private SupervisorInfo buildSupervisorInfo(Map conf, Supervisor
private Map mkSupervisorCapacities(Map conf) {
Map ret = new HashMap();
-Double mem = (double)
(conf.get(Config.SUPERVISOR_MEMORY_CAPACITY_MB));
-ret.put(Config.SUPERVISOR_MEMORY_CAPACITY_MB, mem);
-Double cpu = (double) (conf.get(Config.SUPERVISOR_CPU_CAPACITY));
-ret.put(Config.SUPERVISOR_CPU_CAPACITY, cpu);
+Number mem = (Number)
(conf.get(Config.SUPERVISOR_MEMORY_CAPACITY_MB));
+ret.put(Config.SUPERVISOR_MEMORY_CAPACITY_MB, mem.doubleValue());
+Number cpu = (Number) (conf.get(Config.SUPERVISOR_CPU_CAPACITY));
--- End diff --
Possibility of NPE here. Can we use utils method to do this.
---
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 #1699: STORM-2109: Treat Supervisor CPU/MEMORY Configs as...
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1699#discussion_r80074819
--- Diff:
storm-core/src/jvm/org/apache/storm/daemon/supervisor/timer/SupervisorHeartbeat.java
---
@@ -72,10 +72,10 @@ private SupervisorInfo buildSupervisorInfo(Map conf, Supervisor
private Map mkSupervisorCapacities(Map conf) {
Map ret = new HashMap();
-Double mem = (double)
(conf.get(Config.SUPERVISOR_MEMORY_CAPACITY_MB));
-ret.put(Config.SUPERVISOR_MEMORY_CAPACITY_MB, mem);
-Double cpu = (double) (conf.get(Config.SUPERVISOR_CPU_CAPACITY));
-ret.put(Config.SUPERVISOR_CPU_CAPACITY, cpu);
+Number mem = (Number)
(conf.get(Config.SUPERVISOR_MEMORY_CAPACITY_MB));
+ret.put(Config.SUPERVISOR_MEMORY_CAPACITY_MB, mem.doubleValue());
+Number cpu = (Number) (conf.get(Config.SUPERVISOR_CPU_CAPACITY));
--- End diff --
Good point. Utils.getDouble() with default value would be safer.
---
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 #1694: STORM-2101: fixes npe in compute-executors in nimbus
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1694 +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 #1695: STORM-2101: fixes npe in compute-executors in nimbus
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1695 +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 #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1702 Hm, nevermind the new client, it requires Java 8. It's still an option for Storm 2.0 though. --- 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 #1708: [STORM-2119] - bug in log message printing to stdo...
GitHub user jerrypeng opened a pull request: https://github.com/apache/storm/pull/1708 [STORM-2119] - bug in log message printing to stdout You can merge this pull request into a Git repository by running: $ git pull https://github.com/jerrypeng/storm STORM-2119 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/1708.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 #1708 commit 967132b80245cdfd8de70cbe4112524c85b75392 Author: Boyang Jerry Peng Date: 2016-09-22T18:39:24Z [STORM-2119] - bug in log message printing to stdout --- 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 #1708: [STORM-2119] - bug in log message printing to stdout
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1708 +1 Nice finding. --- 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 #1706: [STORM-2118] A few fixes for storm-sql standalone ...
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1706#discussion_r80131848
--- Diff:
external/sql/storm-sql-runtime/src/test/org/apache/storm/sql/TestUtils.java ---
@@ -60,6 +62,25 @@ public static String result(String accumulator) {
}
+ public static class TopN {
+public static PriorityQueue init() {
+ return new PriorityQueue<>();
+}
+public static PriorityQueue add(PriorityQueue
accumulator, Integer n, Integer val) {
+ if (accumulator.size() >= n) {
--- End diff --
Logic need to be changed to add val first, and remove the lowest `if
accumulator.size() > n` because val could be lower than lowest value of
priority queue.
Suppose the input `3, 2, 1`, accumulator will be changed to 3 -> 2, 3 -> 1,
3 (should be 2, 3).
We could even do nothing when `(n > 0 && accumulator.size() >= n &&
accumulator.peek() >= val)`.
---
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 #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80133950 --- Diff: external/storm-eventhubs/pom.xml --- @@ -21,19 +21,19 @@ storm org.apache.storm -1.0.3-SNAPSHOT +1.0.2 --- End diff -- Yes @srdo is right. --- 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 #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80133790 --- Diff: external/storm-eventhubs/pom.xml --- @@ -21,19 +21,19 @@ storm org.apache.storm -1.0.3-SNAPSHOT +1.0.2 ../../pom.xml storm-eventhubs -1.0.3-SNAPSHOT +1.0.2.1 jar storm-eventhubs EventHubs Storm Spout UTF-8 -0.9.1 +1.0.1 --- End diff -- IMHO it might be better to maintain the version of dependencies from the root pom.xml. Some modules do, and some modules don't, so it's not a mandatory but just food for thought. --- 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 #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/1702#discussion_r80133198 --- Diff: external/storm-eventhubs/pom.xml --- @@ -21,19 +21,19 @@ storm org.apache.storm -1.0.3-SNAPSHOT +1.0.2 ../../pom.xml storm-eventhubs -1.0.3-SNAPSHOT +1.0.2.1 --- End diff -- Here too. Every modules should follow the Storm version. Would be better to remove this line as same as other modules. --- 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 #1702: Update Eventhub-Client jar dependency to 1.0.1
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1702 Btw, IMO, upgrading the dependency is worth to file an issue. @raviperi Could you file it and copy description of PR to issue's description? --- 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 #1701: Emit to _spoutConfig.outputStreamId
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1701 +1 Nice finding. `storm-kafka` CI build fails but looks unrelated. Btw, IMO this bug is worth to file an issue, since tuple is sent to the wrong stream or throwing errors or even not sent. @aichow Could you please file an issue and add issue name to prefix of PR's title? I can handle it if you mind. --- 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 #1680: Minor typos in documentation
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1680 +1 Nice. --- 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 #1676: STORM-2085: Remove guava from storm-core pom.
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1676 I'm +1 on this. @revans2 @ptgoetz Could you review this as well? This is a change of dependency but removing shaded dependency. Which version line do you think we can apply? --- 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 #1674: STORM-2083: Blacklist scheduler
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1674 @nilday Do you have any updates? --- 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 #1669: STORM-2078: enable paging in worker datatable
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1669 +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 #1661: [STORM-2071] Add in retry after rebalance in unit test
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1661 @ppoulosk Could we close this as Supervisor V2 was merged? 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 #1701: STORM-2120 - Emit to _spoutConfig.outputStreamId
Github user aichow commented on the issue: https://github.com/apache/storm/pull/1701 @HeartSaVioR Not sure if I tagged the JIRA issue with the right Affects Versions, so feel free to edit as appropriate. 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 #1661: [STORM-2071] Add in retry after rebalance in unit ...
Github user ppoulosk closed the pull request at: https://github.com/apache/storm/pull/1661 --- 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 #1661: [STORM-2071] Add in retry after rebalance in unit test
Github user ppoulosk commented on the issue: https://github.com/apache/storm/pull/1661 Yes. 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 #1654: STORM-2066: make error message in IsolatedPool.java more ...
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1654 @jerrypeng Could you get the chance to review this? Or could anyone familiar with IsolatedPool review this? --- 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 #1640: Fix version command in storm.cmd
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1640 +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 #1611: [storm-2022]fix test case
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1611 @lujinhong Could we close this as #1606 is merged? --- 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 #1604: [STORM-2013] Upgrade Netty to 3.10.6
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1604 @darionyaphet Since 3.10.6 is EOL of 3.x, I'd rather move on Netty 4. Patches are already here #728 (master) #1591 (1.x), and when we're OK with the performance test I think we can check them in. --- 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 #1590: [STORM-2003] Make sure config contains TOPIC before get i...
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1590 +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 #1582: A better way to get default value
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1582 +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 #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1591 Since Netty made 3.x version line EOL, I'd like to bump Netty to 4.0.x or even 4.1.x if there's no performance / resource usage issue. @hsun-cnnxty Please upmerge this. If you really don't mind, could you do performance tests again on two latest versions: 4.0.41.Final and 4.1.5.Final, too? --- 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 #1296: STORM-1675 - Allow submitting multiple jars from the clie...
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1296 I think STORM-2016 covers this, so we can close this. @abhishekagarwal87 What do you think? --- 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 #1470: STORM-1886 Extend KeyValueState iface with delete
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1470 @kosii Any update on this? --- 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 #1399: update readme.md
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1399 This is resolved via STORM-1993. --- 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 #1674: STORM-2083: Blacklist scheduler
Github user nilday commented on the issue: https://github.com/apache/storm/pull/1674 @HeartSaVioR not yet, busy with something else. Will do it as soon as I have some time. --- 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 #1135: [STORM-1567] in defaults.yaml 'topology.disable.lo...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1135 --- 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 #1455: [STORM-1872]Release Jedis connection when topology...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1455 --- 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 #1582: A better way to get default value
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1582 --- 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 #1590: [STORM-2003] Make sure config contains TOPIC befor...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1590 --- 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 #1640: Fix version command in storm.cmd
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1640 --- 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 #1680: Minor typos in documentation
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1680 --- 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 #1691: STORM-2090: Add integration test for storm windowing
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1691 I squashed commits before merging (b779ca4e270f6f2a86d4d30336004e4a642ec690) but forgot to write 'Closes #1691' to commit log. @raghavgautam Could you close this? 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 #1694: STORM-2101: fixes npe in compute-executors in nimb...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1694 --- 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 #1695: STORM-2101: fixes npe in compute-executors in nimb...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1695 --- 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 #1700: STORM-2110: strip out empty String in worker opts
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1700 --- 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 #1701: STORM-2120 - Emit to _spoutConfig.outputStreamId
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1701 --- 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 #1276: STORM-1664: Allow Java users to start a local cluster wit...
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1276 +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 #1276: STORM-1664: Allow Java users to start a local clus...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1276 --- 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 #1691: STORM-2090: Add integration test for storm windowing
Github user raghavgautam commented on the issue: https://github.com/apache/storm/pull/1691 Thanks @harshach @HeartSaVioR 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 #1691: STORM-2090: Add integration test for storm windowi...
Github user raghavgautam closed the pull request at: https://github.com/apache/storm/pull/1691 --- 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 #1691: STORM-2090: Add integration test for storm windowing
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1691 This a fairly large commit that seemingly includes code from other projects. That's fine as long as you can document what code was copied, and what the license for that code was. --- 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 #1669: STORM-2078: enable paging in worker datatable
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1669 --- 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 #1691: STORM-2090: Add integration test for storm windowing
Github user raghavgautam commented on the issue: https://github.com/apache/storm/pull/1691 I had mention this on the jira. A good part of the vagrant setup has been picked up from: https://github.com/ptgoetz/storm-vagrant https://github.com/harshach/storm-vagrant https://issues.apache.org/jira/browse/STORM-2090 Both of them have apache license: https://github.com/ptgoetz/storm-vagrant/blob/master/LICENSE https://github.com/harshach/storm-vagrant/blob/master/LICENSE Rest has been authored by me (contributed by Hortonworks). This includes TestngListner.java which was originally contributed to apache falcon. https://github.com/apache/falcon/blob/master/falcon-regression/merlin/src/test/java/org/apache/falcon/regression/TestngListener.java --- 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 #1691: STORM-2090: Add integration test for storm windowing
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1691 Just to be clear, that wasn't a criticism. I just wanted to point out that it is important that we know the provenance and license of all code that enters our repository. --- 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 #1591: STORM-1038: Upgrade netty to 4.x in 1.x-branch
Github user hsun-cnnxty commented on the issue: https://github.com/apache/storm/pull/1591 I will find some time this weekend to work on it. -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 #1691: STORM-2090: Add integration test for storm windowing
Github user raghavgautam commented on the issue: https://github.com/apache/storm/pull/1691 @ptgoetz Do we need to document any other place or just mentioning on the jira is sufficient ? --- 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 #1709: STORM-2116 [Storm SQL] Support 'CASE' statement
GitHub user HeartSaVioR opened a pull request:
https://github.com/apache/storm/pull/1709
STORM-2116 [Storm SQL] Support 'CASE' statement
NOTE: This patch is on top of STORM-2089 and STORM-2111, and STORM-2113.
```
SELECT CASE WHEN NAME IN ('a', 'abc', 'abcde') THEN UPPER('a')
WHEN UPPER(NAME) = 'AB' THEN 'b' ELSE {fn CONCAT(NAME, '#')} END FROM FOO
```
(`{fn CONCAT(a, b)}` is same to `a || b`. Please refer [SQL
reference](http://calcite.apache.org/docs/reference.html) page on Calcite web
site.)
`CASE` statement in above sql statement is translated to Java code as
```
import org.apache.storm.tuple.Values;
java.lang.String t1 = null;
// WHEN #0 THEN #1
java.lang.Boolean t2 = Boolean.FALSE;
java.lang.Boolean anyNull_t3 = Boolean.FALSE;
// operand #0
java.lang.String t5 = (java.lang.String)(_data.get(1));
final java.lang.String t6 = (java.lang.String) "a";
java.lang.Boolean t4 =
org.apache.storm.sql.runtime.StormSqlFunctions.eq(t5,t6);
final java.lang.Boolean t7 = t4;
if ((t7 != null) && (t7)) { t2 = Boolean.TRUE; anyNull_t3 = Boolean.FALSE; }
else {
// updating null occurrence with operand #0
if ((t7 == null) && !(anyNull_t3)) { anyNull_t3 = Boolean.TRUE; }
// operand #1
java.lang.String t9 = (java.lang.String)(_data.get(1));
final java.lang.String t10 = (java.lang.String) "abc";
java.lang.Boolean t8 =
org.apache.storm.sql.runtime.StormSqlFunctions.eq(t9,t10);
final java.lang.Boolean t11 = t8;
if ((t11 != null) && (t11)) { t2 = Boolean.TRUE; anyNull_t3 =
Boolean.FALSE; }
else {
// updating null occurrence with operand #1
if ((t11 == null) && !(anyNull_t3)) { anyNull_t3 = Boolean.TRUE; }
// operand #2
java.lang.String t13 = (java.lang.String)(_data.get(1));
final java.lang.String t14 = (java.lang.String) "abcde";
java.lang.Boolean t12 =
org.apache.storm.sql.runtime.StormSqlFunctions.eq(t13,t14);
final java.lang.Boolean t15 = t12;
if ((t15 != null) && (t15)) { t2 = Boolean.TRUE; anyNull_t3 =
Boolean.FALSE; }
else {
// updating null occurrence with operand #2
if ((t15 == null) && !(anyNull_t3)) { anyNull_t3 = Boolean.TRUE; }
}}}
t2 = anyNull_t3 ? ((java.lang.Boolean) null) : t2;
final java.lang.String t17;
if (false) {}
else { t17 = org.apache.calcite.runtime.SqlFunctions.upper("a"); }
final java.lang.String t16 = (java.lang.String) t17;
if (t2 == true) { t1 = t16; }
else {
// WHEN #2 THEN #3
final java.lang.String t20;
java.lang.String t21 = (java.lang.String)(_data.get(1));
if (false) {}
else if (t21 == null) { t20 = null; }
else { t20 = org.apache.calcite.runtime.SqlFunctions.upper(t21); }
final java.lang.String t19 = (java.lang.String) t20;
java.lang.Boolean t18 =
org.apache.storm.sql.runtime.StormSqlFunctions.eq(t19,"AB");
if (t18 == true) { t1 = "b"; }
else {
// ELSE
final java.lang.String t23;
java.lang.String t24 = (java.lang.String)(_data.get(1));
if (false) {}
else if (t24 == null) { t23 = null; }
else { t23 = org.apache.calcite.runtime.SqlFunctions.concat(t24,"#"); }
final java.lang.String t22 = (java.lang.String) t23;
t1 = t22;
}}
return new Values(t1);
```
@arunmahadevan Please take a look. Thanks.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HeartSaVioR/storm STORM-2116
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/1709.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 #1709
commit ce73a8b9fc4ee13620d07c1f621a36a7e327a872
Author: Jungtaek Lim
Date: 2016-09-13T02:40:13Z
STORM-2089 Replace Consumer of ISqlTridentDataSource with SqlTridentConsumer
* SqlTridentConsumer contains StateFactory and StateUpdater which is needed
to store tuples to State via batch
* Apply the change to storm-sql-kafka
* move out JsonScheme and JsonSerializer to runtime
* it will be used from other external sql modules
* add javadoc to ISqlTridentDataSource
commit 3e050569e06276446e2bcec42dff80bfd659e8f4
Author: Jungtaek Lim
Date: 2016-09-22T06:38:44Z
STORM-2111 [Storm SQL] support 'LIKE' and 'SIMILAR TO'
* associate 'LIKE' and 'SIMILAR TO' to its implementation Calcite is
providing
* NOTE: SIMILAR TO takes SQL regex, not Java regex
* add unit tests
commit 3e428ea4814d0a549a6e1101be26231623c8eb00
Author: Jungtaek Lim
Date: 2016-09-22T14:37:44Z
STORM-2113 [Storm SQL] fix 'OR' and 'AND' operators handle more than 2
operands
* This fix makes 'IN' and 'NOT IN' working properly
commit 60b6b054148a76b62761c1e9fd78c78bb88d88f6
Author: Jungtaek Lim
Date: 2016-09-23T03:47:54Z
STORM-2116 [Storm SQL]
[GitHub] storm pull request #1706: [STORM-2118] A few fixes for storm-sql standalone ...
Github user arunmahadevan commented on a diff in the pull request:
https://github.com/apache/storm/pull/1706#discussion_r80183483
--- Diff:
external/sql/storm-sql-runtime/src/test/org/apache/storm/sql/TestUtils.java ---
@@ -60,6 +62,25 @@ public static String result(String accumulator) {
}
+ public static class TopN {
+public static PriorityQueue init() {
+ return new PriorityQueue<>();
+}
+public static PriorityQueue add(PriorityQueue
accumulator, Integer n, Integer val) {
+ if (accumulator.size() >= n) {
--- End diff --
Thanks for catching. It should be
```java
if (accumulator.size() >= n) {
if (val > accumulator.peek()) {
accumulator.remove();
accumulator.add(val);
}
} else {
accumulator.add(n);
}
---
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 #1706: [STORM-2118] A few fixes for storm-sql standalone ...
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1706#discussion_r80184767
--- Diff:
external/sql/storm-sql-runtime/src/test/org/apache/storm/sql/TestUtils.java ---
@@ -60,6 +62,25 @@ public static String result(String accumulator) {
}
+ public static class TopN {
+public static PriorityQueue init() {
+ return new PriorityQueue<>();
+}
+public static PriorityQueue add(PriorityQueue
accumulator, Integer n, Integer val) {
+ if (accumulator.size() >= n) {
--- End diff --
One missed spot is that it should do nothing if n is 0. Now if n is 0, it
will behave same as top 1. n should be greater than 0 anyway but we can do some
defensive programming.
Other then that it looks 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 pull request #1706: [STORM-2118] A few fixes for storm-sql standalone ...
Github user arunmahadevan commented on a diff in the pull request:
https://github.com/apache/storm/pull/1706#discussion_r80185268
--- Diff:
external/sql/storm-sql-runtime/src/test/org/apache/storm/sql/TestUtils.java ---
@@ -60,6 +62,25 @@ public static String result(String accumulator) {
}
+ public static class TopN {
+public static PriorityQueue init() {
+ return new PriorityQueue<>();
+}
+public static PriorityQueue add(PriorityQueue
accumulator, Integer n, Integer val) {
+ if (accumulator.size() >= n) {
--- End diff --
yes, it will throw NPE if n is 0, since it tries to peek from empty queue.
will address this.
---
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 #1706: [STORM-2118] A few fixes for storm-sql standalone mode
Github user arunmahadevan commented on the issue: https://github.com/apache/storm/pull/1706 @HeartSaVioR addressed comments. --- 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 #1706: [STORM-2118] A few fixes for storm-sql standalone ...
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1706#discussion_r80186596
--- Diff:
external/sql/storm-sql-runtime/src/test/org/apache/storm/sql/TestUtils.java ---
@@ -60,6 +62,25 @@ public static String result(String accumulator) {
}
+ public static class TopN {
+public static PriorityQueue init() {
+ return new PriorityQueue<>();
+}
+public static PriorityQueue add(PriorityQueue
accumulator, Integer n, Integer val) {
+ if (accumulator.size() >= n) {
--- End diff --
Yes you're right. It'll throw NPE.
---
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 #1706: [STORM-2118] A few fixes for storm-sql standalone mode
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1706 @arunmahadevan Thanks for the quick fix. +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. ---
