From 180c51d0cf1017c872e87845cc356b8803e978cf Mon Sep 17 00:00:00 2001 From: renkai Date: Sun, 6 Sep 2015 11:47:02 +0800 Subject: [PATCH 1/4] set storm.local.dir relative values relative to storm.home rather than where user run storm command --- docs/documentation/Setting-up-a-Storm-cluster.md | 10 ++++++++-- storm-core/src/clj/backtype/storm/config.clj | 13 +++++++++---- storm-core/src/clj/backtype/storm/util.clj | 4 ++++ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/docs/documentation/Setting-up-a-Storm-cluster.md b/docs/documentation/Setting-up-a-Storm-cluster.md index 07b4edaeb36..e92061f199c 100644 --- a/docs/documentation/Setting-up-a-Storm-cluster.md +++ b/docs/documentation/Setting-up-a-Storm-cluster.md @@ -52,12 +52,18 @@ storm.zookeeper.servers: If the port that your Zookeeper cluster uses is different than the default, you should set **storm.zookeeper.port** as well. -2) **storm.local.dir**: The Nimbus and Supervisor daemons require a directory on the local disk to store small amounts of state (like jars, confs, and things like that). You should create that directory on each machine, give it proper permissions, and then fill in the directory location using this config. For example: +2) **storm.local.dir**: The Nimbus and Supervisor daemons require a directory on the local disk to store small amounts of state (like jars, confs, and things like that). + You should create that directory on each machine, give it proper permissions, and then fill in the directory location using this config. For example: ```yaml storm.local.dir: "/mnt/storm" ``` - +If you run storm on windows,it could be: +```yaml +storm.local.dir: "C:\\\\storm-local\\" +``` +If you use a relative path,it will be relative to where you installed storm(STORM_HOME). +You can leave it empty with default value `$STORM_HOME/storm-local` 3) **nimbus.host**: The worker nodes need to know which machine is the master in order to download topology jars and confs. For example: ```yaml diff --git a/storm-core/src/clj/backtype/storm/config.clj b/storm-core/src/clj/backtype/storm/config.clj index 320948bbcac..f7fabc459e1 100644 --- a/storm-core/src/clj/backtype/storm/config.clj +++ b/storm-core/src/clj/backtype/storm/config.clj @@ -131,9 +131,14 @@ ([name] (read-yaml-config true))) +(defn absolute-storm-local-dir [conf] + (let [storm-home (System/getProperty "storm.home") + path (conf STORM-LOCAL-DIR)] + (if (is-absolute-path? path) path (str storm-home file-path-separator path)))) + (defn master-local-dir [conf] - (let [ret (str (conf STORM-LOCAL-DIR) file-path-separator "nimbus")] + (let [ret (str (absolute-storm-local-dir conf) file-path-separator "nimbus")] (FileUtils/forceMkdir (File. ret)) ret)) @@ -176,7 +181,7 @@ (defn supervisor-local-dir [conf] - (let [ret (str (conf STORM-LOCAL-DIR) file-path-separator "supervisor")] + (let [ret (str (absolute-storm-local-dir conf) file-path-separator "supervisor")] (FileUtils/forceMkdir (File. ret)) ret)) @@ -233,7 +238,7 @@ )) (defn worker-user-root [conf] - (str (conf STORM-LOCAL-DIR) "/workers-users")) + (str (absolute-storm-local-dir conf) "/workers-users")) (defn worker-user-file [conf worker-id] (str (worker-user-root conf) "/" worker-id)) @@ -260,7 +265,7 @@ (defn worker-root ([conf] - (str (conf STORM-LOCAL-DIR) file-path-separator "workers")) + (str (absolute-storm-local-dir conf) file-path-separator "workers")) ([conf id] (str (worker-root conf) file-path-separator id))) diff --git a/storm-core/src/clj/backtype/storm/util.clj b/storm-core/src/clj/backtype/storm/util.clj index acd0b6bc39d..1762a5f1f24 100644 --- a/storm-core/src/clj/backtype/storm/util.clj +++ b/storm-core/src/clj/backtype/storm/util.clj @@ -18,6 +18,7 @@ (:import [java.net InetAddress]) (:import [java.util Map Map$Entry List ArrayList Collection Iterator HashMap]) (:import [java.io FileReader FileNotFoundException]) + (:import [java.nio.file Paths]) (:import [backtype.storm Config]) (:import [backtype.storm.utils Time Container ClojureTimerTask Utils MutableObject MutableInt]) @@ -58,6 +59,9 @@ (def class-path-separator (System/getProperty "path.separator")) +(defn is-absolute-path? [path] + (.isAbsolute (Paths/get path (into-array String [])))) + (defmacro defalias "Defines an alias for a var: a new var with the same root binding (if any) and similar metadata. The metadata of the alias is its initial From 0dbe4aed0f74430ec0008974895b1104794ad8d1 Mon Sep 17 00:00:00 2001 From: renkai Date: Sun, 6 Sep 2015 13:07:09 +0800 Subject: [PATCH 2/4] typo fix --- docs/documentation/Setting-up-a-Storm-cluster.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/documentation/Setting-up-a-Storm-cluster.md b/docs/documentation/Setting-up-a-Storm-cluster.md index e92061f199c..9fabb2769da 100644 --- a/docs/documentation/Setting-up-a-Storm-cluster.md +++ b/docs/documentation/Setting-up-a-Storm-cluster.md @@ -60,7 +60,7 @@ storm.local.dir: "/mnt/storm" ``` If you run storm on windows,it could be: ```yaml -storm.local.dir: "C:\\\\storm-local\\" +storm.local.dir: "C:\\storm-local" ``` If you use a relative path,it will be relative to where you installed storm(STORM_HOME). You can leave it empty with default value `$STORM_HOME/storm-local` From a4dc5fe2379a611f5c66df4e9bf77eaf507b51de Mon Sep 17 00:00:00 2001 From: renkai Date: Tue, 8 Sep 2015 14:26:41 +0800 Subject: [PATCH 3/4] add unit test --- .../test/clj/backtype/storm/config_test.clj | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/storm-core/test/clj/backtype/storm/config_test.clj b/storm-core/test/clj/backtype/storm/config_test.clj index 0d7a9630f7d..c9608909fc1 100644 --- a/storm-core/test/clj/backtype/storm/config_test.clj +++ b/storm-core/test/clj/backtype/storm/config_test.clj @@ -168,3 +168,24 @@ (catch Exception e e))))) (is (thrown-cause? java.lang.IllegalArgumentException (.validateField validator "test" 42))))))) + +(deftest test-absolute-storm-local-dir + (let [storm-home-key "storm.home" + old-storm-home (System/getProperty storm-home-key) + conf-relative {STORM-LOCAL-DIR "storm-local"} + conf-absolute {STORM-LOCAL-DIR + (if on-windows? + "C:\\storm-local" + "/var/storm-local")}] + (try + (System/setProperty storm-home-key (if on-windows? "C:\\storm-home" "/usr/local/storm-home")) + (testing + "for relative path" + (is (= (str (System/getProperty storm-home-key) file-path-separator (conf-relative STORM-LOCAL-DIR)) + (absolute-storm-local-dir conf-relative)))) + (testing + "for absolute path" + (is (= (if on-windows? "C:\\storm-local" "/var/storm-local") + (absolute-storm-local-dir conf-absolute)))) + (finally (if (not-nil? old-storm-home) + (System/setProperty storm-home-key old-storm-home)))))) \ No newline at end of file From 53516452569e1506b04e4d53bf881b95b0f171da Mon Sep 17 00:00:00 2001 From: renkai Date: Fri, 18 Sep 2015 10:49:18 +0800 Subject: [PATCH 4/4] remove the change of system property --- .../test/clj/backtype/storm/config_test.clj | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/storm-core/test/clj/backtype/storm/config_test.clj b/storm-core/test/clj/backtype/storm/config_test.clj index c9608909fc1..99032bd2bb8 100644 --- a/storm-core/test/clj/backtype/storm/config_test.clj +++ b/storm-core/test/clj/backtype/storm/config_test.clj @@ -33,7 +33,7 @@ (.validateField validator "test" x)))) (doseq [x [64 4294967296 1 nil]] - (is (nil? (try + (is (nil? (try (.validateField validator "test" x) (catch Exception e e))))))) @@ -61,7 +61,7 @@ ["42" "64"] nil ]] - (is (nil? (try + (is (nil? (try (.validateField validator "test" x) (catch Exception e e))))))) @@ -129,11 +129,11 @@ (deftest test-isolation-scheduler-machines-is-map (let [validator (CONFIG-SCHEMA-MAP ISOLATION-SCHEDULER-MACHINES)] - (is (nil? (try - (.validateField validator "test" {}) + (is (nil? (try + (.validateField validator "test" {}) (catch Exception e e)))) - (is (nil? (try - (.validateField validator "test" {"host0" 1 "host1" 2}) + (is (nil? (try + (.validateField validator "test" {"host0" 1 "host1" 2}) (catch Exception e e)))) (is (thrown-cause? java.lang.IllegalArgumentException (.validateField validator "test" 42))))) @@ -171,21 +171,16 @@ (deftest test-absolute-storm-local-dir (let [storm-home-key "storm.home" - old-storm-home (System/getProperty storm-home-key) conf-relative {STORM-LOCAL-DIR "storm-local"} conf-absolute {STORM-LOCAL-DIR (if on-windows? "C:\\storm-local" "/var/storm-local")}] - (try - (System/setProperty storm-home-key (if on-windows? "C:\\storm-home" "/usr/local/storm-home")) - (testing - "for relative path" - (is (= (str (System/getProperty storm-home-key) file-path-separator (conf-relative STORM-LOCAL-DIR)) - (absolute-storm-local-dir conf-relative)))) - (testing - "for absolute path" - (is (= (if on-windows? "C:\\storm-local" "/var/storm-local") - (absolute-storm-local-dir conf-absolute)))) - (finally (if (not-nil? old-storm-home) - (System/setProperty storm-home-key old-storm-home)))))) \ No newline at end of file + (testing + "for relative path" + (is (= (str (System/getProperty storm-home-key) file-path-separator (conf-relative STORM-LOCAL-DIR)) + (absolute-storm-local-dir conf-relative)))) + (testing + "for absolute path" + (is (= (if on-windows? "C:\\storm-local" "/var/storm-local") + (absolute-storm-local-dir conf-absolute)))))) \ No newline at end of file