Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Stig Rohde Døssing
Alexandre, Could you also post the BasicKafkaSpout source file? I'm curious what it's doing. 2017-11-20 7:50 GMT+01:00 Alexandre Vermeerbergen : > Hello Jungtaek, > > OK I will activate these traces, but since we need to capture the Spouts' > initialization traces, how should I activate these t

[GitHub] storm pull request #2428: STORM-2826: Set key/value deserializer fields when...

2017-11-19 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/2428#discussion_r151916643 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java --- @@ -292,17 +292,21 @@ private Builder(String bootstr

[GitHub] storm pull request #2426: STORM-2825: Fix ClassCastException when storm-kafk...

2017-11-19 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/2426#discussion_r151912622 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java --- @@ -718,7 +718,13 @@ private static void setAutoComm

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Alexandre Vermeerbergen
Hello Jungtaek, OK I will activate these traces, but since we need to capture the Spouts' initialization traces, how should I activate these traces? Indeed, if I use one of the techniques shown here https://community.hortonworks.com/articles/36151/debugging-an-apache-storm-topology.html then I'm

[GitHub] storm pull request #2426: STORM-2825: Fix ClassCastException when storm-kafk...

2017-11-19 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/2426#discussion_r151887752 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java --- @@ -718,7 +718,13 @@ private static void setAutoComm

[GitHub] storm pull request #2428: STORM-2826: Set key/value deserializer fields when...

2017-11-19 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/2428#discussion_r151886343 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java --- @@ -292,17 +292,21 @@ private Builder(String bootstr

[GitHub] storm issue #2427: MINOR: Use booleans instead of strings for 'enable.auto.c...

2017-11-19 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2427 +1 ---

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Jungtaek Lim
It would be much appreciated if you could change topology log level to the following: level: DEBUG, logger name: 'ROOT' or 'org.apache', timeout: long enough (say 1800 or 3600), and kill worker which contains Spout in UI or console. Above instruction enables logging with DEBUG level, and Kafka spo

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Alexandre Vermeerbergen
Hello Stig, Thanks again for your latest fix. I have no longer any exception when submitting my topologies, but then they read nothing from my Kafka topics. So I made another test: I reverted from your latest storm-kafka-client 1.2.0 snapshot jar to storm-kafka-client-1.1.0.jar in my topologies,

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Stig Rohde Døssing
Thanks, I've addressed the issue here https://github.com/apache/storm/pull/2428 and uploaded a new jar at the same link here https://drive.google.com/file/d/1DgJWjhWwczYgZS82YGd63V3GT2G_ v9fd/view?usp=sharing. I went over the PR that made these changes, and I don't believe anything else breaks back

[GitHub] storm pull request #2428: STORM-2826: Set key/value deserializer fields when...

