[GitHub] storm pull request: [STORM-954] Topology event inspector

2015-08-18 Thread lazyval
Github user lazyval commented on a diff in the pull request:

https://github.com/apache/storm/pull/673#discussion_r37303658
  
--- Diff: storm-core/src/clj/backtype/storm/cluster.clj ---
@@ -240,7 +240,8 @@
 (when cb
   (cb id
 
-(defn- maybe-deserialize
+;; public for stubbing in nimbus_test
+(defn maybe-deserialize
--- End diff --

I don't see any change in tests, are you sure it's necessary? 


---
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: [Minor] Removing changelog file from kafka-sto...

2015-08-11 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/655#issuecomment-130072102
  
@revans2 any love for it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: add cgroup function that can limit cpu share o...

2015-08-09 Thread lazyval
Github user lazyval commented on a diff in the pull request:

https://github.com/apache/storm/pull/667#discussion_r36594606
  
--- Diff: storm-core/src/clj/backtype/storm/cgroup/cgroup_manager.clj ---
@@ -0,0 +1,109 @@
+(ns backtype.storm.cgroup.cgroup-manager
+  (:import [java.io File BufferedReader FileReader])
+  (:use [backtype.storm log]
+[backtype.storm.cgroup constant])
+  (:require [backtype.storm.util :as util]))
+
+(defn- get-dir [dir file-dir]
+  (util/normalize-path (str dir file-dir)))
+
+(defn get-tasks [dir]
+  (if-let [string-tasks (util/read-file (get-dir dir TASKS))]
+(into #{} (map #(util/parse-int %) string-tasks
+
+(defn add-task [dir task]
+  (util/append-line (get-dir dir TASKS) (str task)))
+
+(defn set-cpu-shares [dir weight]
+  (util/append-line (get-dir dir CPU-SHARES) (str weight)))
+
+(defn set-physical-usage-limit [dir value]
+  (util/append-line (get-dir dir MEMORY-LIMIT-IN-BYTES) (str value)))
+
+(defn set-with-swap-usage-limit [dir value]
+  (util/append-line (get-dir dir MEMORY-MEMSW-LIMIT-IN-BYTES) (str value)))
+
+(defn set-oom-control [dir flag]
+  (util/append-line (get-dir dir MEMORY-OOM-CONTROL) (if flag 0 1)))
+
+(defn analyse-subsystem [subsystems]
+  (loop [systems #{}
+ rest-systems (.split subsystems ,)]
+(if-let [system (keyword (first rest-systems))]
+  (recur (if (contains? SUBSYSTEM-TYPE system)
+   (conj systems system)
+   systems)
+(rest rest-systems))
+  systems)))
+
+(defn get-hierarchies []
+  (with-open [^FileReader file (FileReader. MOUNT-STATUS-FILE)
+  ^BufferedReader br (BufferedReader. file)]
+(loop [hierarchies {}
+   line (.readLine br)]
+  (if line
+(let [fields (.split line  )]
+  (if (= (get fields 2) cgroup)
+(let [name (get fields 0)
+  type (get fields 3)
+  dir (get fields 1)
+  hierarchy {:name name :subsystems (analyse-subsystem 
type) :dir dir :parent nil :is-root? true}]
+  (recur (conj hierarchies {type hierarchy}) (.readLine br)))
+(recur hierarchies (.readLine br
+(vals hierarchies)
+
+(defn busy [subsystem]
+  (loop [_hierarchy (get-hierarchies)]
+(if-let [hierarchy (first _hierarchy)]
+  (if (contains? (:subsystems hierarchy) subsystem)
+hierarchy
+(recur (rest _hierarchy))
+
+(defn check-cgroup
+  return true means cgroup service is ok, else means that cgroup hasn't 
been install or hasn't been started.
+  [conf]
+  (log-message checking cgroup mode)
+  (if (true? (conf CGROUP-ENABLE))
+(if-not (.exists (File. CGROUP-STATUS-FILE))
+  (log-error Cgroup error, please check /proc/cgroups, maybe hasn't 
been started.)
+  (if-let [root-dir (conf CGROUP-ROOT-DIR)]
+(let [_files (map #(File. (util/normalize-path (str % 
File/separator root-dir))) [CPU-HIERARCHY-DIR MEMORY-HIERARCHY-DIR])]
+  (loop [files _files]
+(if-let [file (first files)]
+  (if-not (.exists file)
+(log-error (.getAbsolutePath file)  is not existing.)
+(recur (rest files)))
+  true)))
+
+(defn mounted [hierarchy]
+  (let [dir (:dir hierarchy)
+name (:name hierarchy)]
+(if (util/exists-dir? dir)
+  (loop [a_hierarchy (get-hierarchies)]
+(if-let [_hierarchy (first a_hierarchy)]
+  (if (and (= dir (:dir _hierarchy)) (= name (:name _hierarchy)))
+_hierarchy
+(recur (rest a_hierarchy
+
+
+;(defn mount [hierarchy]
--- End diff --

commented code? why would you want it? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: Eventhub spout meta data

2015-07-28 Thread lazyval
Github user lazyval commented on a diff in the pull request:

https://github.com/apache/storm/pull/651#discussion_r35654576
  
--- Diff: 
external/storm-eventhubs/src/main/java/org/apache/storm/eventhubs/spout/EventDataScheme.java
 ---
@@ -42,6 +44,9 @@
 AmqpValue amqpValue = (AmqpValue) section;
 fieldContents.add(amqpValue.getValue().toString());
 return fieldContents;
+  }else if (section instanceof MessageAnnotations) {
--- End diff --

quote minor, but additional space will not hurt


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-768] Support JDK 8 compile and runtime.

2015-07-28 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/519#issuecomment-125667064
  
Hm, I tried to do the same on completely different machine (beefy linux 
server instead of OSX laptop) and results are the very same: 

```
[INFO] 

[INFO] Reactor Summary:
[INFO] 
[INFO] Storm .. SUCCESS [  
1.046 s]
[INFO] maven-shade-clojure-transformer  SUCCESS [  
1.756 s]
[INFO] storm-maven-plugins  SUCCESS [  
1.629 s]
[INFO] multilang-javascript ... SUCCESS [  
0.109 s]
[INFO] multilang-python ... SUCCESS [  
0.107 s]
[INFO] multilang-ruby . SUCCESS [  
0.104 s]
[INFO] Storm Core . FAILURE [03:24 
min]
[INFO] storm-starter .. SKIPPED
[INFO] storm-kafka  SKIPPED
[INFO] storm-hdfs . SKIPPED
[INFO] storm-hbase  SKIPPED
[INFO] storm-hive . SKIPPED
[INFO] storm-jdbc . SKIPPED
[INFO] storm-redis  SKIPPED
[INFO] storm-eventhubs  SKIPPED
[INFO] flux ... SKIPPED
[INFO] flux-wrappers .. SKIPPED
[INFO] flux-core .. SKIPPED
[INFO] flux-examples .. SKIPPED
[INFO] 

[INFO] BUILD FAILURE
[INFO] 

[INFO] Total time: 03:30 min
[INFO] Finished at: 2015-07-28T16:12:37+00:00
[INFO] Final Memory: 62M/1550M
[INFO] 

[ERROR] Failed to execute goal 
com.theoryinpractise:clojure-maven-plugin:1.7.1:test (test-clojure) on project 
storm-core: Clojure failed. - [Help 1]
```

So far reproduction rate is 100% 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-768] Support JDK 8 compile and runtime.

2015-07-28 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/519#issuecomment-125639681
  
Am I right that Storm still has no support for java 8? I tried to run test 
from master on my local machine, but they're failing: 

```
» mvn clean install
...
[INFO] 

[INFO] Reactor Summary:
[INFO] 
[INFO] Storm .. SUCCESS [  
1.453 s]
[INFO] maven-shade-clojure-transformer  SUCCESS [  
2.123 s]
[INFO] storm-maven-plugins  SUCCESS [  
2.324 s]
[INFO] multilang-javascript ... SUCCESS [  
0.048 s]
[INFO] multilang-python ... SUCCESS [  
0.049 s]
[INFO] multilang-ruby . SUCCESS [  
0.049 s]
[INFO] Storm Core . FAILURE [02:41 
min]
[INFO] storm-starter .. SKIPPED
[INFO] storm-kafka  SKIPPED
[INFO] storm-hdfs . SKIPPED
[INFO] storm-hbase  SKIPPED
[INFO] storm-hive . SKIPPED
[INFO] storm-jdbc . SKIPPED
[INFO] storm-redis  SKIPPED
[INFO] storm-eventhubs  SKIPPED
[INFO] flux ... SKIPPED
[INFO] flux-wrappers .. SKIPPED
[INFO] flux-core .. SKIPPED
[INFO] flux-examples .. SKIPPED
[INFO] 

[INFO] BUILD FAILURE
[INFO] 

» mvn -version 
Apache Maven 3.3.3 (7994120775791599e205a5524ec3e0dfe41d4a06; 
2015-04-22T15:57:37+04:00)
Maven home: /usr/local/Cellar/maven/3.3.3/libexec
Java version: 1.8.0_25, vendor: Oracle Corporation
Java home: 
/Library/Java/JavaVirtualMachines/jdk1.8.0_25.jdk/Contents/Home/jre
Default locale: en_US, platform encoding: UTF-8
OS name: mac os x, version: 10.10.4, arch: x86_64, family: mac
```

Same happens on travis (e.g. 
https://travis-ci.org/apache/storm/builds/72494388). It looks like a quite 
major issue. 

Is there plans to address it? How can I help with it? 



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [Minor] Removing changelog file from kafka-sto...

2015-07-28 Thread lazyval
GitHub user lazyval opened a pull request:

https://github.com/apache/storm/pull/655

[Minor] Removing changelog file from kafka-storm sub-project

No other storm/external project has changelog and it looks like log became 
obsolete ever since storm-kafka was incorporated into storm.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/lazyval/storm removing-kafkaspout-changelog

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/655.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 #655


commit 36366c801dc74be7111bcb2e7f5d78e6fe3570f4
Author: Kostya Golikov johnysilv...@gmail.com
Date:   2015-07-28T18:44:22Z

No other storm/external project has changelog and looks like log became 
obsolete ever since storm-kafka was incorporated into storm.




---
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: [Minor] Removing changelog file from kafka-sto...

2015-07-28 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/655#issuecomment-125712502
  
I also doubt that https://github.com/apache/storm/blob/master/TODO should 
be around


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-643] KafkaUtils repeatedly fetches mess...

2015-07-20 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/642#issuecomment-122858859
  
Minor complain, but [next time]  it would be handy to keep commit messages 
a bit more descriptive

img width=465 alt=storm 2015-07-20 14-42-47 
src=https://cloud.githubusercontent.com/assets/235297/8775105/97f3d9c4-2eed-11e5-84c7-07d467a1dab5.png;



---
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: HiveBolt: misc code cleanups

2015-07-07 Thread lazyval
Github user lazyval commented on a diff in the pull request:

https://github.com/apache/storm/pull/610#discussion_r34056406
  
--- Diff: 
external/storm-hive/src/main/java/org/apache/storm/hive/bolt/HiveBolt.java ---
@@ -262,7 +261,7 @@ private void retireEldestWriter() {
 private int retireIdleWriters() {
 int count = 0;
 long now = System.currentTimeMillis();
-ArrayListHiveEndPoint retirees = new ArrayListHiveEndPoint();
+ArrayListHiveEndPoint retirees = new ArrayList();
--- End diff --

Diamond types are also Java 7 feature.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-826] Update KafkaBolt to use the new ka...

2015-07-03 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/572#issuecomment-118368768
  
Okay, I am too late to the party :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-826] Update KafkaBolt to use the new ka...

2015-07-03 Thread lazyval
Github user lazyval commented on a diff in the pull request:

https://github.com/apache/storm/pull/572#discussion_r33868816
  
--- Diff: external/storm-kafka/pom.xml ---
@@ -113,8 +113,8 @@
 /dependency
 dependency
 groupIdorg.apache.kafka/groupId
-artifactIdkafka_2.9.2/artifactId
-version0.8.1.1/version
+artifactIdkafka_2.10/artifactId
--- End diff --

Why bump version? Why not set it to 2.11? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: STORM-924:Set the file mode of the files inclu...

2015-07-02 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/613#issuecomment-118044177
  
:+1: 

For those who wonders what this octal numbers mean, there is a great 
[*humanizator*](http://permissions-calculator.org/decode/0644/).


---
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: add dotNet adapter.

2015-07-02 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/612#issuecomment-118042465
  
Code aside, I guess storing *compiled* binaries and libraries in git isn't 
quite a good idea. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: STORM-919 Gathering worker and supervisor proc...

2015-07-02 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/608#issuecomment-118050871
  
It might be too late for this particular PR, but it makes sense to separate 
human changes and generated code into two separate commits. Otherwise it's 
really hard to review the code. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-712] Storm daemons shutdown if OutOfMem...

2015-04-09 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/468#issuecomment-91172000
  
:+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: Add a convenience target to use when debugging...

2015-04-08 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/510#issuecomment-90884297
  
@Parth-Brahmbhatt +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: fastfix-01:modify config.read-supervisor-storm...

2015-04-01 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/469#issuecomment-88447333
  
Minor, but you would be better off with *Removed unused `topology-path` 
variable* as PR title. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: STORM-188. Allow user to specifiy full configu...

2015-04-01 Thread lazyval
Github user lazyval commented on a diff in the pull request:

https://github.com/apache/storm/pull/495#discussion_r27562061
  
--- Diff: storm-core/src/jvm/backtype/storm/utils/Utils.java ---
@@ -135,35 +137,68 @@ public static void sleep(long millis) {
 }
 
 public static Map findAndReadConfigFile(String name, boolean 
mustExist) {
+InputStream in = null;
+Boolean confFileEmpty = false;
--- End diff --

no need in boxed value


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: STORM-188. Allow user to specifiy full configu...

2015-04-01 Thread lazyval
Github user lazyval commented on a diff in the pull request:

https://github.com/apache/storm/pull/495#discussion_r27561516
  
--- Diff: storm-core/src/jvm/backtype/storm/utils/Utils.java ---
@@ -135,35 +137,68 @@ public static void sleep(long millis) {
 }
 
 public static Map findAndReadConfigFile(String name, boolean 
mustExist) {
+InputStream in = null;
+Boolean confFileEmpty = false;
 try {
-HashSetURL resources = new HashSetURL(findResources(name));
-if(resources.isEmpty()) {
-if(mustExist) throw new RuntimeException(Could not find 
config file on classpath  + name);
-else return new HashMap();
-}
-if(resources.size()  1) {
-throw new RuntimeException(Found multiple  + name +  
resources. You're probably bundling the Storm jars with your topology jar. 
-  + resources);
-}
-URL resource = resources.iterator().next();
-Yaml yaml = new Yaml(new SafeConstructor());
-Map ret = null;
-InputStream input = resource.openStream();
-try {
-ret = (Map) yaml.load(new InputStreamReader(input));
-} finally {
-input.close();
+in = getConfigFileInputStream(name);
+if (null != in) {
+Yaml yaml = new Yaml(new SafeConstructor());
+Map ret = (Map) yaml.load(new InputStreamReader(in));
+if (null != ret) {
+return new HashMap(ret);
+} else {
+confFileEmpty = true;
+}
 }
-if(ret==null) ret = new HashMap();
-
 
-return new HashMap(ret);
-
+if (mustExist) {
+if(confFileEmpty)
+throw new RuntimeException(Config file  + name +  
doesn't have any valid storm configs);
+else
+throw new RuntimeException(Could not find config file 
on classpath  + name);
+} else {
+return new HashMap();
+}
 } catch (IOException e) {
 throw new RuntimeException(e);
+} finally {
+if (null != in) {
+try {
+in.close();
+} catch (IOException e) {
+throw new RuntimeException(e);
+}
+}
 }
 }
 
+private static InputStream getConfigFileInputStream(String 
configFilePath)
+throws IOException {
+if (null == configFilePath) {
+throw new IOException(
+Could not find config file, name not specified);
+}
+
+HashSetURL resources = new 
HashSetURL(findResources(configFilePath));
+if (resources.isEmpty()) {
+File configFile = new File(configFilePath);
+if (configFile.exists()) {
+return new FileInputStream(configFile);
+}
--- End diff --

Why not throw IOException, when path contains no actual file? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: STORM-188. Allow user to specifiy full configu...

2015-04-01 Thread lazyval
Github user lazyval commented on a diff in the pull request:

https://github.com/apache/storm/pull/495#discussion_r27562802
  
--- Diff: storm-core/src/jvm/backtype/storm/utils/Utils.java ---
@@ -135,35 +137,68 @@ public static void sleep(long millis) {
 }
 
 public static Map findAndReadConfigFile(String name, boolean 
mustExist) {
+InputStream in = null;
+Boolean confFileEmpty = false;
 try {
-HashSetURL resources = new HashSetURL(findResources(name));
-if(resources.isEmpty()) {
-if(mustExist) throw new RuntimeException(Could not find 
config file on classpath  + name);
-else return new HashMap();
-}
-if(resources.size()  1) {
-throw new RuntimeException(Found multiple  + name +  
resources. You're probably bundling the Storm jars with your topology jar. 
-  + resources);
-}
-URL resource = resources.iterator().next();
-Yaml yaml = new Yaml(new SafeConstructor());
-Map ret = null;
-InputStream input = resource.openStream();
-try {
-ret = (Map) yaml.load(new InputStreamReader(input));
-} finally {
-input.close();
+in = getConfigFileInputStream(name);
+if (null != in) {
+Yaml yaml = new Yaml(new SafeConstructor());
+Map ret = (Map) yaml.load(new InputStreamReader(in));
+if (null != ret) {
+return new HashMap(ret);
+} else {
+confFileEmpty = true;
+}
 }
-if(ret==null) ret = new HashMap();
-
 
-return new HashMap(ret);
-
+if (mustExist) {
+if(confFileEmpty)
+throw new RuntimeException(Config file  + name +  
doesn't have any valid storm configs);
+else
+throw new RuntimeException(Could not find config file 
on classpath  + name);
+} else {
+return new HashMap();
+}
 } catch (IOException e) {
 throw new RuntimeException(e);
+} finally {
+if (null != in) {
+try {
+in.close();
+} catch (IOException e) {
+throw new RuntimeException(e);
+}
+}
 }
 }
 
+private static InputStream getConfigFileInputStream(String 
configFilePath)
+throws IOException {
+if (null == configFilePath) {
+throw new IOException(
+Could not find config file, name not specified);
+}
+
+HashSetURL resources = new 
HashSetURL(findResources(configFilePath));
+if (resources.isEmpty()) {
+File configFile = new File(configFilePath);
+if (configFile.exists()) {
+return new FileInputStream(configFile);
+}
--- End diff --

Yeah, I just a bit concerned that flow is driven by both exceptions and 
flag/null.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: STORM-188. Allow user to specifiy full configu...

2015-04-01 Thread lazyval
Github user lazyval commented on a diff in the pull request:

https://github.com/apache/storm/pull/495#discussion_r27571878
  
--- Diff: storm-core/src/jvm/backtype/storm/utils/Utils.java ---
@@ -135,35 +137,68 @@ public static void sleep(long millis) {
 }
 
 public static Map findAndReadConfigFile(String name, boolean 
mustExist) {
+InputStream in = null;
+Boolean confFileEmpty = false;
--- End diff --

There are two variables types in Java: primitive (like boolean, int, long, 
short and so on), and their boxed counterparts (Boolean, Integer, Long, ...), 
which are seen as reference types. It's minor, but unless you're working with 
collections, there is no need to use later one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: STORM-188. Allow user to specifiy full configu...

2015-04-01 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/495#issuecomment-88550651
  
:+1: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-712] Storm daemons shutdown if OutOfMem...

2015-03-26 Thread lazyval
Github user lazyval commented on a diff in the pull request:

https://github.com/apache/storm/pull/468#discussion_r27231134
  
--- Diff: storm-core/src/jvm/backtype/storm/drpc/DRPCSpout.java ---
@@ -129,7 +133,9 @@ private void checkFutures() {
 public void open(Map conf, TopologyContext context, 
SpoutOutputCollector collector) {
 _collector = collector;
 if(_local_drpc_id==null) {
-_backround = Executors.newCachedThreadPool();
+_backround = new ExtendedThreadPoolExecutor(0, 
Integer.MAX_VALUE,
+60L, TimeUnit.SECONDS,
+new SynchronousQueueRunnable());
--- End diff --

You swapped out thread pool that caches threads with implementation that 
does proper exception handling, are you sure there won't be any performance 
degradations? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-712] Storm daemons shutdown if OutOfMem...

2015-03-26 Thread lazyval
Github user lazyval commented on a diff in the pull request:

https://github.com/apache/storm/pull/468#discussion_r27230992
  
--- Diff: storm-core/src/jvm/backtype/storm/utils/Utils.java ---
@@ -533,4 +533,16 @@ private static SerializationDelegate 
getSerializationDelegate(Map stormConf) {
 delegate.prepare(stormConf);
 return delegate;
 }
+
+public static void handleUncaughtException(Throwable t) {
+  if(t!= null  t instanceof OutOfMemoryError) {
+ try {
+ System.err.println(Halting due to Out Of Memory Error... + 
Thread.currentThread().getName());
+ } catch (Throwable err) {
+ //Again we done want to exit because of logging issues.
+ }
+ Runtime.getRuntime().halt(-1);
+  }
+}
--- End diff --

Why log only OOMs? Am I right that all other exceptions/errors will be 
silently swallowed? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-712] Storm daemons shutdown if OutOfMem...

2015-03-23 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/468#issuecomment-85088535
  
And you sure it's not already 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 pull request: Exhibitor support

2015-02-16 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/432#issuecomment-74532426
  
For those who wanders what exhibitor is -- [it's a zookeeper supervisor 
used for monitoring, back up and 
visualization](https://github.com/Netflix/exhibitor). 

@atdixon are you sure this properties should be kept in `storm.yaml`? Why 
should storm be aware of them? Is there a ticket backing this change (as it's 
not clear what are benefits of such support)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-391] KafkaSpout to await for the topic

2014-12-08 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/338#issuecomment-66127049
  
Guys, how about using curator api for this particular problem and raising 
new ticket *Kafka spout should rely on kafka APIs, rather than perform 
low-level manipulations with zk nodes*? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-391] KafkaSpout to await for the topic

2014-12-05 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/338#issuecomment-65802076
  
Why not use kafka api for performing such check? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-391] KafkaSpout to await for the topic

2014-12-05 Thread lazyval
Github user lazyval commented on the pull request:

https://github.com/apache/storm/pull/338#issuecomment-65808799
  
@Lewuathe there is [similar method in kafka's 
ZkUtils](https://github.com/apache/kafka/blob/0.8/core/src/main/scala/kafka/utils/ZkUtils.scala#L749-L755)


---
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.
---