Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2502
Congrats @roshannaik great effort and perseverance to get this in and
thanks to @revans2 for reviewing in great detail.
---
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2241
@revans2 here is the initial test cases we are looking to run against
master & STORM-2306. Let us know if you would like to any further cases
https://docs.google.com/docume
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2241
@revans2 I am trying to reproduce the worst-case in your last chart.
Running TVL topology with 4 spout, 10 splitters, 4 counters, 2 ackers. Here is
the code
https://gist.github.com/harshach
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2241
@revans2 I am trying to reproduce the worst-case in your last chart.
Running TVL topology with 4 spout, 10 splitters, 4 counters, 2 ackers. Here is
the code
https://gist.github.com/harshach
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2241
@HeartSaVioR Its not 12 executors per worker. If you don't pass a
command-line argument, it sets parallelism variable here to 4
https://github.com/apache/storm/blob/master/examples/storm-starter
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2241
@revans2 @HeartSaVioR
Here are my findings
https://docs.google.com/spreadsheets/d/1wPpC3YXp-vTIelRTUVoLxuxIYxUekiIHUpZ2ZEysC4Y/edit#gid=1644511...
1. Looking
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2241
@HeartSaVioR I don't mind breaking this into multiple PRs if it helps
reviewing and merging in. Its up to @roshannaik .
---
If your project is set up for it, you can reply to this email and have
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2241
@HeartSaVioR lets keep this discussion to reviews. This is not forum to
discuss what one should tweet or not that's up to individuals. Nobody is trying
to promote something that's not feasible lets
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2241
@revans2 Do you mind posting your storm.yaml or are you running with
defaults. We will try to see if we can reproduce this same behavior on our
side. If there are any bugs we will work to fix
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2151
@srdo are we not planning on pushing this into 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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2208
@HeartSaVioR wouldn't that be an issue incase of non-secure cluster if we
are defaulting to "digest"?
---
If your project is set up for it, you can reply to this email and have your
re
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2147
@hmcl @srdo I don't think we need this given this PR
https://github.com/apache/storm/pull/2151 makes manual assignment as default.
---
If your project is set up for it, you can reply
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2151
+1. Thanks @srdo this looks great.
---
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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2155
still +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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2217
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2215
+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
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/2215#discussion_r127821694
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/SerializableDeserializer.java
---
@@ -16,10 +16,14 @@
package
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2208
@liu-zhaokun I think the comment there meant to say by default it will be
"No Authentication". I.e Its users responsibility to set to digest in a secure
clusters. But since the defaul
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2209
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2165
+1. Tried on windows 10 looks good to me.
---
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
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/2157#discussion_r124087558
--- Diff:
external/storm-hdfs/src/main/java/org/apache/storm/hdfs/bolt/AbstractHdfsBolt.java
---
@@ -297,9 +296,21 @@ protected Path
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2166
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2173
@revans2 this means the users exported STORM_EXT_CLASSPATH must contain a
wildcard "*"
This could result in issues with current users who are just passing the dir
and not adding
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2155
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2176
+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
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/2150#discussion_r123881560
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/PatternTopicFilter.java
---
@@ -0,0 +1,59 @@
+/*
+ * Copyright
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2117
@srdo thought I sent and replied your earlier emails as well. Looks like
some issue with gmail they are showing as sent but didn't reached the mailing
list. I sent them again.
---
If your project
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2117
@srdo Sorry not meant as negative for the PR. But want to get a better
exposure to everyone on changes that we make and for users/devs who might not
be able to follow dev list day-in/day-out
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2117
@srdo @HeartSaVioR @erikdw
I understand this PR is merged. But we should be extremely careful when we
break the backward incompatibility , if it justifies better implementation of a
connector
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2121
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2120
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2122
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2112
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2114
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2115
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2116
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2104
@vivekmittal I think you need to open another PR against 1.x-branch. Don't
think this can be cherry-picked onto 1.x-branch.
---
If your project is set up for it, you can reply to this email
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2104
+1. Thanks @vivekmittal
---
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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2104
@vivekmittal over LGTM. I am +1 once the method name is addressed.
Thanks for finding & addressing the bug.
---
If your project is set up for it, you can reply to this email and have your
r
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/2104#discussion_r115551459
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/internal/OffsetManager.java
---
@@ -49,10 +51,14 @@ public OffsetManager
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2058
@liu-zhaokun I am can help merge this. But my comment is not addressed.
I think its better to break this into two files no?
1. storm_jaas.conf which contains storm related sections only
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2071
@vesense also if we make this static all the internal producer state
becomes shared and this could result in unexpected behavior as per user. Since
producer doesn't call flush to broker until
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2071
@vesense not sure any benefit doing this static and making one instance per
JVM. This actually adds complexity in code without giving any benefit.
For the most part , when users configures
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2098
@rban1 looks good. Can you add the new config to README and also squash the
commits into 1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2104
@vivekmittal can you squash your commits into singe one.
"Topology stopped processing (or died) & topic got compacted
(cleanup.policy=compact) leaving offset voids in
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/2104#discussion_r115412857
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/internal/OffsetManager.java
---
@@ -68,13 +74,34 @@ public
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2102
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2100
Thanks @adityasharad this looks good. +1. Can you please squash your
commits into single one.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2097
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2081
@arunmahadevan we need a PR for 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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1924
Thanks @srdo for your patience. Merged into 1.x & 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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2081
still +1 after the above 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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2083
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2082
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2086
overall LGTM. +1 .
@tandrup would like to see these configs documented here
https://github.com/apache/storm/blob/master/external/storm-cassandra/README.md
---
If your project is set up
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2086
@tandrup for small docs changes etc. we don't file JIRAs but this one had
quite few changes good to have that in JIRA and subsequently in CHANGELOG.
---
If your project is set up for it, you can
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2089
@vesense this looks good. +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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2066
@liu-zhaokun following upon my previous comment , do not use
stom-kafka-client from 1.0.x-branch as there are lot of bug-fixes went into
storm-kafka-client in Storm 1.1 release
---
If your
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2066
@liu-zhaokun I am still not sure why are we adding this code. One shouldn't
be using 0.9 kafka clients and this storm-kafka-client will not work with 0.9
as the interface changed in 0.10 and we
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2074
+1. once the commits gets squashed.
---
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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2074
@kamleshbhatt can you please squash commits into single commit.
---
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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2088
+1. Thanks @erikdw
---
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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2090
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2086
@tandrup can you please a file a JIRA
https://issues.apache.org/jira/browse/ under STORM project. Also update the
title of the JIRA and squash the commits in this PR. More details
https
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2084
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2024
LGTM @vesense . +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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2029
Thanks @hmcc merged into master.
@HeartSaVioR agree we can keep this in master and look at releasing 2.0
instead of back-porting.
---
If your project is set up for it, you can reply
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2080
+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
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/2024#discussion_r111462620
--- Diff:
external/storm-rocketmq/src/main/java/org/apache/storm/rocketmq/spout/RocketMQSpout.java
---
@@ -0,0 +1,189 @@
+/**
+ * Licensed
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/2024#discussion_r111524719
--- Diff:
external/storm-rocketmq/src/main/java/org/apache/storm/rocketmq/spout/RocketMQSpout.java
---
@@ -0,0 +1,189 @@
+/**
+ * Licensed
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/2024#discussion_r111462177
--- Diff:
external/storm-rocketmq/src/main/java/org/apache/storm/rocketmq/spout/RocketMQSpout.java
---
@@ -0,0 +1,189 @@
+/**
+ * Licensed
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2058
@liu-zhaokun why are we including Server section intended for Zookeeper
server in this file?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2066
@liu-zhaokun you can set it via topology.worker.childopts or
worker.childopts. Lets is not put work-around for this when there is viable
option to set it. Also 0.9 consumer API is for Alpha and its
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2062
@liu-zhaokun KafkaSpoutConfig offers this method to add security-related
configs
https://github.com/apache/storm/blob/master/external/storm-kafka-client/src/main/java/org/apache/storm/kafka
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2029
@hmcc can you please up-merge your patch. I'll merge it into master &
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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2054
@ambud we should add the auth_to_local rules an make that as part of
storm.yaml config option . Adding regex will not be helpful here.
---
If your project is set up for it, you can reply
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2062
@liu-zhaokun More details are documented here
http://kafka.apache.org/documentation.html#security_sasl . We don't need these
changes on storm-kafka-client side
---
If your project is set up
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2062
@liu-zhaokun with the latest kafka consumer APIs one can pass a keytab and
principal via consumer or producer properties. Even before that they can pass
the jaas config via JVM param and set
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2053
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2026
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2029
Thanks @hmcc. +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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2056
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2032
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2053
@arunmahadevan this looks like backward incompatible change?
---
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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2004
@srdo can you squash some of those commits.
---
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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2028
+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
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/2029#discussion_r110038799
--- Diff: examples/storm-elasticsearch-examples/pom.xml ---
@@ -53,6 +54,11 @@
storm-elasticsearch
${project.version
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2031
@megha10 thanks for the patch. Pleas follow the guide lines here
https://github.com/apache/storm/blob/master/DEVELOPER.md#contribute-code.
1. Open a STORM jira
https://issues.apache.org
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2042
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2044
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2026
@rban1 can you change the PR title to reflect the JIRA title. You can look
other PRs for example
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2026
@rban1 you are keep re-opening the PRs. We should keep only one PR. If you
want to address the comments and update the PR , all you need to do is to work
on the same git branch and push changes
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1998
@rban1 there are still some unaddressed comments and also merge conflicts.
Make sure you squashed your commits for the PR as well.
---
If your project is set up for it, you can reply to this email
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2027
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/1999
+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
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2020
+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
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/1998#discussion_r106768060
--- Diff:
external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/bolt/EventHubBolt.java
---
@@ -84,11 +85,47 @@ public void prepare(Map config
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/1998#discussion_r106768022
--- Diff:
external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/bolt/EventHubBolt.java
---
@@ -41,7 +42,8
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/1998#discussion_r106768166
--- Diff:
external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/spout/EventHubReceiverImpl.java
---
@@ -65,77 +57,81 @@ public
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/1998#discussion_r106768041
--- Diff:
external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/bolt/EventHubBolt.java
---
@@ -84,11 +85,47 @@ public void prepare(Map config
1 - 100 of 1114 matches
Mail list logo
