YARN-11823: add new endpoints for getting jstacks of ApplicationId and NodeManager process#8123
YARN-11823: add new endpoints for getting jstacks of ApplicationId and NodeManager process#8123Hean-Chhinling wants to merge 26 commits intoapache:trunkfrom
Conversation
…xception response in json format
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
…oints to enable or disable
|
💔 -1 overall
This message was automatically generated. |
...hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
Outdated
Show resolved
Hide resolved
...hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
Show resolved
Hide resolved
|
|
||
| /** Flag to turn on/off jstack endpoints for NodeManager. By default is True **/ | ||
| public static final String NM_JSTACK_ENDPOINTS_ENABLED = | ||
| NM_PREFIX + "jstack-endpoints.enabled"; |
There was a problem hiding this comment.
I think diagnostic-api instead of jstack-endpoints would be more suitable here.
There was a problem hiding this comment.
hmmm...since this configuration only control the jstack endpoints, so I think it is better to name it jstack instead of diagnostic.
May I know why do you want to name it diagnostic-api?
It seems to general to me.
There was a problem hiding this comment.
If later someone would like to add other functionality to the endpoint a more generic name can be useful here.
There was a problem hiding this comment.
but yes, based on the property desc we can keep as it is
There was a problem hiding this comment.
Okay, i see your point. I will keep it as it is since this is only for the jstack maybe in the future a new configuration shall be added or refactored to combine both together maybe jmap or something.
.../src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/DiagnosticJStackService.java
Show resolved
Hide resolved
.../src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/DiagnosticJStackService.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/DiagnosticJStackService.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/DiagnosticJStackService.java
Outdated
Show resolved
Hide resolved
…default jstack endpoint to false
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
All the endpoints failed with 404 as of now. Not sure why this is happening
Maybe of not running with the up-to-date hadoop build |
|
💔 -1 overall
This message was automatically generated. |
|
Next step is to create unit-test for DiagnosticJstackService class:
|
|
💔 -1 overall
This message was automatically generated. |
...odemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/NMWebServices.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/DiagnosticJStackService.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/DiagnosticJStackService.java
Outdated
Show resolved
Hide resolved
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
ApplicationJStack_application_1772624045426.txt |
|
💔 -1 overall
This message was automatically generated. |
.../src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/DiagnosticJStackService.java
Show resolved
Hide resolved
|
|
||
| List<Long> javaContainerPids = ProcessHandle.of(parentPid).stream() | ||
| .flatMap(ProcessHandle::descendants) | ||
| .filter(childProcess -> childProcess.info().command().orElse("").contains("java")) |
There was a problem hiding this comment.
Now only java based container checked with jstack what is good.
However now if a container is not java proc, and user wants to debug it with the API, the container wont be present in the response what can be confusing.
Instead if skipping non java based containers, can you please add them to the response and marked them there like not java containers so jstack can not be done for them?
There was a problem hiding this comment.
Good idea, i will put those non-java container in the response.
There was a problem hiding this comment.
also maybe there is a more robust way to check what is java process:
can you please check this?
https://docs.oracle.com/javase/7/docs/technotes/tools/share/jps.html
...odemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/NMWebServices.java
Show resolved
Hide resolved
...odemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/NMWebServices.java
Outdated
Show resolved
Hide resolved
...odemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/NMWebServices.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
…ainerId in the response
…ion when running jstack for specific pid in case it exited during running
Test in Secured ClusterACL Checks for knox ui user not authorised https://{nm}:8443/gateway/cdp-proxy/yarn-nm/ws/v1/node/apps/application_1774020354194_0001/jstack/3
After adding ‘knoxui’ at ‘yarn.admin.acl’ Next steps:
|
|
💔 -1 overall
This message was automatically generated. |
| this.conf = context.getConf(); | ||
| } | ||
|
|
||
| public String collectNodeThreadDump(int numberOfJStack) throws IOException { |
There was a problem hiding this comment.
In case of NM stack i think we should check user is admin or not
There was a problem hiding this comment.
Hmm...I see, so only an admin should be able to run the jstack for NM?
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Description of PR
Add two new endpoints to ease the process of collecting JStack for running Application and Node.
{nm_address}/ws/v1/node/apps/{app_id}/jstack
{nm_address}/ws/v1/node/jstack
How was this patch tested?
Manually tested.
application_jstack_response.txt.rtf
NodeManager_Jstack_Shell.txt
when set
nodemanager.jstack-endpoints.enabled = false;For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?