Skip to content

Add other time accounting in HotThreads #79392

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.

Sign up for GitHub

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

Merged
merged 17 commits into from
Oct 19, 2021
Merged

Add other time accounting in HotThreads #79392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5d8c9ca
Add other time accounting in HotThreads
Oct 14, 2021
a434a16
Improve sampling of wait/blocked.
Oct 15, 2021
75d7e85
Small refactor.
Oct 15, 2021
d6e5947
Refactor.
Oct 15, 2021
d9d746c
Add sort everywhere.
Oct 15, 2021
16520a2
Make monitoring part of bootstrap
Oct 18, 2021
22c3be8
Update tests and remove security setting.
Oct 18, 2021
423ad61
Make code clearer.
Oct 18, 2021
ec7848e
Fix style and tests.
Oct 18, 2021
5dd7579
Implement comments from Ryan.
Oct 18, 2021
1417916
Undo edit.
Oct 18, 2021
4bc05a0
Merge branch 'master' into feature/hot_threads_other
Oct 18, 2021
3dcec18
Merge branch 'master' into feature/hot_threads_other
elasticmachine Oct 19, 2021
9eb0804
Fix test
Oct 19, 2021
f06b946
Change version from static to straight constant.
Oct 19, 2021
25ab3d7
Merge branch 'feature/hot_threads_other' of github.com:grcevski/elast…
Oct 19, 2021
c1c57d6
Apply changes as per review.
Oct 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions ...n/java/org/elasticsearch/action/admin/cluster/node/hotthreads/NodesHotThreadsRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

package org.elasticsearch.action.admin.cluster.node.hotthreads;

import org.elasticsearch.Version;
import org.elasticsearch.action.support.nodes.BaseNodesRequest;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.io.stream.StreamInput;
Expand All @@ -22,6 +23,7 @@ public class NodesHotThreadsRequest extends BaseNodesRequest<NodesHotThreadsRequ

int threads = 3;
HotThreads.ReportType type = HotThreads.ReportType.CPU;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should all be final, I'm surprised they are not. We should have a ctor to take all the members. That can be a followup, though (doesn't have to make 7.16 since this is already the state of the class).

HotThreads.SortOrder sortOrder = HotThreads.SortOrder.TOTAL;
TimeValue interval = new TimeValue(500, TimeUnit.MILLISECONDS);
int snapshots = 10;
boolean ignoreIdleThreads = true;
Expand All @@ -34,6 +36,9 @@ public NodesHotThreadsRequest(StreamInput in) throws IOException {
type = HotThreads.ReportType.of(in.readString());
interval = in.readTimeValue();
snapshots = in.readInt();
if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
sortOrder = HotThreads.SortOrder.of(in.readString());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice (as a followup) to look at changing read/write enum to use the value, instead of ordinal. As it is, the enum methods for stream input/output are brittle since they rely on the enum definition not changing.

}
}

