K0K0V0K commented on code in PR #8123: URL: https://github.com/apache/hadoop/pull/8123#discussion_r2797582209
########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/DiagnosticJStackService.java: ########## @@ -0,0 +1,134 @@ +/** * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.yarn.server.nodemanager.webapp; + +import org.apache.hadoop.util.Shell; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +public class DiagnosticJStackService { + + private static final Logger LOG = LoggerFactory + .getLogger(DiagnosticJStackService.class); + Review Comment: Please move the default constructor to private ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/DiagnosticJStackService.java: ########## @@ -0,0 +1,134 @@ +/** * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.yarn.server.nodemanager.webapp; + +import org.apache.hadoop.util.Shell; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +public class DiagnosticJStackService { + + private static final Logger LOG = LoggerFactory + .getLogger(DiagnosticJStackService.class); + + public static String collectNodeThreadDump(int numberOfJStack) + throws Exception { Review Comment: Please use specific exceptions instead of general one. ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java: ########## @@ -2315,6 +2315,11 @@ public static boolean isAclEnabled(Configuration conf) { public static final String LINUX_CONTAINER_RUNTIME_PREFIX = NM_PREFIX + "runtime.linux."; + /** 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"; Review Comment: I think `diagnostic-api` instead of `jstack-endpoints` would be more suitable here. ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java: ########## @@ -2315,6 +2315,11 @@ public static boolean isAclEnabled(Configuration conf) { public static final String LINUX_CONTAINER_RUNTIME_PREFIX = NM_PREFIX + "runtime.linux."; + /** Flag to turn on/off jstack endpoints for NodeManager. By default is True **/ + public static final String NM_JSTACK_ENDPOINTS_ENABLED = Review Comment: This should be added to the `yarn-default.xml` ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/DiagnosticJStackService.java: ########## @@ -0,0 +1,134 @@ +/** * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.yarn.server.nodemanager.webapp; + +import org.apache.hadoop.util.Shell; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +public class DiagnosticJStackService { + + private static final Logger LOG = LoggerFactory + .getLogger(DiagnosticJStackService.class); + + public static String collectNodeThreadDump(int numberOfJStack) + throws Exception { + if (Shell.WINDOWS) { + throw new UnsupportedOperationException("Not implemented for Windows"); + } + + List<String> nodeManagerPids = getNodeManagerPids(); + + return runJStack(nodeManagerPids, numberOfJStack); + + } + + + + public static String collectApplicationThreadDump(String appId, int numberOfJStack) + throws Exception { + if (Shell.WINDOWS) { + throw new UnsupportedOperationException("Not implemented for Windows."); + } + List<String> applicationPids = getApplicationPids(appId); + + return runJStack(applicationPids, numberOfJStack); + } + + + public static List<String> getNodeManagerPids() throws IOException { + Shell.ShellCommandExecutor cmd = new Shell.ShellCommandExecutor( + new String[]{ + "bash", + "-c", + "ps aux | grep nodemanager | grep -v grep" Review Comment: what if other `*nodemanager*` process is running on the host? ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/DiagnosticJStackService.java: ########## @@ -0,0 +1,134 @@ +/** * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.yarn.server.nodemanager.webapp; + +import org.apache.hadoop.util.Shell; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +public class DiagnosticJStackService { + + private static final Logger LOG = LoggerFactory + .getLogger(DiagnosticJStackService.class); + + public static String collectNodeThreadDump(int numberOfJStack) + throws Exception { + if (Shell.WINDOWS) { + throw new UnsupportedOperationException("Not implemented for Windows"); + } + + List<String> nodeManagerPids = getNodeManagerPids(); + + return runJStack(nodeManagerPids, numberOfJStack); + + } + + + + public static String collectApplicationThreadDump(String appId, int numberOfJStack) + throws Exception { + if (Shell.WINDOWS) { + throw new UnsupportedOperationException("Not implemented for Windows."); + } + List<String> applicationPids = getApplicationPids(appId); + + return runJStack(applicationPids, numberOfJStack); + } + + + public static List<String> getNodeManagerPids() throws IOException { + Shell.ShellCommandExecutor cmd = new Shell.ShellCommandExecutor( + new String[]{ + "bash", + "-c", + "ps aux | grep nodemanager | grep -v grep" + }, + null, + null, + 10_000 + ); + cmd.execute(); + return extractPids(cmd.getOutput()); + } + + public static List<String> getApplicationPids(String appId) throws IOException { + String psCmd = "ps aux | grep jvm/java | grep " + appId + " | grep -v -e /bin/bash -e grep"; Review Comment: If user provide appId like "app_123; rm -rf /" what will happen? ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java: ########## @@ -2315,6 +2315,11 @@ public static boolean isAclEnabled(Configuration conf) { public static final String LINUX_CONTAINER_RUNTIME_PREFIX = NM_PREFIX + "runtime.linux."; + /** 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"; + public static final boolean DEFAULT_NM_JSTACK_ENDPOINTS_ENABLED = true; Review Comment: I think for security reasons this should be false by default. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
