[ML] Don't get stats for jobs that are being deleted (elastic/x-pack-elasticsearch#899)
If jobs are being deleted then the operations required to get stats could fail with unexpected exceptions. When stats for multiple jobs were being requested, this would previously cause the whole operation to fail. This commit changes the stats endpoint to ignore jobs that are being deleted. Fixes elastic/prelert-legacy#837 Original commit: elastic/x-pack-elasticsearch@6ac141a987
This commit is contained in:
parent
c5a2fba70a
commit
2d01c3884b
|
@ -73,6 +73,11 @@ public class MlMetadata implements MetaData.Custom {
|
|||
return jobs;
|
||||
}
|
||||
|
||||
public boolean isJobDeleted(String jobId) {
|
||||
Job job = jobs.get(jobId);
|
||||
return job == null || job.isDeleted();
|
||||
}
|
||||
|
||||
public SortedMap<String, DatafeedConfig> getDatafeeds() {
|
||||
return datafeeds;
|
||||
}
|
||||
|
|
|
@ -379,7 +379,8 @@ public class GetJobsStatsAction extends Action<GetJobsStatsAction.Request, GetJo
|
|||
}
|
||||
|
||||
ActionListener<Response> finalListener = listener;
|
||||
listener = ActionListener.wrap(response -> gatherStatsForClosedJobs(request, response, finalListener), listener::onFailure);
|
||||
listener = ActionListener.wrap(response -> gatherStatsForClosedJobs(mlMetadata,
|
||||
request, response, finalListener), listener::onFailure);
|
||||
super.doExecute(task, request, listener);
|
||||
}
|
||||
|
||||
|
@ -428,8 +429,10 @@ public class GetJobsStatsAction extends Action<GetJobsStatsAction.Request, GetJo
|
|||
|
||||
// Up until now we gathered the stats for jobs that were open,
|
||||
// This method will fetch the stats for missing jobs, that was stored in the jobs index
|
||||
void gatherStatsForClosedJobs(Request request, Response response, ActionListener<Response> listener) {
|
||||
List<String> jobIds = determineJobIdsWithoutLiveStats(request.expandedJobsIds, response.jobsStats.results());
|
||||
void gatherStatsForClosedJobs(MlMetadata mlMetadata, Request request, Response response,
|
||||
ActionListener<Response> listener) {
|
||||
List<String> jobIds = determineNonDeletedJobIdsWithoutLiveStats(mlMetadata,
|
||||
request.expandedJobsIds, response.jobsStats.results());
|
||||
if (jobIds.isEmpty()) {
|
||||
listener.onResponse(response);
|
||||
return;
|
||||
|
@ -477,9 +480,12 @@ public class GetJobsStatsAction extends Action<GetJobsStatsAction.Request, GetJo
|
|||
}
|
||||
}
|
||||
|
||||
static List<String> determineJobIdsWithoutLiveStats(List<String> requestedJobIds, List<Response.JobStats> stats) {
|
||||
static List<String> determineNonDeletedJobIdsWithoutLiveStats(MlMetadata mlMetadata,
|
||||
List<String> requestedJobIds,
|
||||
List<Response.JobStats> stats) {
|
||||
Set<String> excludeJobIds = stats.stream().map(Response.JobStats::getJobId).collect(Collectors.toSet());
|
||||
return requestedJobIds.stream().filter(jobId -> !excludeJobIds.contains(jobId)).collect(Collectors.toList());
|
||||
return requestedJobIds.stream().filter(jobId -> !excludeJobIds.contains(jobId) &&
|
||||
!mlMetadata.isJobDeleted(jobId)).collect(Collectors.toList());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -7,6 +7,7 @@ package org.elasticsearch.xpack.ml.action;
|
|||
|
||||
import org.elasticsearch.common.unit.TimeValue;
|
||||
import org.elasticsearch.test.ESTestCase;
|
||||
import org.elasticsearch.xpack.ml.MlMetadata;
|
||||
import org.elasticsearch.xpack.ml.job.config.JobState;
|
||||
import org.elasticsearch.xpack.ml.job.process.autodetect.state.DataCounts;
|
||||
|
||||
|
@ -16,27 +17,37 @@ import java.util.Collections;
|
|||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
|
||||
import static org.elasticsearch.xpack.ml.action.GetJobsStatsAction.TransportAction.determineJobIdsWithoutLiveStats;
|
||||
import static org.elasticsearch.xpack.ml.action.GetJobsStatsAction.TransportAction.determineNonDeletedJobIdsWithoutLiveStats;
|
||||
|
||||
import static org.mockito.Matchers.eq;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
public class GetJobsStatsActionTests extends ESTestCase {
|
||||
|
||||
public void testDetermineJobIds() {
|
||||
List<String> result = determineJobIdsWithoutLiveStats(Collections.singletonList("id1"), Collections.emptyList());
|
||||
|
||||
MlMetadata mlMetadata = mock(MlMetadata.class);
|
||||
when(mlMetadata.isJobDeleted(eq("id4"))).thenReturn(true);
|
||||
|
||||
List<String> result = determineNonDeletedJobIdsWithoutLiveStats(mlMetadata,
|
||||
Collections.singletonList("id1"), Collections.emptyList());
|
||||
assertEquals(1, result.size());
|
||||
assertEquals("id1", result.get(0));
|
||||
|
||||
result = determineJobIdsWithoutLiveStats(Collections.singletonList("id1"), Collections.singletonList(
|
||||
new GetJobsStatsAction.Response.JobStats("id1", new DataCounts("id1"), null, JobState.CLOSED, null, null, null)));
|
||||
result = determineNonDeletedJobIdsWithoutLiveStats(mlMetadata,
|
||||
Collections.singletonList("id1"), Collections.singletonList(
|
||||
new GetJobsStatsAction.Response.JobStats("id1", new DataCounts("id1"), null, JobState.OPENED, null, null, null)));
|
||||
assertEquals(0, result.size());
|
||||
|
||||
result = determineJobIdsWithoutLiveStats(
|
||||
result = determineNonDeletedJobIdsWithoutLiveStats(mlMetadata,
|
||||
Arrays.asList("id1", "id2", "id3"), Collections.emptyList());
|
||||
assertEquals(3, result.size());
|
||||
assertEquals("id1", result.get(0));
|
||||
assertEquals("id2", result.get(1));
|
||||
assertEquals("id3", result.get(2));
|
||||
|
||||
result = determineJobIdsWithoutLiveStats(
|
||||
result = determineNonDeletedJobIdsWithoutLiveStats(mlMetadata,
|
||||
Arrays.asList("id1", "id2", "id3"),
|
||||
Collections.singletonList(new GetJobsStatsAction.Response.JobStats("id1", new DataCounts("id1"), null,
|
||||
JobState.CLOSED, null, null, null))
|
||||
|
@ -45,19 +56,28 @@ public class GetJobsStatsActionTests extends ESTestCase {
|
|||
assertEquals("id2", result.get(0));
|
||||
assertEquals("id3", result.get(1));
|
||||
|
||||
result = determineJobIdsWithoutLiveStats(Arrays.asList("id1", "id2", "id3"), Arrays.asList(
|
||||
new GetJobsStatsAction.Response.JobStats("id1", new DataCounts("id1"), null, JobState.CLOSED, null, null, null),
|
||||
new GetJobsStatsAction.Response.JobStats("id3", new DataCounts("id3"), null, JobState.CLOSED, null, null, null)
|
||||
result = determineNonDeletedJobIdsWithoutLiveStats(mlMetadata,
|
||||
Arrays.asList("id1", "id2", "id3"), Arrays.asList(
|
||||
new GetJobsStatsAction.Response.JobStats("id1", new DataCounts("id1"), null, JobState.OPENED, null, null, null),
|
||||
new GetJobsStatsAction.Response.JobStats("id3", new DataCounts("id3"), null, JobState.OPENED, null, null, null)
|
||||
));
|
||||
assertEquals(1, result.size());
|
||||
assertEquals("id2", result.get(0));
|
||||
|
||||
result = determineJobIdsWithoutLiveStats(Arrays.asList("id1", "id2", "id3"),
|
||||
result = determineNonDeletedJobIdsWithoutLiveStats(mlMetadata, Arrays.asList("id1", "id2", "id3"),
|
||||
Arrays.asList(
|
||||
new GetJobsStatsAction.Response.JobStats("id1", new DataCounts("id1"), null, JobState.CLOSED, null, null, null),
|
||||
new GetJobsStatsAction.Response.JobStats("id2", new DataCounts("id2"), null, JobState.CLOSED, null, null, null),
|
||||
new GetJobsStatsAction.Response.JobStats("id3", new DataCounts("id3"), null, JobState.CLOSED, null, null, null)));
|
||||
new GetJobsStatsAction.Response.JobStats("id1", new DataCounts("id1"), null, JobState.OPENED, null, null, null),
|
||||
new GetJobsStatsAction.Response.JobStats("id2", new DataCounts("id2"), null, JobState.OPENED, null, null, null),
|
||||
new GetJobsStatsAction.Response.JobStats("id3", new DataCounts("id3"), null, JobState.OPENED, null, null, null)));
|
||||
assertEquals(0, result.size());
|
||||
|
||||
// No jobs running, but job 4 is being deleted
|
||||
result = determineNonDeletedJobIdsWithoutLiveStats(mlMetadata,
|
||||
Arrays.asList("id1", "id2", "id3", "id4"), Collections.emptyList());
|
||||
assertEquals(3, result.size());
|
||||
assertEquals("id1", result.get(0));
|
||||
assertEquals("id2", result.get(1));
|
||||
assertEquals("id3", result.get(2));
|
||||
}
|
||||
|
||||
public void testDurationToTimeValue() {
|
||||
|
|
Loading…
Reference in New Issue