[GitHub] storm pull request: [STORM-954] Topology event inspector
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...
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...
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
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.
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.
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...
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...
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...
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
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...
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...
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...
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.
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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
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
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
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
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. ---