[GitHub] storm pull request: STORM-1213: Remove sigar binaries from source ...

2015-11-19 Thread revans2 
Github user revans2 commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-158199509
  
@ptgoetz 
+1 the changes look good and my manual tests all pass.

I Agree the it kind of ugly.  Ideally what we need to do is to talk to 
sigar and see if they can update their native jar to do what we want.  It 
should be fairly simple to have them remove the version number from the jar.


---
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-1213: Remove sigar binaries from source ...

2015-11-19 Thread ptgoetz 
Github user ptgoetz commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-158184985
  
@revans2 I couldn't get the maven dependency plugin to do what I wanted, so 
I changed tactics. I switched to using the maven antrun plugin to download and 
unpack the sigar native binaries. It downloads the archive to the local maven 
repository alongside the other sigar dependencies so it only needs to download 
it once.

The good news is that allowed me to use the full binary archive, so it 
includes support for windows and other OSes/architectures. On the down side 
it's somewhat brittle compared to using "pure" maven, and IMO is kind of ugly. 
It's also hard-coded to use google code, which is supposedly going away at some 
point, but that's the only place I could find the archive with a published 
checksum.


---
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-1213: Remove sigar binaries from source ...

2015-11-19 Thread revans2 
Github user revans2 commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-158094151
  
@ptgoetz Yes you are correct about the changes.


---
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-1213: Remove sigar binaries from source ...

2015-11-18 Thread jerrypeng 
Github user jerrypeng commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-157827199
  
+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-1213: Remove sigar binaries from source ...

2015-11-18 Thread revans2 
Github user revans2 commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-157843581
  
+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-1213: Remove sigar binaries from source ...

2015-11-18 Thread revans2 
Github user revans2 commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-157858618
  
Sorry about the comments after giving it a +1.  I manually ran 
LatencyVsThroughput and saw that it was not working.


---
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-1213: Remove sigar binaries from source ...

2015-11-18 Thread revans2 
Github user revans2 commented on a diff in the pull request:

https://github.com/apache/storm/pull/887#discussion_r45257318
  
--- Diff: external/storm-metrics/pom.xml ---
@@ -55,4 +62,28 @@
   provided
 
   
+  
+
+  
+org.apache.maven.plugins
+maven-dependency-plugin
+2.10
+
+  
+unpack-dependencies
+generate-resources
+
+  unpack-dependencies
+
+
+  
${project.build.directory}/classes/resources/resources
--- End diff --

This is the line causing the issue.  It should just be 
`${project.build.directory}/classes/resources`


---
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-1213: Remove sigar binaries from source ...

2015-11-18 Thread revans2 
Github user revans2 commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-157862181
  
Even with changing the directory I am getting 
```
java.lang.UnsatisfiedLinkError: org.hyperic.sigar.Sigar.getPid()J
at org.hyperic.sigar.Sigar.getPid(Native Method) 
~[stormjar.jar:0.11.0-SNAPSHOT]
at org.apache.storm.metrics.sigar.CPUMetric.(CPUMetric.java:38) 
~[stormjar.jar:0.11.0-SNAPSHOT]
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native 
Method) ~[?:1.8.0]
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
 ~[?:1.8.0]
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
 ~[?:1.8.0]
at java.lang.reflect.Constructor.newInstance(Constructor.java:408) 
~[?:1.8.0]
at java.lang.Class.newInstance(Class.java:433) ~[?:1.8.0]
at backtype.storm.utils.Utils.newInstance(Utils.java:91) 
~[storm-core-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
at 
backtype.storm.metric.SystemBolt.registerMetrics(SystemBolt.java:150) 
~[storm-core-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
at backtype.storm.metric.SystemBolt.prepare(SystemBolt.java:143) 
~[storm-core-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
at 
backtype.storm.daemon.executor$fn__5157$fn__5170.invoke(executor.clj:777) 
~[storm-core-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
at backtype.storm.util$async_loop$fn__556.invoke(util.clj:480) 
[storm-core-0.11.0-SNAPSHOT.jar:0.11.0-SNAPSHOT]
at clojure.lang.AFn.run(AFn.java:22) [clojure-1.7.0.jar:?]
at java.lang.Thread.run(Thread.java:744) [?:1.8.0]
```

