Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Explore changing logic to capture thread dump in PA #161

Open
sgup432 opened this issue Apr 8, 2022 · 7 comments
Open

[FEATURE] Explore changing logic to capture thread dump in PA #161

sgup432 opened this issue Apr 8, 2022 · 7 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@sgup432
Copy link
Contributor

sgup432 commented Apr 8, 2022

Is your feature request related to a problem?
Right now we use HotSpotVirtualMachine class here to capture thread dump. We type cast VirtualMachine into HotSpotVirtualMachine and then try to access the method at runtime. This worked okay(with warnings) until JDK 17 came in which doesn't allow this behaviour and resulted in error.

For now we will fix this by adding "add-open" access but we should explore if we can change the logic itself to avoid runtime access of jdk internal classes/methods. As in future JDK versions, even "add-opens" practice maybe disallowed.

What solution would you like?
A clear and concise description of what you want to happen.

What alternatives have you considered?
A clear and concise description of any alternative solutions or features you've considered.

Do you have any additional context?
Add any other context or screenshots about the feature request here.

@kkhatua
Copy link
Member

kkhatua commented Jul 11, 2022

To be picked up by @@kaushalmahi12 once onboarded

@kiranprakash154
Copy link
Contributor

kiranprakash154 commented Aug 16, 2022

Did some research around this and here's what I have to add :

The description of the issue is misleading, since it points the problem being the typecasting, but infact the problem is the class HotSpotVirtualMachine being inaccessible

Image

Image (1)

In the above two images you can see the module properties expose only the package com.sun.tools.* and not sun.tools.* which contains the class HotSpotVirtualMachine

we are solving this issue using --add-opens which is used to open a package and make all its types and members accessible - more on this

The correct problem statement

Can we avoid using the inaccessible class HotSpotVirtualMachine and hence avoid --addopens ?

@kkhatua
Copy link
Member

kkhatua commented Aug 16, 2022

@kiranprakash154
Is there any other way we can get thread dumps, etc in JDK17? We are fundamentally hackng our way to get that information, and might need to do so going forward, or lose the ability to get this information in PA.

@Tjofil
Copy link
Contributor

Tjofil commented Jan 17, 2023

Hey @kkhatua, @kiranprakash154
Is there a reason for not using a ThreadMXBean thread dump ( ThreadMXBeanObject.getThreadInfo(id/id[], stackTraceMaxDepth ) ?

Edit: Just realized that it doesn't contain thread native id :/ .

@Tjofil
Copy link
Contributor

Tjofil commented Jan 19, 2023

I explored a bit and found out the following:

Apparently this package and interface were hidden on purpose because they're prone to change and not recommended to be used by external packages.

Most of the command tools providing JVM profiling and dumping rely on it as well, i.e. jcmd, jstat : Example jcmd implementation.
It can be seen in @kiranprakash154 's comment above that this package is explicitly exported to jcmd's package which makes sense as jcmd is also internally packaged tool so it has no problems using it.

On internet, there are not much use cases and examples of mapping JVM-assigned thread id to native thread id's and not even of utilizing native thread id, especially programmatically (usually it is done through command line tools like jcmd).
As much as I could deduce, native thread id is the only information we are currently getting through HotSpotVirtualMachine dumping that we cannot through ThreadMXBean, correct me if I'm wrong.

So far, I haven't been able to find a "non-hacky" way to reformat the existing logic and keep the native id.

One of possible hacks could be Runtime execution of a separate jcmd process and consuming it's output stream (that would by the way result in the same output we're currently getting through HotSpotVirtualMachine). From one side this may not be a bad idea as jcmd is shipped with jdk and will follow the internal implementations and can provide us with relatively stable interface to these functionalities. From the other this is not clean and there are probably consequences of doing things this way inside OpenSearch environment and similar that I'm not aware of so please give your thoughts about it (probably also performance ?).

Second one is even more convoluted and would imply using JNI or JNA to dynamically load code that does a proxy system call and gets native id of a thread, something similar to this, and is of course extremely system specific so proxy calls would vary for every operating system. This dl would have to be dragged around and it would also be hell for deployment, making this option bad imo.

Third option is keeping the current --addopens hack and the fourth one would be dropping this functionality and transitioning to MXBean completely which we're already using for other dumping info.

@kiranprakash154
Copy link
Contributor

kiranprakash154 commented Jan 20, 2023

Hi @Tjofil, Thanks for your detailed findings.
I had worked on this issue last year and documented a few things. Here's some of it that aligns with your recommendations.
(cc'ing @khushbr for visibility)

  1. I found this bug file in openjdk to expose getnativeid in ThreadInfo - this unfortunately has not been resolved yet and has concerns over cross platform notion of “native thread id”.

  2. Use Processbuilder and load an Agent into the VM

ProcessBuilder lets you run commands and produce output into a file.
PA-RCA’s Threadlist.java instead of parsing the input stream (

private static void parseLine(String line) {
String[] tokens = line.split(" os_prio=[0-9]* ");
ThreadState t = new ThreadState();
t.javaTid = -1;
Matcher m = linePattern.matcher(tokens[0]);
if (!m.find()) {
t.threadName = tokens[0];
} else {
t.threadName = m.group(1);
if (!tokens[0].equals("\"" + t.threadName + "\"")) {
t.javaTid =
Long.parseLong(
tokens[0]
.split(Pattern.quote("\"" + t.threadName + "\" "))[1]
.split(" ")[0]
.split("#")[1]);
}
}
tokens = tokens[1].split(" ");
for (String token : tokens) {
String[] keyValuePare = token.split("=");
if (keyValuePare.length < 2) {
continue;
}
if (t.javaTid == -1 && keyValuePare[0].equals("tid")) {
t.javaTid = Long.decode(keyValuePare[1]);
}
if (keyValuePare[0].equals("nid")) {
t.nativeTid = Long.decode(keyValuePare[1]);
}
}
t.tState = tokens[2]; // TODO: stuff like "in Object.wait()"
nativeTidMap.put(t.nativeTid, t);
jTidMap.put(t.javaTid, t);
nameMap.put(t.threadName, t); // XXX: we assume no collisions
}
) we will have to change it to parse this output from a file.
After attaching to the vm, we have to load a jar which does all of these things.

  • The current thread could sleep for a while until the jar has finished has created a log file.
  • We also need to have appropriate backoff and retry mechanisms setup.
  • We will also have to think about rolling log files to reclaim space.

It would look something like this.

Screen Shot 2022-08-25 at 5 14 20 PM

@Tjofil
Copy link
Contributor

Tjofil commented Jan 26, 2023

Hey @kiranprakash154,
ProcessBuilder sounds like a good option.
I originally had something like this in mind (simplified):

       ...

ProcessBuilder pb = new ProcessBuilder("jcmd", pid, "Thread.print");
p = pb.start();
try (InputStream in = p.getInputStream()) {
            createMap(in);

       ...

which would replace current approach:

try (InputStream in = ((HotSpotVirtualMachine) vm).remoteDataDump(args); ) {
createMap(in);

Process' output is by default piped to an InputStream object from which we can instantly read and keep the rest of parsing process the same. Did you suggest the file writing so we can delay the reading or for some other reason, also what kind of interface would this jar you mentioned use to access the JVM info once it's attached, do we rely on some specific implementation of JVM ?
I'm planning to do this soon and the answer above looks like a good plan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants