You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by kishorvpatil <gi...@git.apache.org> on 2015/11/02 20:03:14 UTC
[GitHub] storm pull request: [STORM-1157] Adding dynamic profiling for work...
GitHub user kishorvpatil opened a pull request:
https://github.com/apache/storm/pull/842
[STORM-1157] Adding dynamic profiling for worker, restarting worker, jstack, heap dump, and profiling
- Request restarting worker.
- Request jstack,
- Request heap dump,
- Start Profiling
- Dump profiling
- Stop Profiling
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kishorvpatil/incubator-storm profiling-worker
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/842.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 #842
----
commit 0c2021e6331cb5324a2803aff9783b539a05f63c
Author: Kishor Patil <kp...@yahoo-inc.com>
Date: 2015-11-02T18:51:58Z
Adding dynamic profiling for worker
----
---
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-1157] Adding dynamic profiling for work...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153550150
Just some nits about docs. Otherwise looks really good to me.
---
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-1157] Adding dynamic profiling for work...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153757288
+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-1157] Adding dynamic profiling for work...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153793312
@revans2 Fixed REST API documentation for consistency. Moved it to right location and added necessary link for Documentation.md
---
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-1157] Adding dynamic profiling for work...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153134999
It looks good to me. I am a little bit concerned that there is no way to turn this off, partially or all the way. So for windows where there is currently no support for this we will end up trying to run a shell script that will fail, and if someone does not have/want commercial java support from Oracle, i.e. using open JDK, or IBM JDK, etc. They have to make code modifications, not just config modifications to not show users buttons that will not work.
Could we put in something that could let users turn off part or all of this feature? At a minimum in the UI, but it would be good if nimbus could also let users know that this particular feature is not currently configured on, and if the worker could see the config and know to add in ```-XX:+UnlockCommercialFeatures -XX:+FlightRecorder``` on the command line.
Other then that I am +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-1157] Adding dynamic profiling for work...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153257989
@revans2 Thank you for the feedback. Following changes should help make this feature completely optional and pluggable.
- Added `worker.profiler.enabled` to shutdown use of profilers -
- If disabled, UI simply removes the section **Profiling and Debugging** section.
- If disabled, the supervisor would not add "worker.profiler.childopts" to worker JVM options - hence avoiding issues with jvm that does not provide commercial JProfiler.
- Added `worker.profiler.command` to specify which script to use at supervisor level so that it can be customized for different profiler scheme.
---
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-1157] Adding dynamic profiling for work...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on a diff in the pull request:
https://github.com/apache/storm/pull/842#discussion_r45083275
--- Diff: storm-core/src/clj/backtype/storm/ui/core.clj ---
@@ -986,6 +1021,127 @@
(log-message "Setting topology " id " log config " new-log-config)
(.setLogConfig nimbus id new-log-config)
(json-response (log-config id) (m "callback")))))
+
+ (GET "/api/v1/topology/:id/profiling/start/:host-port/:timeout"
+ [:as {:keys [servlet-request]} id host-port timeout & m]
+ (thrift/with-configured-nimbus-connection nimbus
+ (let [user (.getUserName http-creds-handler servlet-request)
+ topology-conf (from-json
+ (.getTopologyConf ^Nimbus$Client nimbus id))]
+ (assert-authorized-user "setWorkerProfiler" (topology-config id))
+ (assert-authorized-profiler-action "start"))
+
+ (let [[host, port] (split host-port #":")
+ nodeinfo (NodeInfo. host (set [(Long. port)]))
+ timestamp (+ (System/currentTimeMillis) (* 60000 (Long. timeout)))
+ request (ProfileRequest. nodeinfo
+ ProfileAction/JPROFILE_STOP)]
+ (.set_time_stamp request timestamp)
+ (.setWorkerProfiler nimbus id request)
+ (json-response {"status" "ok"
+ "id" host-port
+ "timeout" timeout
+ "dumplink" (worker-dump-link
+ host
+ port
+ id)}
+ (m "callback")))))
+
+ (GET "/api/v1/topology/:id/profiling/stop/:host-port"
--- End diff --
As rest of the communication between nimbus and supervisor for assignments - going through ZK answers all Thread concurrency issues. As user ProfileAction is written to ZK by nimbus ( which performs 2 phase commit). The supervisor can read old value or new depending on whether it reads is before or after the commit phase.. The supervisor reads it every 30 seconds.
---
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-1157] Adding dynamic profiling for work...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153258132
Currently, this feature lacks support for windows and command-line interface.
---
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-1157] Adding dynamic profiling for work...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153492437
@zhuoliu I moved docs under documentation location. Also, added pointers from main page.
---
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-1157] Adding dynamic profiling for work...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153789535
The changes look good and I am +1 on merging this in. I had one minor comment on the documentation, but it is not critical and I am happy to have this merged in, with or without the change.
---
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-1157] Adding dynamic profiling for work...
Posted by hustfxj <gi...@git.apache.org>.
Github user hustfxj commented on a diff in the pull request:
https://github.com/apache/storm/pull/842#discussion_r45024551
--- Diff: storm-core/src/clj/backtype/storm/ui/core.clj ---
@@ -986,6 +1021,127 @@
(log-message "Setting topology " id " log config " new-log-config)
(.setLogConfig nimbus id new-log-config)
(json-response (log-config id) (m "callback")))))
+
+ (GET "/api/v1/topology/:id/profiling/start/:host-port/:timeout"
+ [:as {:keys [servlet-request]} id host-port timeout & m]
+ (thrift/with-configured-nimbus-connection nimbus
+ (let [user (.getUserName http-creds-handler servlet-request)
+ topology-conf (from-json
+ (.getTopologyConf ^Nimbus$Client nimbus id))]
+ (assert-authorized-user "setWorkerProfiler" (topology-config id))
+ (assert-authorized-profiler-action "start"))
+
+ (let [[host, port] (split host-port #":")
+ nodeinfo (NodeInfo. host (set [(Long. port)]))
+ timestamp (+ (System/currentTimeMillis) (* 60000 (Long. timeout)))
+ request (ProfileRequest. nodeinfo
+ ProfileAction/JPROFILE_STOP)]
+ (.set_time_stamp request timestamp)
+ (.setWorkerProfiler nimbus id request)
+ (json-response {"status" "ok"
+ "id" host-port
+ "timeout" timeout
+ "dumplink" (worker-dump-link
+ host
+ port
+ id)}
+ (m "callback")))))
+
+ (GET "/api/v1/topology/:id/profiling/stop/:host-port"
--- End diff --
I am concern about Thread concurrency. If user is setting the worker's ProfileAction, supervisor is reading the worker's ProfileAction. Then what happened？
---
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-1157] Adding dynamic profiling for work...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/842
---
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-1157] Adding dynamic profiling for work...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/842#discussion_r43905552
--- Diff: STORM-UI-REST-API.md ---
@@ -636,6 +636,201 @@ Sample response:
}
```
+## Profiling and Debugging GET Operations
+
+### /api/v1/topology/:id/profiling/start/:host-port/:timeout (GET)
+
+Request to start profiler on worker with timeout. Returns status and link to profiler artifacts for worker.
+
+|Parameter |Value |Description |
+|----------|--------|-------------|
+|id |String (required)| Topology Id |
+|:host-port |String (required)| Worker Id |
+|:timeout |String (required)| Time out for profiler to stop in minutes |
+
+Response fields:
+
+|Field |Value |Description|
+|----- |----- |-----------|
+|id | String | Worker id|
+|status | String | Response Status |
+|timeout | String | Requested timeout
+|dumplink | String | Link to logviewer URL for worker profiler documents.|
+
+Examples:
+
+```no-highlight
+1. http://ui-daemon-host-name:8080/api/v1/topology/wordcount-1-1446614150/profiling/start/10.11.1.7:6701/10
+2. http://ui-daemon-host-name:8080/api/v1/topology/wordcount-1-1446614150/profiling/start/10.11.1.7:6701/5
+3. http://ui-daemon-host-name:8080/api/v1/topology/wordcount-1-1446614150/profiling/start/10.11.1.7:6701/20
+```
+
+Sample response:
+
+```json
+{
+ "status": "ok",
+ "id": "10.11.1.7:6701",
+ "timeout": "10",
+ "dumplink": "http:\/\/10.11.1.7:8000\/dumps\/wordcount-1-1446614150\/10.11.1.7%3A6701"
+}
+```
+
+### /api/v1/topology/:id/profiling/dumpprofile/:host-port (GET)
+
+Request to dump profiler recording on worker. Returns status and worker id for the request.
+
+|Parameter |Value |Description |
+|----------|--------|-------------|
+|id |String (required)| Topology Id |
--- End diff --
Here too.
---
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-1157] Adding dynamic profiling for work...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153374899
+1 it looks like a great addition.
---
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-1157] Adding dynamic profiling for work...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/842#discussion_r43835235
--- Diff: storm-core/src/clj/backtype/storm/ui/core.clj ---
@@ -986,6 +1021,127 @@
(log-message "Setting topology " id " log config " new-log-config)
(.setLogConfig nimbus id new-log-config)
(json-response (log-config id) (m "callback")))))
+
+ (GET "/api/v1/topology/:id/profiling/start/:host-port/:timeout"
--- End diff --
We may need to update the REST API doc for 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-1157] Adding dynamic profiling for work...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/842#discussion_r43905522
--- Diff: STORM-UI-REST-API.md ---
@@ -636,6 +636,201 @@ Sample response:
}
```
+## Profiling and Debugging GET Operations
+
+### /api/v1/topology/:id/profiling/start/:host-port/:timeout (GET)
+
+Request to start profiler on worker with timeout. Returns status and link to profiler artifacts for worker.
+
+|Parameter |Value |Description |
+|----------|--------|-------------|
+|id |String (required)| Topology Id |
--- End diff --
Do we want an `:id` here to be consistent with the other Parameters?
---
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-1157] Adding dynamic profiling for work...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/842#discussion_r43805080
--- Diff: docs/DYNAMIC_WORKER_PROFILING.md ---
@@ -0,0 +1,29 @@
+Dynamic Worker Profiling
--- End diff --
Should we move this doc file to docs/documentation directory?
Also the DYNAMIC_LOG_LEVEL_SETTINGS.md 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1157] Adding dynamic profiling for work...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153794637
Looks good +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-1157] Adding dynamic profiling for work...
Posted by zhuoliu <gi...@git.apache.org>.
Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/842#discussion_r43835385
--- Diff: storm-core/src/clj/backtype/storm/ui/core.clj ---
@@ -986,6 +1021,127 @@
(log-message "Setting topology " id " log config " new-log-config)
(.setLogConfig nimbus id new-log-config)
(json-response (log-config id) (m "callback")))))
+
+ (GET "/api/v1/topology/:id/profiling/start/:host-port/:timeout"
+ [:as {:keys [servlet-request]} id host-port timeout & m]
+ (thrift/with-configured-nimbus-connection nimbus
+ (let [user (.getUserName http-creds-handler servlet-request)
+ topology-conf (from-json
+ (.getTopologyConf ^Nimbus$Client nimbus id))]
+ (assert-authorized-user "setWorkerProfiler" (topology-config id))
+ (assert-authorized-profiler-action "start"))
+
+ (let [[host, port] (split host-port #":")
+ nodeinfo (NodeInfo. host (set [(Long. port)]))
+ timestamp (+ (System/currentTimeMillis) (* 60000 (Long. timeout)))
+ request (ProfileRequest. nodeinfo
+ ProfileAction/JPROFILE_STOP)]
+ (.set_time_stamp request timestamp)
+ (.setWorkerProfiler nimbus id request)
+ (json-response {"status" "ok"
+ "id" host-port
+ "timeout" timeout
+ "dumplink" (worker-dump-link
+ host
+ port
+ id)}
+ (m "callback")))))
+
+ (GET "/api/v1/topology/:id/profiling/stop/:host-port"
--- End diff --
Same here and below.
---
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-1157] Adding dynamic profiling for work...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153595069
@zhuoliu I have updated the REST API documentation for all new endpoints.
---
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-1157] Adding dynamic profiling for work...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the pull request:
https://github.com/apache/storm/pull/842#issuecomment-153258716
<img width="1354" alt="dynamic_profiling_debugging_1" src="https://cloud.githubusercontent.com/assets/6090397/10902398/ec51a46a-81c2-11e5-92f5-d4c79d0b80df.png">
<img width="1366" alt="dynamic_profiling_debugging_2" src="https://cloud.githubusercontent.com/assets/6090397/10902399/ef7e868a-81c2-11e5-9f1d-ce85c33afa3e.png">
<img width="731" alt="dynamic_profiling_debugging_3" src="https://cloud.githubusercontent.com/assets/6090397/10902400/f18d7c6a-81c2-11e5-96a7-f5a7fc11a98f.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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1157] Adding dynamic profiling for work...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/842#discussion_r43905569
--- Diff: STORM-UI-REST-API.md ---
@@ -636,6 +636,201 @@ Sample response:
}
```
+## Profiling and Debugging GET Operations
+
+### /api/v1/topology/:id/profiling/start/:host-port/:timeout (GET)
+
+Request to start profiler on worker with timeout. Returns status and link to profiler artifacts for worker.
+
+|Parameter |Value |Description |
+|----------|--------|-------------|
+|id |String (required)| Topology Id |
+|:host-port |String (required)| Worker Id |
+|:timeout |String (required)| Time out for profiler to stop in minutes |
+
+Response fields:
+
+|Field |Value |Description|
+|----- |----- |-----------|
+|id | String | Worker id|
+|status | String | Response Status |
+|timeout | String | Requested timeout
+|dumplink | String | Link to logviewer URL for worker profiler documents.|
+
+Examples:
+
+```no-highlight
+1. http://ui-daemon-host-name:8080/api/v1/topology/wordcount-1-1446614150/profiling/start/10.11.1.7:6701/10
+2. http://ui-daemon-host-name:8080/api/v1/topology/wordcount-1-1446614150/profiling/start/10.11.1.7:6701/5
+3. http://ui-daemon-host-name:8080/api/v1/topology/wordcount-1-1446614150/profiling/start/10.11.1.7:6701/20
+```
+
+Sample response:
+
+```json
+{
+ "status": "ok",
+ "id": "10.11.1.7:6701",
+ "timeout": "10",
+ "dumplink": "http:\/\/10.11.1.7:8000\/dumps\/wordcount-1-1446614150\/10.11.1.7%3A6701"
+}
+```
+
+### /api/v1/topology/:id/profiling/dumpprofile/:host-port (GET)
+
+Request to dump profiler recording on worker. Returns status and worker id for the request.
+
+|Parameter |Value |Description |
+|----------|--------|-------------|
+|id |String (required)| Topology Id |
+|:host-port |String (required)| Worker Id |
+
+Response fields:
+
+|Field |Value |Description|
+|----- |----- |-----------|
+|id | String | Worker id|
+|status | String | Response Status |
+
+Examples:
+
+```no-highlight
+1. http://ui-daemon-host-name:8080/api/v1/topology/wordcount-1-1446614150/profiling/dumpprofile/10.11.1.7:6701
+```
+
+Sample response:
+
+```json
+{
+ "status": "ok",
+ "id": "10.11.1.7:6701",
+}
+```
+
+### /api/v1/topology/:id/profiling/stop/:host-port (GET)
+
+Request to stop profiler on worker. Returns status and worker id for the request.
+
+|Parameter |Value |Description |
+|----------|--------|-------------|
+|id |String (required)| Topology Id |
--- End diff --
And here
---
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.
---