From 8e73085047ff9a943d0b2197889130b015ea14cb Mon Sep 17 00:00:00 2001 From: David Kyle Date: Thu, 8 Feb 2018 11:44:16 +0000 Subject: [PATCH] [ML] Enable adding multiple jobs to a calendar (elastic/x-pack-elasticsearch#3786) Original commit: elastic/x-pack-elasticsearch@56a70a4580ed6ebc87793b0adab8b9171035ac62 --- .../rest-api/ml/delete-calendar-job.asciidoc | 3 +- docs/en/rest-api/ml/put-calendar-job.asciidoc | 4 +- .../ml/action/UpdateCalendarJobAction.java | 37 ++++---- .../UpdateCalendarJobActionResquestTests.java | 1 - .../TransportUpdateCalendarJobAction.java | 21 +++-- .../xpack/ml/job/persistence/JobProvider.java | 4 +- .../xpack/ml/integration/JobProviderIT.java | 22 ++--- .../rest-api-spec/test/ml/calendar_crud.yml | 85 ++++++++++++++++++- 8 files changed, 125 insertions(+), 52 deletions(-) diff --git a/docs/en/rest-api/ml/delete-calendar-job.asciidoc b/docs/en/rest-api/ml/delete-calendar-job.asciidoc index 704d589f69c..54fe9ebdaba 100644 --- a/docs/en/rest-api/ml/delete-calendar-job.asciidoc +++ b/docs/en/rest-api/ml/delete-calendar-job.asciidoc @@ -19,7 +19,8 @@ This API enables you to delete jobs from a calendar. (string) Identifier for the calendar. `job_id` (required):: - (string) Identifier for the job. + (string) An identifier for the job. It can be a job identifier, a group name, or a + comma-separated list of jobs or groups. ==== Authorization diff --git a/docs/en/rest-api/ml/put-calendar-job.asciidoc b/docs/en/rest-api/ml/put-calendar-job.asciidoc index 0bfd20d6ccb..5d2c012a919 100644 --- a/docs/en/rest-api/ml/put-calendar-job.asciidoc +++ b/docs/en/rest-api/ml/put-calendar-job.asciidoc @@ -18,8 +18,8 @@ This API enables you to add a job to a calendar. (string) Identifier for the calendar. `job_id` (required):: - (string) Identifier for the job. - + (string) An identifier for the job. It can be a job identifier, a group name, or a + comma-separated list of jobs or groups. ==== Authorization diff --git a/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/UpdateCalendarJobAction.java b/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/UpdateCalendarJobAction.java index 9420343f735..d1a453c7e16 100644 --- a/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/UpdateCalendarJobAction.java +++ b/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/UpdateCalendarJobAction.java @@ -16,7 +16,6 @@ import org.elasticsearch.xpack.core.ml.calendars.Calendar; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import java.io.IOException; -import java.util.ArrayList; import java.util.Objects; public class UpdateCalendarJobAction extends Action { @Override diff --git a/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateCalendarJobAction.java b/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateCalendarJobAction.java index beee767f3a1..e159f2b55d7 100644 --- a/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateCalendarJobAction.java +++ b/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateCalendarJobAction.java @@ -8,12 +8,16 @@ package org.elasticsearch.xpack.ml.action; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.ml.MLMetadataField; +import org.elasticsearch.xpack.core.ml.MlMetadata; import org.elasticsearch.xpack.core.ml.action.PutCalendarAction; import org.elasticsearch.xpack.core.ml.action.UpdateCalendarJobAction; import org.elasticsearch.xpack.ml.job.JobManager; @@ -42,17 +46,16 @@ public class TransportUpdateCalendarJobAction extends HandledTransportAction listener) { - - Set jobIdsToAdd = new HashSet<>(); - if (request.getJobIdToAdd() != null && request.getJobIdToAdd().isEmpty() == false) { - jobIdsToAdd.add(request.getJobIdToAdd()); - } - Set jobIdsToRemove = new HashSet<>(); - if (request.getJobIdToRemove() != null && request.getJobIdToRemove().isEmpty() == false) { - jobIdsToRemove.add(request.getJobIdToRemove()); + ClusterState clusterState = clusterService.state(); + MlMetadata mlMetadata = clusterState.getMetaData().custom(MLMetadataField.TYPE); + if (mlMetadata == null) { + mlMetadata = MlMetadata.EMPTY_METADATA; } - jobProvider.updateCalendar(request.getCalendarId(), jobIdsToAdd, jobIdsToRemove, clusterService.state(), + Set jobIdsToAdd = Strings.tokenizeByCommaToSet(request.getJobIdsToAddExpression()); + Set jobIdsToRemove = Strings.tokenizeByCommaToSet(request.getJobIdsToRemoveExpression()); + + jobProvider.updateCalendar(request.getCalendarId(), jobIdsToAdd, jobIdsToRemove, mlMetadata, c -> { jobManager.updateProcessOnCalendarChanged(c.getJobIds()); listener.onResponse(new PutCalendarAction.Response(c)); diff --git a/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobProvider.java b/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobProvider.java index 1185ecb582c..13eb54d7fc8 100644 --- a/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobProvider.java +++ b/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobProvider.java @@ -67,7 +67,6 @@ import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.sort.FieldSortBuilder; import org.elasticsearch.search.sort.SortBuilders; import org.elasticsearch.search.sort.SortOrder; -import org.elasticsearch.xpack.core.ml.MLMetadataField; import org.elasticsearch.xpack.core.ml.MlMetaIndex; import org.elasticsearch.xpack.core.ml.MlMetadata; import org.elasticsearch.xpack.core.ml.action.GetBucketsAction; @@ -1082,7 +1081,7 @@ public class JobProvider { result -> handler.accept(result.result), errorHandler, () -> null); } - public void updateCalendar(String calendarId, Set jobIdsToAdd, Set jobIdsToRemove, ClusterState clusterState, + public void updateCalendar(String calendarId, Set jobIdsToAdd, Set jobIdsToRemove, MlMetadata mlMetadata, Consumer handler, Consumer errorHandler) { ActionListener getCalendarListener = ActionListener.wrap( @@ -1090,7 +1089,6 @@ public class JobProvider { Set currentJobs = new HashSet<>(calendar.getJobIds()); for (String jobToAdd : jobIdsToAdd) { - MlMetadata mlMetadata = clusterState.getMetaData().custom(MLMetadataField.TYPE); if (mlMetadata.isGroupOrJob(jobToAdd) == false) { errorHandler.accept(ExceptionsHelper.missingJobException(jobToAdd)); return; diff --git a/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobProviderIT.java b/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobProviderIT.java index 751b490b358..62bf1ed6ee9 100644 --- a/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobProviderIT.java +++ b/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobProviderIT.java @@ -10,9 +10,7 @@ import org.elasticsearch.action.bulk.BulkRequestBuilder; import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.support.WriteRequest; -import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.routing.UnassignedInfo; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; @@ -22,7 +20,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.xpack.core.XPackSettings; -import org.elasticsearch.xpack.core.ml.MLMetadataField; import org.elasticsearch.xpack.core.ml.MlMetaIndex; import org.elasticsearch.xpack.core.ml.MlMetadata; import org.elasticsearch.xpack.core.ml.action.PutJobAction; @@ -67,7 +64,6 @@ import java.util.List; import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicReference; -import java.util.stream.Collectors; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.hasSize; @@ -129,7 +125,7 @@ public class JobProviderIT extends MlSingleNodeTestCase { assertThat(queryResult, hasSize(3)); Long matchedCount = queryResult.stream().filter( c -> c.getId().equals("foo calendar") || c.getId().equals("foo bar calendar") || c.getId().equals("cat foo calendar")) - .collect(Collectors.counting()); + .count(); assertEquals(new Long(3), matchedCount); queryResult = getCalendars("bar"); @@ -142,10 +138,6 @@ public class JobProviderIT extends MlSingleNodeTestCase { mlBuilder.putJob(createJob("foo").build(), false); mlBuilder.putJob(createJob("bar").build(), false); - ClusterState clusterState = ClusterState.builder(new ClusterName("_name")) - .metaData(new MetaData.Builder().putCustom(MLMetadataField.TYPE, mlBuilder.build())) - .build(); - String calendarId = "empty calendar"; Calendar emptyCal = new Calendar(calendarId, Collections.emptyList(), null); indexCalendars(Collections.singletonList(emptyCal)); @@ -153,15 +145,13 @@ public class JobProviderIT extends MlSingleNodeTestCase { Set addedIds = new HashSet<>(); addedIds.add("foo"); addedIds.add("bar"); - updateCalendar(calendarId, addedIds, Collections.emptySet(), clusterState); + updateCalendar(calendarId, addedIds, Collections.emptySet(), mlBuilder.build()); Calendar updated = getCalendar(calendarId); assertEquals(calendarId, updated.getId()); assertEquals(addedIds, new HashSet<>(updated.getJobIds())); - Set removedIds = new HashSet<>(); - removedIds.add("foo"); - updateCalendar(calendarId, Collections.emptySet(), removedIds, clusterState); + updateCalendar(calendarId, Collections.emptySet(), Collections.singleton("foo"), mlBuilder.build()); updated = getCalendar(calendarId); assertEquals(calendarId, updated.getId()); @@ -250,11 +240,11 @@ public class JobProviderIT extends MlSingleNodeTestCase { return result.get().results(); } - private void updateCalendar(String calendarId, Set idsToAdd, Set idsToRemove, ClusterState clusterState) + private void updateCalendar(String calendarId, Set idsToAdd, Set idsToRemove, MlMetadata mlMetadata) throws Exception { CountDownLatch latch = new CountDownLatch(1); AtomicReference exceptionHolder = new AtomicReference<>(); - jobProvider.updateCalendar(calendarId, idsToAdd, idsToRemove, clusterState, + jobProvider.updateCalendar(calendarId, idsToAdd, idsToRemove, mlMetadata, r -> latch.countDown(), e -> { exceptionHolder.set(e); @@ -357,7 +347,7 @@ public class JobProviderIT extends MlSingleNodeTestCase { events.add(buildScheduledEvent("downtime_A", now.plusDays(1), now.plusDays(2), calendarAId)); String calendarBId = "calendar_b"; - calendars.add(new Calendar(calendarBId, Arrays.asList(groupB), null)); + calendars.add(new Calendar(calendarBId, Collections.singletonList(groupB), null)); events.add(buildScheduledEvent("downtime_B", now.plusDays(12), now.plusDays(13), calendarBId)); indexCalendars(calendars); diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/calendar_crud.yml b/plugin/src/test/resources/rest-api-spec/test/ml/calendar_crud.yml index 7a78aa6cb51..0ec96cec7fb 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/calendar_crud.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/calendar_crud.yml @@ -218,6 +218,19 @@ } - match: { job_id: "tiger" } + - do: + xpack.ml.put_job: + job_id: otter + body: > + { + "analysis_config" : { + "detectors" :[{"function":"metric","field_name":"responsetime","by_field_name":"airline"}] + }, + "data_description" : { + } + } + - match: { job_id: "otter" } + - do: xpack.ml.put_calendar_job: calendar_id: "wildlife" @@ -240,6 +253,12 @@ - match: { calendar_id: "wildlife" } - length: { job_ids: 0 } + - do: + catch: /Cannot remove \[otter\] as it is not present in calendar \[wildlife\]/ + xpack.ml.delete_calendar_job: + calendar_id: "wildlife" + job_id: "otter" + - do: xpack.ml.get_calendars: calendar_id: "wildlife" @@ -251,14 +270,13 @@ catch: missing xpack.ml.put_calendar_job: calendar_id: "wildlife" - job_id: "missing job" + job_id: "missing_job" - do: catch: /Cannot remove \[missing_job\] as it is not present in calendar \[wildlife\]/ xpack.ml.delete_calendar_job: calendar_id: "wildlife" job_id: "missing_job" - --- "Test calendar get events": @@ -632,4 +650,65 @@ catch: /No calendar with id \[unknown\]/ xpack.ml.delete_calendar_job: calendar_id: "unknown" - job_id: "missing_job" + job_id: "missing_calendar" + +--- +"Test list of job Ids": + - do: + xpack.ml.put_job: + job_id: foo-a + body: > + { + "analysis_config" : { + "detectors" :[{"function":"metric","field_name":"responsetime","by_field_name":"airline"}] + }, + "data_description" : { + } + } + + - do: + xpack.ml.put_job: + job_id: foo-b + body: > + { + "analysis_config" : { + "detectors" :[{"function":"metric","field_name":"responsetime","by_field_name":"airline"}] + }, + "data_description" : { + } + } + + - do: + xpack.ml.put_job: + job_id: bar-a + body: > + { + "analysis_config" : { + "detectors" :[{"function":"metric","field_name":"responsetime","by_field_name":"airline"}] + }, + "data_description" : { + } + } + + - do: + xpack.ml.put_calendar: + calendar_id: "expression" + body: > + { + "job_ids": ["bar-a"] + } + + - do: + xpack.ml.put_calendar_job: + calendar_id: "expression" + job_id: "foo-a,foo-b" + - match: { calendar_id: "expression" } + - length: { job_ids: 3 } + + - do: + xpack.ml.delete_calendar_job: + calendar_id: "expression" + job_id: "foo-a,foo-b" + - match: { calendar_id: "expression" } + - length: { job_ids: 1 } + - match: { job_ids.0: "bar-a" }