Ping!

Unless I hear back otherwise, I'll attempt to add the extra exception handling, finish up testing (sans running all tests under docker), and send out a review.

thanks,

Chris

On 8/17/17 5:15 PM, Chris Plummer wrote:
Hi TJ,

JDK-8179498 is assigned to me. I've been playing around with your changes to make sure they work fine, and plan on sponsoring the commit. I did run into one issue. It ended up being user error, but highlights a gap in the jcmd docker support. More specifically, "jcmd -l" and "jcmd <MainClass>" don't work. jcmd on the host has no visibility into the list of JVM pid's running on the docker. Normally this list is obtained by looking in /tmp/hsperfdata_<user>/* to find the list of JVM pid's for each user. However, since the docker maps to a different /tmp, jcmd on the host does not see these tmp files for docker JVMs. Thus you first need to find the pid of the JVM running on the docker using ps on the host, and then run jcmd using "jcmd <pid>". Because of this for quite a while I thought your changes were not working, but when I dug more into the attach API and learned how "jcmd -l" discovers the list of JVM pid's, I realized my error, and everything was fine when I specified a PID. This should probably be documented somewhere, at the very lead in the release notes for 10.

Regarding any cleanup of the changes, the only thing I noted was the following comment from Erik and your reply:

On Wed, May 03, 2017 at 03:14:29PM +0200, Erik Gahlin wrote:
>/I noticed thatgetNamespacePid throws IOException and InvalidPathException. />/Perhaps we want to catch those, so we can default to the original pid if />/there is a I/O related problem. /
That seems reasonable, I'll add that.
Is there a webrev that has this change? The only one I have access to is the following that David setup before this change was suggested:

http://cr.openjdk.java.net/~dholmes/8179498/webrev/

As for testing, I've verified that at least jcmd works between a docker container and its host, and I will verify that these changes don't break anything when the tool and the target JVM are on the same host. What will be a challenge is verifying that all our tests pass when the target JVM is in a docker container. I assume the tests simply spawn a JVM process to attach to, so default behavior is always same host. It is likely far from trivial to modify them to spawn the process in a docker. Any suggestions?

thanks,

Chris


Reply via email to