Skip to content

Commit ca49faa

Browse files
[ML] Get job stats request should filter non-ML job tasks (#33516)
When requesting job stats for `_all`, all ES tasks are accepted resulting to loads of cluster traffic and a memory overhead. This commit correctly filters out non ML job tasks. Closes #33515
1 parent aca1679 commit ca49faa

File tree

4 files changed

+39
-4
lines changed

4 files changed

+39
-4
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetJobsStatsAction.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import org.elasticsearch.action.support.tasks.BaseTasksRequest;
1515
import org.elasticsearch.action.support.tasks.BaseTasksResponse;
1616
import org.elasticsearch.client.ElasticsearchClient;
17-
import org.elasticsearch.cluster.metadata.MetaData;
1817
import org.elasticsearch.cluster.node.DiscoveryNode;
1918
import org.elasticsearch.common.Nullable;
2019
import org.elasticsearch.common.ParseField;
@@ -100,7 +99,7 @@ public boolean allowNoJobs() {
10099

101100
@Override
102101
public boolean match(Task task) {
103-
return jobId.equals(MetaData.ALL) || OpenJobAction.JobTaskMatcher.match(task, jobId);
102+
return OpenJobAction.JobTaskMatcher.match(task, jobId);
104103
}
105104

106105
@Override

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/OpenJobAction.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.action.support.master.AcknowledgedResponse;
1313
import org.elasticsearch.action.support.master.MasterNodeRequest;
1414
import org.elasticsearch.client.ElasticsearchClient;
15+
import org.elasticsearch.cluster.metadata.MetaData;
1516
import org.elasticsearch.common.ParseField;
1617
import org.elasticsearch.common.Strings;
1718
import org.elasticsearch.common.io.stream.StreamInput;
@@ -258,8 +259,14 @@ static class RequestBuilder extends ActionRequestBuilder<Request, AcknowledgedRe
258259
public interface JobTaskMatcher {
259260

260261
static boolean match(Task task, String expectedJobId) {
261-
String expectedDescription = "job-" + expectedJobId;
262-
return task instanceof JobTaskMatcher && expectedDescription.equals(task.getDescription());
262+
if (task instanceof JobTaskMatcher) {
263+
if (MetaData.ALL.equals(expectedJobId)) {
264+
return true;
265+
}
266+
String expectedDescription = "job-" + expectedJobId;
267+
return expectedDescription.equals(task.getDescription());
268+
}
269+
return false;
263270
}
264271
}
265272

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetJobStatsActionRequestTests.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@
66
package org.elasticsearch.xpack.core.ml.action;
77

88
import org.elasticsearch.cluster.metadata.MetaData;
9+
import org.elasticsearch.tasks.Task;
910
import org.elasticsearch.test.AbstractStreamableTestCase;
1011
import org.elasticsearch.xpack.core.ml.action.GetJobsStatsAction.Request;
1112

13+
import static org.hamcrest.Matchers.is;
14+
import static org.mockito.Mockito.mock;
15+
1216
public class GetJobStatsActionRequestTests extends AbstractStreamableTestCase<Request> {
1317

1418
@Override
@@ -23,4 +27,9 @@ protected Request createBlankInstance() {
2327
return new Request();
2428
}
2529

30+
public void testMatch_GivenAll_FailsForNonJobTasks() {
31+
Task nonJobTask = mock(Task.class);
32+
33+
assertThat(new Request("_all").match(nonJobTask), is(false));
34+
}
2635
}

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportOpenJobActionTests.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.elasticsearch.persistent.PersistentTasksCustomMetaData;
3131
import org.elasticsearch.persistent.PersistentTasksCustomMetaData.Assignment;
3232
import org.elasticsearch.rest.RestStatus;
33+
import org.elasticsearch.tasks.Task;
3334
import org.elasticsearch.test.ESTestCase;
3435
import org.elasticsearch.test.VersionUtils;
3536
import org.elasticsearch.xpack.core.ml.MlMetaIndex;
@@ -66,6 +67,7 @@
6667

6768
import static org.elasticsearch.xpack.core.ml.job.config.JobTests.buildJobBuilder;
6869
import static org.hamcrest.Matchers.containsString;
70+
import static org.hamcrest.Matchers.is;
6971
import static org.mockito.Mockito.mock;
7072
import static org.mockito.Mockito.when;
7173

@@ -642,6 +644,24 @@ public void testNodeNameAndMlAttributes() {
642644
assertEquals("{_node_name1}{ml.machine_memory=5}{node.ml=true}", TransportOpenJobAction.nodeNameAndMlAttributes(node));
643645
}
644646

647+
public void testJobTaskMatcherMatch() {
648+
Task nonJobTask1 = mock(Task.class);
649+
Task nonJobTask2 = mock(Task.class);
650+
TransportOpenJobAction.JobTask jobTask1 = new TransportOpenJobAction.JobTask("ml-1",
651+
0, "persistent", "", null, null);
652+
TransportOpenJobAction.JobTask jobTask2 = new TransportOpenJobAction.JobTask("ml-2",
653+
1, "persistent", "", null, null);
654+
655+
assertThat(OpenJobAction.JobTaskMatcher.match(nonJobTask1, "_all"), is(false));
656+
assertThat(OpenJobAction.JobTaskMatcher.match(nonJobTask2, "_all"), is(false));
657+
assertThat(OpenJobAction.JobTaskMatcher.match(jobTask1, "_all"), is(true));
658+
assertThat(OpenJobAction.JobTaskMatcher.match(jobTask2, "_all"), is(true));
659+
assertThat(OpenJobAction.JobTaskMatcher.match(jobTask1, "ml-1"), is(true));
660+
assertThat(OpenJobAction.JobTaskMatcher.match(jobTask2, "ml-1"), is(false));
661+
assertThat(OpenJobAction.JobTaskMatcher.match(jobTask1, "ml-2"), is(false));
662+
assertThat(OpenJobAction.JobTaskMatcher.match(jobTask2, "ml-2"), is(true));
663+
}
664+
645665
public static void addJobTask(String jobId, String nodeId, JobState jobState, PersistentTasksCustomMetaData.Builder builder) {
646666
builder.addTask(MlTasks.jobTaskId(jobId), OpenJobAction.TASK_NAME, new OpenJobAction.JobParams(jobId),
647667
new Assignment(nodeId, "test assignment"));

0 commit comments

Comments
 (0)