/**
Expand Down Expand Up @@ -78,6 +83,15 @@ public HotThreads.ReportType type() {
return this.type;
}

public NodesHotThreadsRequest sortOrder(HotThreads.SortOrder sortOrder) {
this.sortOrder = sortOrder;
return this;
}

public HotThreads.SortOrder sortOrder() {
return this.sortOrder;
}

public NodesHotThreadsRequest interval(TimeValue interval) {
this.interval = interval;
return this;
Expand All @@ -104,5 +118,8 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeString(type.getTypeValue());
out.writeTimeValue(interval);
out.writeInt(snapshots);
if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
out.writeString(sortOrder.getOrderValue());
}
}
}
5 changes: 5 additions & 0 deletions ...org/elasticsearch/action/admin/cluster/node/hotthreads/NodesHotThreadsRequestBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ public NodesHotThreadsRequestBuilder setType(HotThreads.ReportType type) {
return this;
}

public NodesHotThreadsRequestBuilder setSortOrder(HotThreads.SortOrder sortOrder) {
request.sortOrder(sortOrder);
return this;
}

public NodesHotThreadsRequestBuilder setInterval(TimeValue interval) {
request.interval(interval);
return this;
Expand Down
1 change: 1 addition & 0 deletions ...rg/elasticsearch/action/admin/cluster/node/hotthreads/TransportNodesHotThreadsAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ protected NodeHotThreads nodeOperation(NodeRequest request, Task task) {
HotThreads hotThreads = new HotThreads()
.busiestThreads(request.request.threads)
.type(request.request.type)
.sortOrder(request.request.sortOrder)
.interval(request.request.interval)
.threadElementsSnapshotCount(request.request.snapshots)
.ignoreIdleThreads(request.request.ignoreIdleThreads);
Expand Down
2 changes: 2 additions & 0 deletions server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.env.Environment;
import org.elasticsearch.jdk.JarHell;
import org.elasticsearch.monitor.jvm.HotThreads;
import org.elasticsearch.monitor.jvm.JvmInfo;
import org.elasticsearch.monitor.os.OsProbe;
import org.elasticsearch.monitor.process.ProcessProbe;
Expand Down Expand Up @@ -156,6 +157,7 @@ static void initializeProbes() {
ProcessProbe.getInstance();
OsProbe.getInstance();
JvmInfo.jvmInfo();
HotThreads.initializeRuntimeMonitoring();
}

private void setup(boolean addShutdownHook, Environment environment) throws BootstrapException {
Expand Down
290 changes: 202 additions & 88 deletions server/src/main/java/org/elasticsearch/monitor/jvm/HotThreads.java
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 6 additions & 2 deletions .../src/main/java/org/elasticsearch/rest/action/admin/cluster/RestNodesHotThreadsAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,13 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
String[] nodesIds = Strings.splitStringByCommaToArray(request.param("nodeId"));
NodesHotThreadsRequest nodesHotThreadsRequest = new NodesHotThreadsRequest(nodesIds);
nodesHotThreadsRequest.threads(request.paramAsInt("threads", nodesHotThreadsRequest.threads()));
nodesHotThreadsRequest.ignoreIdleThreads(request.paramAsBoolean("ignore_idle_threads", nodesHotThreadsRequest.ignoreIdleThreads()));
nodesHotThreadsRequest.ignoreIdleThreads(
request.paramAsBoolean("ignore_idle_threads", nodesHotThreadsRequest.ignoreIdleThreads()));
nodesHotThreadsRequest.type(HotThreads.ReportType.of(request.param("type", nodesHotThreadsRequest.type().getTypeValue())));
nodesHotThreadsRequest.interval(TimeValue.parseTimeValue(request.param("interval"), nodesHotThreadsRequest.interval(), "interval"));
nodesHotThreadsRequest.sortOrder(
HotThreads.SortOrder.of(request.param("sort", nodesHotThreadsRequest.sortOrder().getOrderValue())));
nodesHotThreadsRequest.interval(
TimeValue.parseTimeValue(request.param("interval"), nodesHotThreadsRequest.interval(), "interval"));
nodesHotThreadsRequest.snapshots(request.paramAsInt("snapshots", nodesHotThreadsRequest.snapshots()));
nodesHotThreadsRequest.timeout(request.param("timeout"));
return channel -> client.admin().cluster().nodesHotThreads(
Expand Down
2 changes: 0 additions & 2 deletions server/src/main/resources/org/elasticsearch/bootstrap/security.policy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ grant codeBase "${codebase.elasticsearch-secure-sm}" {
grant codeBase "${codebase.elasticsearch}" {
// needed for loading plugins which may expect the context class loader to be set
permission java.lang.RuntimePermission "setContextClassLoader";
// needed by HotThreads to enable wait/block time accounting on demand
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer needed since we now enable the monitoring before we install the security manager.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great to see permissions removed. :)

permission java.lang.management.ManagementPermission "control";
};

//// Very special jar permissions:
Expand Down
2 changes: 1 addition & 1 deletion ...t/java/org/elasticsearch/action/admin/cluster/hotthreads/NodesHotThreadsRequestTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void testBWCSerialization() throws IOException {
out.setVersion(previous);
request.writeTo(out);
try (StreamInput in = out.bytes().streamInput()) {
in.setVersion(latest);
in.setVersion(previous);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had this wrong before, out.bytes().streamInput() resets the original version that the out had before, so I had to forcibly set it to the old version to simulate 7.x sending over the request.

NodesHotThreadsRequest deserialized = new NodesHotThreadsRequest(in);
assertEquals(request.threads(), deserialized.threads());
assertEquals(request.ignoreIdleThreads(), deserialized.ignoreIdleThreads());
Expand Down
Loading