Skip to content

Commit fa81134

Browse files
davidkyledavidkyle
authored
[ML] Update running process when global calendar changes (#83044) (#83084)
Adding events to global calendars did not update open jobs as the special _all job Id was not checked.
1 parent 1ad7028 commit fa81134

File tree

4 files changed

+100
-47
lines changed

4 files changed

+100
-47
lines changed

‎docs/changelog/83044.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 83044
2+
summary: Update running process when global calendar changes
3+
area: Machine Learning
4+
type: bug
5+
issues: []

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdate.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package org.elasticsearch.xpack.core.ml.job.config;
88

99
import org.elasticsearch.Version;
10+
import org.elasticsearch.common.Strings;
1011
import org.elasticsearch.common.io.stream.StreamInput;
1112
import org.elasticsearch.common.io.stream.StreamOutput;
1213
import org.elasticsearch.common.io.stream.Writeable;
@@ -445,6 +446,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
445446
return builder;
446447
}
447448

449+
@Override
450+
public String toString() {
451+
return Strings.toString(this::toXContent);
452+
}
453+
448454
public Set<String> getUpdateFields() {
449455
Set<String> updateFields = new TreeSet<>();
450456
if (groups != null) {

‎x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/ScheduledEventsIT.java

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.List;
3434
import java.util.stream.Collectors;
3535

36+
import static org.hamcrest.Matchers.contains;
3637
import static org.hamcrest.Matchers.empty;
3738
import static org.hamcrest.Matchers.equalTo;
3839
import static org.hamcrest.Matchers.is;
@@ -372,7 +373,10 @@ public void testAddOpenedJobToGroupWithCalendar() throws Exception {
372373
}
373374

374375
/**
375-
* An open job that later gets added to a calendar, should take the scheduled events into account
376+
* Add a global calendar then create a job that will pick
377+
* up the calendar.
378+
* Add a new scheduled event to the calendar, the open
379+
* job should pick up the new event
376380
*/
377381
public void testNewJobWithGlobalCalendar() throws Exception {
378382
String calendarId = "test-global-calendar";
@@ -381,28 +385,56 @@ public void testNewJobWithGlobalCalendar() throws Exception {
381385
putCalendar(calendarId, Collections.singletonList(Metadata.ALL), "testNewJobWithGlobalCalendar calendar");
382386

383387
long startTime = 1514764800000L;
384-
final int bucketCount = 3;
388+
final int bucketCount = 6;
385389
TimeValue bucketSpan = TimeValue.timeValueMinutes(30);
386390

387391
// Put events in the calendar
388-
List<ScheduledEvent> events = new ArrayList<>();
392+
List<ScheduledEvent> preOpenEvents = new ArrayList<>();
389393
long eventStartTime = startTime;
390394
long eventEndTime = eventStartTime + (long) (1.5 * bucketSpan.millis());
391-
events.add(
392-
new ScheduledEvent.Builder().description("Some Event")
395+
preOpenEvents.add(
396+
new ScheduledEvent.Builder().description("Pre open Event")
393397
.startTime((Instant.ofEpochMilli(eventStartTime)))
394398
.endTime((Instant.ofEpochMilli(eventEndTime)))
395399
.calendarId(calendarId)
396400
.build()
397401
);
398402

399-
postScheduledEvents(calendarId, events);
400-
401-
Job.Builder job = createJob("scheduled-events-add-to-new-job--with-global-calendar", bucketSpan);
403+
postScheduledEvents(calendarId, preOpenEvents);
402404

403405
// Open the job
406+
Job.Builder job = createJob("scheduled-events-add-to-new-job--with-global-calendar", bucketSpan);
404407
openJob(job.getId());
405408

409+
// Add another event after the job is opened
410+
List<ScheduledEvent> postOpenJobEvents = new ArrayList<>();
411+
eventStartTime = eventEndTime + (3 * bucketSpan.millis());
412+
eventEndTime = eventStartTime + bucketSpan.millis();
413+
postOpenJobEvents.add(
414+
new ScheduledEvent.Builder().description("Event added after job is opened")
415+
.startTime((Instant.ofEpochMilli(eventStartTime)))
416+
.endTime((Instant.ofEpochMilli(eventEndTime)))
417+
.calendarId(calendarId)
418+
.build()
419+
);
420+
postScheduledEvents(calendarId, postOpenJobEvents);
421+
422+
// Wait until the notification that the job was updated is indexed
423+
assertBusy(() -> {
424+
SearchResponse searchResponse = client().prepareSearch(NotificationsIndex.NOTIFICATIONS_INDEX)
425+
.setSize(1)
426+
.addSort("timestamp", SortOrder.DESC)
427+
.setQuery(
428+
QueryBuilders.boolQuery()
429+
.filter(QueryBuilders.termQuery("job_id", job.getId()))
430+
.filter(QueryBuilders.termQuery("level", "info"))
431+
)
432+
.get();
433+
SearchHit[] hits = searchResponse.getHits().getHits();
434+
assertThat(hits.length, equalTo(1));
435+
assertThat(hits[0].getSourceAsMap().get("message"), equalTo("Updated calendars in running process"));
436+
});
437+
406438
// write some buckets of data
407439
postData(
408440
job.getId(),
@@ -416,12 +448,14 @@ public void testNewJobWithGlobalCalendar() throws Exception {
416448
GetBucketsAction.Request getBucketsRequest = new GetBucketsAction.Request(job.getId());
417449
List<Bucket> buckets = getBuckets(getBucketsRequest);
418450

419-
// 1st and 2nd buckets have the event but the last one does not
420-
assertEquals(1, buckets.get(0).getScheduledEvents().size());
421-
assertEquals("Some Event", buckets.get(0).getScheduledEvents().get(0));
422-
assertEquals(1, buckets.get(1).getScheduledEvents().size());
423-
assertEquals("Some Event", buckets.get(1).getScheduledEvents().get(0));
451+
// 1st and 2nd buckets have the first event
452+
// 5th and 6th buckets have the second event
453+
assertThat(buckets.get(0).getScheduledEvents(), contains("Pre open Event"));
454+
assertThat(buckets.get(1).getScheduledEvents(), contains("Pre open Event"));
424455
assertEquals(0, buckets.get(2).getScheduledEvents().size());
456+
assertEquals(0, buckets.get(3).getScheduledEvents().size());
457+
assertThat(buckets.get(4).getScheduledEvents(), contains("Event added after job is opened"));
458+
assertThat(buckets.get(5).getScheduledEvents(), contains("Event added after job is opened"));
425459
}
426460

427461
private Job.Builder createJob(String jobId, TimeValue bucketSpan) {

‎x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import org.apache.logging.log4j.LogManager;
1010
import org.apache.logging.log4j.Logger;
11+
import org.apache.logging.log4j.message.ParameterizedMessage;
1112
import org.elasticsearch.ResourceAlreadyExistsException;
1213
import org.elasticsearch.ResourceNotFoundException;
1314
import org.elasticsearch.Version;
@@ -31,9 +32,6 @@
3132
import org.elasticsearch.persistent.PersistentTasksCustomMetadata;
3233
import org.elasticsearch.threadpool.ThreadPool;
3334
import org.elasticsearch.xcontent.NamedXContentRegistry;
34-
import org.elasticsearch.xcontent.ToXContent;
35-
import org.elasticsearch.xcontent.XContentBuilder;
36-
import org.elasticsearch.xcontent.XContentFactory;
3735
import org.elasticsearch.xpack.core.action.util.QueryPage;
3836
import org.elasticsearch.xpack.core.ml.MachineLearningField;
3937
import org.elasticsearch.xpack.core.ml.MlConfigIndex;
@@ -72,6 +70,7 @@
7270

7371
import java.io.IOException;
7472
import java.util.ArrayList;
73+
import java.util.Collection;
7574
import java.util.Collections;
7675
import java.util.Comparator;
7776
import java.util.Date;
@@ -527,28 +526,28 @@ public void deleteJob(
527526

528527
private void postJobUpdate(UpdateJobAction.Request request, Job updatedJob, ActionListener<PutJobAction.Response> actionListener) {
529528
// Autodetect must be updated if the fields that the C++ uses are changed
530-
if (request.getJobUpdate().isAutodetectProcessUpdate()) {
531-
JobUpdate jobUpdate = request.getJobUpdate();
529+
JobUpdate jobUpdate = request.getJobUpdate();
530+
if (jobUpdate.isAutodetectProcessUpdate()) {
532531
if (isJobOpen(clusterService.state(), request.getJobId())) {
533532
updateJobProcessNotifier.submitJobUpdate(UpdateParams.fromJobUpdate(jobUpdate), ActionListener.wrap(isUpdated -> {
534533
if (isUpdated) {
535534
auditJobUpdatedIfNotInternal(request);
535+
} else {
536+
logger.error("[{}] Updating autodetect failed for job update [{}]", jobUpdate.getJobId(), jobUpdate);
536537
}
537538
}, e -> {
538-
// No need to do anything
539+
logger.error(
540+
new ParameterizedMessage(
541+
"[{}] Updating autodetect failed with an exception, job update [{}] ",
542+
jobUpdate.getJobId(),
543+
jobUpdate
544+
),
545+
e
546+
);
539547
}));
540548
}
541549
} else {
542-
logger.debug("[{}] No process update required for job update: {}", request::getJobId, () -> {
543-
try {
544-
XContentBuilder jsonBuilder = XContentFactory.jsonBuilder();
545-
request.getJobUpdate().toXContent(jsonBuilder, ToXContent.EMPTY_PARAMS);
546-
return Strings.toString(jsonBuilder);
547-
} catch (IOException e) {
548-
return "(unprintable due to " + e.getMessage() + ")";
549-
}
550-
});
551-
550+
logger.debug("[{}] No process update required for job update: {}", jobUpdate::getJobId, jobUpdate::toString);
552551
auditJobUpdatedIfNotInternal(request);
553552
}
554553

@@ -714,29 +713,38 @@ public void updateProcessOnCalendarChanged(List<String> calendarJobIds, ActionLi
714713
return;
715714
}
716715

716+
boolean appliesToAllJobs = calendarJobIds.stream().anyMatch(Metadata.ALL::equals);
717+
if (appliesToAllJobs) {
718+
submitJobEventUpdate(openJobIds, updateListener);
719+
return;
720+
}
721+
717722
// calendarJobIds may be a group or job
718-
jobConfigProvider.expandGroupIds(calendarJobIds, ActionListener.wrap(expandedIds -> {
719-
threadPool.executor(MachineLearning.UTILITY_THREAD_POOL_NAME).execute(() -> {
720-
// Merge the expended group members with the request Ids.
723+
jobConfigProvider.expandGroupIds(
724+
calendarJobIds,
725+
ActionListener.wrap(expandedIds -> threadPool.executor(MachineLearning.UTILITY_THREAD_POOL_NAME).execute(() -> {
726+
// Merge the expanded group members with the request Ids.
721727
// Ids that aren't jobs will be filtered by isJobOpen()
722728
expandedIds.addAll(calendarJobIds);
723729

724-
for (String jobId : expandedIds) {
725-
if (isJobOpen(clusterState, jobId)) {
726-
updateJobProcessNotifier.submitJobUpdate(
727-
UpdateParams.scheduledEventsUpdate(jobId),
728-
ActionListener.wrap(isUpdated -> {
729-
if (isUpdated) {
730-
auditor.info(jobId, Messages.getMessage(Messages.JOB_AUDIT_CALENDARS_UPDATED_ON_PROCESS));
731-
}
732-
}, e -> logger.error("[" + jobId + "] failed submitting process update on calendar change", e))
733-
);
734-
}
735-
}
730+
openJobIds.retainAll(expandedIds);
731+
submitJobEventUpdate(openJobIds, updateListener);
732+
}), updateListener::onFailure)
733+
);
734+
}
736735

737-
updateListener.onResponse(Boolean.TRUE);
738-
});
739-
}, updateListener::onFailure));
736+
private void submitJobEventUpdate(Collection<String> jobIds, ActionListener<Boolean> updateListener) {
737+
for (String jobId : jobIds) {
738+
updateJobProcessNotifier.submitJobUpdate(
739+
UpdateParams.scheduledEventsUpdate(jobId),
740+
ActionListener.wrap(
741+
isUpdated -> { auditor.info(jobId, Messages.getMessage(Messages.JOB_AUDIT_CALENDARS_UPDATED_ON_PROCESS)); },
742+
e -> logger.error("[" + jobId + "] failed submitting process update on calendar change", e)
743+
)
744+
);
745+
}
746+
747+
updateListener.onResponse(Boolean.TRUE);
740748
}
741749

742750
public void revertSnapshot(

0 commit comments

Comments
 (0)