2017-11-19 Thread srdo
GitHub user srdo opened a pull request: https://github.com/apache/storm/pull/2428 STORM-2826: Set key/value deserializer fields when using the convenience builder methods in KafkaSpoutConfig Builds on STORM-2825. You can merge this pull request into a Git repository by running:

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Alexandre Vermeerbergen
Hi Stig, Here's the source of our "BasicKafkaSpout" class's constructor (I can send the full source if needed), public BasicKafkaSpout(KafkaSpoutConfig config) { this.kafkaBrokers = (String) config.getKafkaProps().get( ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG);

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Stig Rohde Døssing
Alexandre, I'm sorry this is giving you so much trouble. Looking at the stack trace you posted, it seems like the NPE is coming from BasicKafkaSpout line 72. Can you post that line (and maybe some of the surrounding code)? 2017-11-19 15:13 GMT+01:00 Alexandre Vermeerbergen : > Hi Stig, > > After

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Alexandre Vermeerbergen
Hi Stig, After having included commons-lang-2.5.jar in my topologies, I still have 11 topologies failing with this new message, this time a NullPointerException: Running: /usr/local/jdk/bin/java -client -Ddaemon.name= -Dstorm.options= -Dstorm.home=/usr/local/Storm/storm-stable -Dstorm.log.dir=/us

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Stig Rohde Døssing
Using Maven and controlling dependencies aren't mutually exclusive. Tools like Nexus (https://help.sonatype.com/display/NXRM2/Procurement+Suite) allow you to control access to dependencies without manually putting jars in an SCM. I realize this doesn't help you right now, just thought I'd mention

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Alexandre Vermeerbergen
Believe it or not, we don't internally use Maven : we strictly control our dependencies. So okay, I have delivered a "storm-commons-lang2.jar" in our SCM system in order to specifically add it to our topologies big jars depending on storm-kafka-client (feeling sad). I'll tell you the outcome ASAP

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Stig Rohde Døssing
I also don't think that adding dependencies constitutes a breaking change, nor should it. Dependency management tooling like Maven will handle pulling the right dependencies automatically. 2017-11-19 12:34 GMT+01:00 Jungtaek Lim : > Alexandre, > > There's so much pain on using relocated artifact

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Stig Rohde Døssing
Storm-core shades its dependencies so we avoid locking users into using the same versions of dependencies as the Storm daemons will use. The way to change storm-core (and shaded dependencies) is to upgrade the cluster. By contrast "external" modules like storm-kafka-client are included in the user

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Jungtaek Lim
Alexandre, There's so much pain on using relocated artifact in other module in a project. You need to refer the class as relocated name, and your IDE (at least IntelliJ) complains and the compilation in IDE will not succeed. (The build via Maven will succeed though.) There was same issue on 2.0.0

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Alexandre Vermeerbergen
Hello Stig, I'm afraid I disagree with you on this point: while I could add into my topologies' big jar files a copy of commons-lang-2.5.jar, I think 1.2.0 storm-kafka-client shouln't introduce such breaking dependency, especially in the light that this dependency is already packaged in storm-core

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Stig Rohde Døssing
Alexandre, Thanks for trying. I think this time it's not a problem with storm-kafka-client. It has a dependency on commons-lang 2.5, which is declared in the storm-kafka-client pom. We usually recommend that people use Maven/Gradle/Ivy or similar systems to ensure that they get all dependencies wh

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Alexandre Vermeerbergen
Hello Stig, Than you very much for you quick answer & fix. Unfortunaly, I still have topologies failing to start but with a different exception this time: Exception in thread "main" java.lang.NoClassDefFoundError: org/apache/commons/lang/StringUtils at org.apache.storm.kafka.spout.NamedTo

[GitHub] storm pull request #2427: MINOR: Use booleans instead of strings for 'enable...

2017-11-19 Thread srdo
GitHub user srdo opened a pull request: https://github.com/apache/storm/pull/2427 MINOR: Use booleans instead of strings for 'enable.auto.commit' setting Kafka understands booleans in this property just fine, there's no reason to use strings instead. You can merge this pull request

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Stig Rohde Døssing
I've put up a fix here https://github.com/apache/storm/pull/2426. There's an updated storm-kafka-client jar at https://drive.google.com/file/d/1DgJWjhWwczYgZS82YGd63V3GT2G_v9fd/view?usp=sharing if you'd like to try it out. 2017-11-19 10:17 GMT+01:00 Stig Rohde Døssing : > Oops. The exception is n

[GitHub] storm issue #2426: STORM-2825: Fix ClassCastException when storm-kafka-clien...

2017-11-19 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2426 This is not an issue in master because we don't try to decide processing guarantee from the `enable.auto.commit` setting there. ---

[GitHub] storm pull request #2426: STORM-2825: Fix ClassCastException when storm-kafk...

2017-11-19 Thread srdo
GitHub user srdo opened a pull request: https://github.com/apache/storm/pull/2426 STORM-2825: Fix ClassCastException when storm-kafka-client uses consu… …mer config with String-type 'enable.auto.commit' You can merge this pull request into a Git repository by running: $ git

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Stig Rohde Døssing
Oops. The exception is not intentional, it's a bug. In 1.2.0 we check for the "enable.auto.commit" key in the kafkaConsumerProp map, and if it is set we warn in the log that it shouldn't be, because users should use the KafkaSpoutConfig.Builder.setProcessingGuarantee method instead. When the proper

Re: [Discuss] Release Storm 1.2.0

2017-11-19 Thread Alexandre Vermeerbergen
Hello Stig, Here's my first feedback on this Storm 1.2.0 preview on my Supervision system based on Storm : I have 11 topologies KO (not even able to start), and 4 topologies which seem to be unaffected. Details: - I used the binaries posted on dropbox by Jungteak and your binary for storm-kafka-c