We need to do some more work to understand the difference between this 
patch and what we were doing before.


---
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-1213: Remove sigar binaries from source ...

2015-11-18 Thread kishorvpatil 
Github user kishorvpatil commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-157705671
  
+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-1213: Remove sigar binaries from source ...

2015-11-18 Thread revans2 
Github user revans2 commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-157876251
  
yes that is it.  When I moved 
`resources/libsigar-universal64-macosx-1.6.4.dylib` to 
`resources/libsigar-universal64-macosx.dylib` and reran it all worked.


---
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-1213: Remove sigar binaries from source ...

2015-11-18 Thread revans2 
Github user revans2 commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-157873921
  
In master 
https://github.com/apache/storm/tree/master/external/storm-metrics/src/main/resources
 gets mapped into the base directory of the jar.  So 
https://github.com/apache/storm/tree/master/external/storm-metrics/src/main/resources/resources
 goes to jar://storm-merics.jar/resources/

For my tests I launch a single node cluster

```
$ mvn clean install -DskipTests -Drat.skip
$ cd storm-dist/binary
$ mvn clean package
$ tar -xzvf ./target/*.tar.gz
$ cd apache-strom*
$ ./bin/storm dev-zookeeper & #nimbus, ui, logviewer, supervisor,
$ ./bin/storm jar 
./examples/storm-starter/storm-starter-topologies-0.11.0-SNAPSHOT.jar 
storm.starter.ThroughputVsLatency 2000 1 5
```


---
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-1213: Remove sigar binaries from source ...

2015-11-18 Thread ptgoetz 
Github user ptgoetz commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-157880047
  
@revans2 Ah... okay. So we need to change `resources/resources` to just 
`resources`, and remove the `-1.6.4` from the libraries, correct?

That should be doable with the dependency plugin.


---
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-1213: Remove sigar binaries from source ...

2015-11-18 Thread revans2 
Github user revans2 commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-157874959
  
I just did a diff of the contents of the two jars, old and new(this patch 
plus me moving the native libraries) and I came up with the following. Around 
the max native pieces

```
Only in ./new/resources: libsigar-universal64-macosx-1.6.4.dylib
Only in ./old/resources: libsigar-universal-macosx.dylib
Only in ./old/resources: libsigar-universal64-macosx.dylib
```
It looks like the version number in the dylib might be messing up the 
linking somehow.



---
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-1213: Remove sigar binaries from source ...

2015-11-18 Thread ptgoetz 
Github user ptgoetz commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-157868784
  
@revans2 How are you running it (i.e. local/remote/unit test)? My first 
thought is that we may just need to bind the dependency plugin to a different 
maven lifecycle.


---
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-1213: Remove sigar binaries from source ...

2015-11-18 Thread ptgoetz 
Github user ptgoetz commented on the pull request:

https://github.com/apache/storm/pull/887#issuecomment-157869384
  
@revans2 That directory structure is what's in master right now:


https://github.com/apache/storm/tree/master/external/storm-metrics/src/main/resources/resources


---
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-1213: Remove sigar binaries from source ...

2015-11-17 Thread ptgoetz 
GitHub user ptgoetz opened a pull request:

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

STORM-1213: Remove sigar binaries from source tree

See https://issues.apache.org/jira/browse/STORM-1213 for description.

Also removes the LICENSE entry for sigar, since I believe that is 
unnecessary since it is Apache licensed.

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

$ git pull https://github.com/ptgoetz/storm storm-metrics-fix

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

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


commit 83eced3329dc62c685aa6784792506766343fd52
Author: P. Taylor Goetz 
Date:   2015-11-16T22:17:19Z

remove sigar binaries from source tree and delete unnecessary LICENSE entry




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