Hi Bob,
Some initial discussion and commentary. I'm a bit concerned by how much
accommodating docker penalises code that is not using containers.
On 12/01/2018 4:41 AM, Bob Vandette wrote:
Bug:
https://bugs.openjdk.java.net/browse/JDK-8193710
Webrev:
http://cr.openjdk.java.net/~bobv/8193710/webrev.00/
Summary:
This changeset enables the ability to use jcmd and jps running on a Host to
list the java processes that are running in docker (cgroup based) containers.
Can you clarify this for me please. I assume we are still only listing
processes for the current user - correct? So the issue is if I am
running mutiple JVMs and also running a Docker process that itself
contains JVMs then jps outside of Docker won't show the JVMs that are
inside Docker. And presumably the opposite is also true?
If we change jps to find all JVMs on the host for the current user,
whether in a container or not (and whether jps is run from in a
container or not?) are the process id's that get returned directly
useable from where jps was run?
I’ve tested this change by examining processes running as root on both host and
in
docker containers as well as under my userid using “jps and jcmd -l”.
I’ve also tested the getFile functions with a small example program that I
wrote.
From what I can tell, these two getFile functions are not used in the JDK
sources
and in fact the PerfDataFile.getFile(int lvmid) method doesn’t appear to have
never worked!
I’d really like to remove these two methods.
If they are not used by anything then by all means drop them.
140 candidate = new File(f.getName(), name); <<——— This line
drops the /tmp directory off of the path.
Here are some implementation details that I’ve added added to one of the Unix
specific source files
(src/java.base/unix/classes/jdk/internal/vm/VMSupportImpl.java)
I have an issue with this. This is not "unix" (ie anything other than
windows) support code, it is Linux support code. I'm also unclear why
this needs to be lifted out of the PerfMemory subsystem.
And if you're going to mess with the jdk.internal module then I think
core-libs folk will want to know before hand.
Other comments ...
src/hotspot/os/linux/perfMemory_linux.cpp
I'm concerned about the MAXPATHLEN stack buffers and the extra overhead
being added to the normal non-container path. We've had issues in the
past where a newly added stack buffer caused StackOverflowError. We
don't need a 4K buffer because os::get_temp_directory() just returns
"/tmp" and we're dealing with all Linux code here. I don't like making
assumptions like this but ...
#define TMP_BUFFER_LEN (4 + 21)
static char* get_user_tmp_dir(const char* user, int vmid, int nspid) {
char buffer[TMP_BUFFER_LEN];
char* tmpdir = os::get_temp_directory();
assert(strlen(tmpdir) == 4, "No longer using /tmp - update buffer size");
if (nspid != -1) {
jio_snprintf(buffer, TMP_BUFFER_LEN, "/proc/%d/root%s", vmid, tmpdir);
tmpdir = buffer;
}
...
657 char fname[64];
Maximum length of "/proc/%d/status" is 23.
Thanks,
David
