You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by d2r <gi...@git.apache.org> on 2016/01/13 23:39:10 UTC

[GitHub] storm pull request: [STORM-1452] Fixes profiling/debugging out of ...

GitHub user d2r opened a pull request:

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

    [STORM-1452] Fixes profiling/debugging out of the box

    Ships the flight.bash script
    Disables UI display of Profiling and Debugging if on windows
    Changes worker.profiler.enabled to control only profiling, not other debugging actions
    Changes default of worker.profiler.enabled to false
    Adds missing UI REST API data.
    Populates user context on profiler UI routes

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

    $ git pull https://github.com/d2r/storm storm-1452-profiler-broken-by-default

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

    https://github.com/apache/storm/pull/1012.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 #1012
    
----
commit ae647a2a93af203778f273316ec6bb3f149ed56d
Author: Derek Dagit <de...@yahoo-inc.com>
Date:   2016-01-13T22:34:14Z

    Fixes profiling/debugging out of the box
    
    Ships the flight.bash script
    Disables UI display of Profiling and Debugging if on windows
    Changes worker.profiler.enabled to control only profiling, not other debugging actions
    Changes default of worker.profiler.enabled to false
    Adds missing UI REST API data.
    Populates user context on profiler UI routes

----


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:

    https://github.com/apache/storm/pull/1012#issuecomment-173361855
  
    @d2r You can just delete the .md file in question. I should have a pull request for the docs in a minute.


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:

    https://github.com/apache/storm/pull/1012#issuecomment-173268746
  
    > It looks like this needs to be upmerged, because the docs are no longer on master, so the REST API docs needs a separate pull request :(
    
    OK, I did not realize that.  I'll do so.


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:

    https://github.com/apache/storm/pull/1012#issuecomment-173363553
  
    PR for moving the docs to `asf-site`: https://github.com/apache/storm/pull/1032
    
    That includes the typo @revans2 mentioned.


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:

    https://github.com/apache/storm/pull/1012#discussion_r50280960
  
    --- Diff: docs/documentation/ui-rest-api.md ---
    @@ -548,6 +548,10 @@ Response fields:
     |boltStats.processLatency| String (double value returned in String format)  |Average time of the bolt to ack a message after it was received|
     |boltStats.acked| Long |Number of messages acked|
     |boltStats.failed| Long |Number of messages failed|
    +|profilingAndDebuggingCapable| Boolean |true if there is support for Profiling and Debugging Actions|
    +|profilActionEnabled| Boolean |true if worker profiling (Java Flight Recorder) is enabled|
    --- End diff --
    
    Good catch! Will fix 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request: [STORM-1452] Fixes profiling/debugging out of ...

Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:

    https://github.com/apache/storm/pull/1012#issuecomment-171816513
  
    Works for me. Thanks @d2r.
    
    +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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:

    https://github.com/apache/storm/pull/1012#issuecomment-171793561
  
    Not really specific to this pull request, but I missed it before when the profiling functionality was originally added.
    
    If profiling is enabled, we turn on the "Commercial Features" of the JVM:
    
    `worker.profiler.childopts: "-XX:+UnlockCommercialFeatures -XX:+FlightRecorder`
    
    The current Oracle Java license [1] states:
    
    >Some of the packages described in the Installation of Oracle Java SE Product Editions section above install Commercial Features that are restricted to Oracle Java SE Advanced, Oracle Java SE Advanced Desktop and/or Oracle Java SE Suite. **This means that even if you download an Oracle Java SE package for free under the Java BCLA, you must separately license from Oracle (and pay the appropriate license fee) the right to use any Commercial Features, described in Table 1-1 below, included in these packages.**
    
    If my interpretation of the license is correct, that means users must have purchased a license from Orace for each node that has profiling enabled. (Read the license [1] for more information and examples.)
    
    I think that's probably okay, but we may want to document that somewhere (`defaults.yaml` maybe?), so users don't accidentally run afoul of Oracle's licensing terms.
    
    [1] http://www.oracle.com/technetwork/java/javase/terms/products/index.html


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:

    https://github.com/apache/storm/pull/1012#discussion_r50281086
  
    --- Diff: storm-core/src/clj/org/apache/storm/ui/core.clj ---
    @@ -863,6 +855,7 @@
                                           (.get_eventlog_host comp-page-info)
                                           (.get_eventlog_port comp-page-info)
                                           secure?)
    +       "profilingAndDebuggingCapable" (not on-windows?)
    --- End diff --
    
    Sure.  We are using this var in several places, and I did not want to change all of them to fix this.


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:

    https://github.com/apache/storm/pull/1012#issuecomment-171805811
  
    @ptgoetz , added.  Do you think it is OK?


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:

    https://github.com/apache/storm/pull/1012#issuecomment-171802145
  
    Yes, I can add something.


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:

    https://github.com/apache/storm/pull/1012#discussion_r50319883
  
    --- Diff: storm-core/src/clj/org/apache/storm/ui/core.clj ---
    @@ -863,6 +855,7 @@
                                           (.get_eventlog_host comp-page-info)
                                           (.get_eventlog_port comp-page-info)
                                           secure?)
    +       "profilingAndDebuggingCapable" (not on-windows?)
    --- End diff --
    
    [STORM-1489](https://issues.apache.org/jira/browse/STORM-1489) created


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/storm/pull/1012#issuecomment-173268259
  
    It looks like this needs to be upmerged, because the docs are no longer on master, so the REST API docs needs a separate pull request :(


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:

    https://github.com/apache/storm/pull/1012#issuecomment-173369417
  
    > PR for moving the docs to asf-site: #1032
    > 
    > That includes the typo @revans2 mentioned.
    
    Thanks @ptgoetz !
    
    OK I have rebased the branch to 1.x-branch, and I removed the .md in that process.


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:

    https://github.com/apache/storm/pull/1012#discussion_r50280610
  
    --- Diff: storm-core/src/clj/org/apache/storm/ui/core.clj ---
    @@ -863,6 +855,7 @@
                                           (.get_eventlog_host comp-page-info)
                                           (.get_eventlog_port comp-page-info)
                                           secure?)
    +       "profilingAndDebuggingCapable" (not on-windows?)
    --- End diff --
    
    Can we file a follow on JIRA to make this a bit cleaner?  Perhaps a script for different environments that we can check at run time if the script is there?


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:

    https://github.com/apache/storm/pull/1012#discussion_r50280146
  
    --- Diff: docs/documentation/ui-rest-api.md ---
    @@ -548,6 +548,10 @@ Response fields:
     |boltStats.processLatency| String (double value returned in String format)  |Average time of the bolt to ack a message after it was received|
     |boltStats.acked| Long |Number of messages acked|
     |boltStats.failed| Long |Number of messages failed|
    +|profilingAndDebuggingCapable| Boolean |true if there is support for Profiling and Debugging Actions|
    +|profilActionEnabled| Boolean |true if worker profiling (Java Flight Recorder) is enabled|
    --- End diff --
    
    Should this be profil**e**ActionEnabled?


---
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: [STORM-1452] Fixes profiling/debugging out of ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


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