You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by mproch <gi...@git.apache.org> on 2016/02/18 19:32:47 UTC
[GitHub] storm pull request: closes STORM-1545
GitHub user mproch opened a pull request:
https://github.com/apache/storm/pull/1123
closes STORM-1545
Use ConfigUtils.workerArtifactsRoot to determine correct path
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mproch/storm STORM-1545
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/1123.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 #1123
----
commit dca0e8476f5e55e5b8f8f064e9e47cfd8c3e27c5
Author: Maciek Próchniak <mp...@touk.pl>
Date: 2016-02-18T18:16:12Z
STORM-1545
commit 02ae42ebe8e42da7a567c2a49309dd6ca4aa5155
Author: Maciek Próchniak <mp...@touk.pl>
Date: 2016-02-18T18:32:18Z
STORM-1545 - more common 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: closes STORM-1545 and STORM-1552
Posted by mproch <gi...@git.apache.org>.
Github user mproch commented on the pull request:
https://github.com/apache/storm/pull/1123#issuecomment-186895540
I removed relative path handling and updated the title - hope this is what you meant?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: closes STORM-1545
Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on a diff in the pull request:
https://github.com/apache/storm/pull/1123#discussion_r53427904
--- Diff: storm-core/src/jvm/org/apache/storm/metric/FileBasedEventLogger.java ---
@@ -79,26 +80,18 @@ public void run() {
@Override
public void prepare(Map stormConf, TopologyContext context) {
- String logDir; // storm local directory
String stormId = context.getStormId();
int port = context.getThisWorkerPort();
- if ((logDir = System.getProperty("storm.local.dir")) == null &&
- (logDir = (String)stormConf.get("storm.local.dir")) == null) {
- String msg = "Could not determine the directory to log events.";
- LOG.error(msg);
- throw new RuntimeException(msg);
- } else {
- LOG.info("FileBasedEventLogger log directory {}.", logDir);
- }
+
+ String workersArtifactRoot = ConfigUtils.workerArtifactsRoot(stormConf, stormId, port);
/*
* Include the topology name & worker port in the file name so that
* multiple event loggers can log independently.
*/
- Path path = Paths.get(logDir, "workers-artifacts", stormId, Integer.toString(port), "events.log");
+ Path path = Paths.get(workersArtifactRoot, "events.log");
if (!path.isAbsolute()) {
--- End diff --
[ConfigUtils](https://github.com/apache/storm/blob/master/storm-core/src/jvm/org/apache/storm/utils/ConfigUtils.java#L407) appears to take care of the relative path, so this check could be removed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: closes STORM-1545
Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on the pull request:
https://github.com/apache/storm/pull/1123#issuecomment-186090463
Can you update the PR title ?
Theres a PR https://github.com/apache/storm/pull/1109 for the same issue but it does not make use of the ConfigUtils since it had to be merged to 1.x as well. Since this PR is a better fix for 2.0, I will close https://github.com/apache/storm/pull/1109 and raise that against 1.x-branch.
+1 to merge against master (not 1.x-branch) once review comment is addressed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: closes STORM-1545 and STORM-1552
Posted by mproch <gi...@git.apache.org>.
Github user mproch commented on the pull request:
https://github.com/apache/storm/pull/1123#issuecomment-189144444
@HeartSaVioR
thanks, I resolved jira issue.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: closes STORM-1545 and STORM-1552
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/1123#issuecomment-189074528
@mproch
Thanks for the work! I merged this into master.
I'd like to also set assignee from JIRA issue to you but I couldn't find your name in Apache JIRA.
If you have your account in JIRA, please do the followings,
- set assignee to yourself at https://issues.apache.org/jira/browse/STORM-1545
- resolve issue as 'resolved'
- set fix version/s to 2.0.0
Please let me know if you're not interested to do the things. I'll just assign assignee as reporter and resolve issue.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: closes STORM-1545 and STORM-1552
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/1123#issuecomment-189065774
+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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: closes STORM-1545 and STORM-1552
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the pull request:
https://github.com/apache/storm/pull/1123#issuecomment-187033903
Thanks @mproch Looks good to me (NB) for merge with 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
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: closes STORM-1545 and STORM-1552
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/1123
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: closes STORM-1545 and STORM-1552
Posted by satishd <gi...@git.apache.org>.
Github user satishd commented on the pull request:
https://github.com/apache/storm/pull/1123#issuecomment-187024988
https://issues.apache.org/jira/browse/STORM-1566 is raised for the above issue.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: closes STORM-1545 and STORM-1552
Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on the pull request:
https://github.com/apache/storm/pull/1123#issuecomment-187022132
When the unit tests are run it creates log files under /logs. It should ideally be under system tmp directory.
Maybe unrelated, but when an example topology is submitted worker crashes with the below error,
`2016-02-22 10:56:51.085 o.a.s.d.worker [ERROR] Error on initialization of server mk-worker
java.lang.ClassCastException: java.lang.String cannot be cast to java.io.File
at org.apache.storm.daemon.worker$fn__6265$exec_fn__1730__auto____6266.invoke(worker.clj:622) ~[storm-core-2.0.0-SNAPSHOT.jar:2.0.0-SNAPSHOT]
at clojure.lang.AFn.applyToHelper(AFn.java:178) ~[clojure-1.7.0.jar:?]
at clojure.lang.AFn.applyTo(AFn.java:144) ~[clojure-1.7.0.jar:?]
at clojure.core$apply.invoke(core.clj:630) ~[clojure-1.7.0.jar:?]
at org.apache.storm.daemon.worker$fn__6265$mk_worker__6362.doInvoke(worker.clj:613) [storm-core-2.0.0-SNAPSHOT.jar:2.0.0-SNAPSHOT]
at clojure.lang.RestFn.invoke(RestFn.java:512) [clojure-1.7.0.jar:?]
at org.apache.storm.daemon.worker$_main.invoke(worker.clj:810) [storm-core-2.0.0-SNAPSHOT.jar:2.0.0-SNAPSHOT]
at clojure.lang.AFn.applyToHelper(AFn.java:165) [clojure-1.7.0.jar:?]
at clojure.lang.AFn.applyTo(AFn.java:144) [clojure-1.7.0.jar:?]
at org.apache.storm.daemon.worker.main(Unknown Source) [storm-core-2.0.0-SNAPSHOT.jar:2.0.0-SNAPSHOT]`
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---