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