From 2850fd67350110fd11d3591813a9af0d4e44eee6 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Tue, 5 Feb 2019 12:29:01 +0000 Subject: [PATCH] [ML] Job deleting optimisations (#29848) * [ML] Job deleting optimisations * fixing force=true * updating deleting jobs check --- x-pack/plugins/ml/common/util/string_utils.js | 13 +++ .../delete_job_modal/delete_job_modal.js | 8 +- .../components/job_actions/management.js | 12 +-- .../components/job_actions/results.js | 4 +- .../components/job_group/job_group.js | 16 +--- .../components/jobs_list/jobs_list.js | 3 +- .../jobs_list_view/jobs_list_view.js | 34 +++++++ .../ml/public/services/ml_api_service/jobs.js | 7 ++ .../ml/server/client/elasticsearch_ml.js | 88 ++++++++++--------- .../ml/server/models/job_service/jobs.js | 32 ++++++- .../plugins/ml/server/routes/job_service.js | 14 +++ 11 files changed, 163 insertions(+), 68 deletions(-) diff --git a/x-pack/plugins/ml/common/util/string_utils.js b/x-pack/plugins/ml/common/util/string_utils.js index 52d2a71e7e1d..383c77f151b7 100644 --- a/x-pack/plugins/ml/common/util/string_utils.js +++ b/x-pack/plugins/ml/common/util/string_utils.js @@ -21,3 +21,16 @@ export function renderTemplate(str, data) { return str; } +export function stringHash(str) { + let hash = 0; + let chr = ''; + if (str.length === 0) { + return hash; + } + for (let i = 0; i < str.length; i++) { + chr = str.charCodeAt(i); + hash = ((hash << 5) - hash) + chr; + hash |= 0; + } + return hash < 0 ? hash * -2 : hash; +} diff --git a/x-pack/plugins/ml/public/jobs/jobs_list/components/delete_job_modal/delete_job_modal.js b/x-pack/plugins/ml/public/jobs/jobs_list/components/delete_job_modal/delete_job_modal.js index d74fbe14b55c..578c6ad76b93 100644 --- a/x-pack/plugins/ml/public/jobs/jobs_list/components/delete_job_modal/delete_job_modal.js +++ b/x-pack/plugins/ml/public/jobs/jobs_list/components/delete_job_modal/delete_job_modal.js @@ -67,10 +67,12 @@ export const DeleteJobModal = injectI18n(class extends Component { deleteJob = () => { this.setState({ deleting: true }); - deleteJobs(this.state.jobs, () => { + deleteJobs(this.state.jobs); + + setTimeout(() => { this.refreshJobs(); - this.closeModal(); - }); + }, 500); + this.closeModal(); } setEL = (el) => { diff --git a/x-pack/plugins/ml/public/jobs/jobs_list/components/job_actions/management.js b/x-pack/plugins/ml/public/jobs/jobs_list/components/job_actions/management.js index 2f867008a844..ba13b06821a1 100644 --- a/x-pack/plugins/ml/public/jobs/jobs_list/components/job_actions/management.js +++ b/x-pack/plugins/ml/public/jobs/jobs_list/components/job_actions/management.js @@ -35,7 +35,7 @@ export function actionsMenuContent(showEditJobFlyout, showDeleteJobModal, showSt defaultMessage: 'Start datafeed' }), icon: 'play', - enabled: () => (canStartStopDatafeed), + enabled: (item) => (item.deleting !== true && canStartStopDatafeed), available: (item) => (isStartable([item])), onClick: (item) => { showStartDatafeedModal([item]); @@ -49,7 +49,7 @@ export function actionsMenuContent(showEditJobFlyout, showDeleteJobModal, showSt defaultMessage: 'Stop datafeed' }), icon: 'stop', - enabled: () => (canStartStopDatafeed), + enabled: (item) => (item.deleting !== true && canStartStopDatafeed), available: (item) => (isStoppable([item])), onClick: (item) => { stopDatafeeds([item], refreshJobs); @@ -63,7 +63,7 @@ export function actionsMenuContent(showEditJobFlyout, showDeleteJobModal, showSt defaultMessage: 'Close job' }), icon: 'cross', - enabled: () => (canCloseJob), + enabled: (item) => (item.deleting !== true && canCloseJob), available: (item) => (isClosable([item])), onClick: (item) => { closeJobs([item], refreshJobs); @@ -87,7 +87,7 @@ export function actionsMenuContent(showEditJobFlyout, showDeleteJobModal, showSt return indexPatternNames.some(ipName => ipName === dfiName); }); - return canCreateJob && jobIndicesAvailable; + return (item.deleting !== true && canCreateJob && jobIndicesAvailable); }, onClick: (item) => { cloneJob(item.id); @@ -101,7 +101,7 @@ export function actionsMenuContent(showEditJobFlyout, showDeleteJobModal, showSt defaultMessage: 'Edit job' }), icon: 'pencil', - enabled: () => (canUpdateJob && canUpdateDatafeed), + enabled: (item) => (item.deleting !== true && canUpdateJob && canUpdateDatafeed), onClick: (item) => { showEditJobFlyout(item); closeMenu(); @@ -115,7 +115,7 @@ export function actionsMenuContent(showEditJobFlyout, showDeleteJobModal, showSt }), icon: 'trash', color: 'danger', - enabled: () => (canDeleteJob), + enabled: () => canDeleteJob, onClick: (item) => { showDeleteJobModal([item]); closeMenu(); diff --git a/x-pack/plugins/ml/public/jobs/jobs_list/components/job_actions/results.js b/x-pack/plugins/ml/public/jobs/jobs_list/components/job_actions/results.js index 054ce12f6593..3815dc2758de 100644 --- a/x-pack/plugins/ml/public/jobs/jobs_list/components/job_actions/results.js +++ b/x-pack/plugins/ml/public/jobs/jobs_list/components/job_actions/results.js @@ -54,6 +54,7 @@ function ResultLinksUI({ jobs, intl }) { }); const singleMetricVisible = (jobs.length < 2); const singleMetricEnabled = (jobs.length === 1 && jobs[0].isSingleMetricViewerJob); + const jobActionsDisabled = (jobs.length === 1 && jobs[0].deleting === true); return ( {(singleMetricVisible) && @@ -66,7 +67,7 @@ function ResultLinksUI({ jobs, intl }) { iconType="stats" aria-label={openJobsInSingleMetricViewerText} className="results-button" - isDisabled={(singleMetricEnabled === false)} + isDisabled={(singleMetricEnabled === false || jobActionsDisabled === true)} /> } @@ -79,6 +80,7 @@ function ResultLinksUI({ jobs, intl }) { iconType="tableOfContents" aria-label={openJobsInAnomalyExplorerText} className="results-button" + isDisabled={(jobActionsDisabled === true)} />
diff --git a/x-pack/plugins/ml/public/jobs/jobs_list/components/job_group/job_group.js b/x-pack/plugins/ml/public/jobs/jobs_list/components/job_group/job_group.js index 60d9b78bcab0..6ac4a3355be9 100644 --- a/x-pack/plugins/ml/public/jobs/jobs_list/components/job_group/job_group.js +++ b/x-pack/plugins/ml/public/jobs/jobs_list/components/job_group/job_group.js @@ -5,6 +5,8 @@ */ +import { stringHash } from '../../../../../common/util/string_utils'; + import PropTypes from 'prop-types'; import React from 'react'; @@ -53,17 +55,3 @@ function tabColor(name) { return colorMap[name]; } } - -function stringHash(str) { - let hash = 0; - let chr = ''; - if (str.length === 0) { - return hash; - } - for (let i = 0; i < str.length; i++) { - chr = str.charCodeAt(i); - hash = ((hash << 5) - hash) + chr; - hash |= 0; - } - return hash < 0 ? hash * -2 : hash; -} diff --git a/x-pack/plugins/ml/public/jobs/jobs_list/components/jobs_list/jobs_list.js b/x-pack/plugins/ml/public/jobs/jobs_list/components/jobs_list/jobs_list.js index b16641530243..b89e34adc9b1 100644 --- a/x-pack/plugins/ml/public/jobs/jobs_list/components/jobs_list/jobs_list.js +++ b/x-pack/plugins/ml/public/jobs/jobs_list/components/jobs_list/jobs_list.js @@ -101,7 +101,7 @@ class JobsListUI extends Component { render() { const { intl, loading } = this.props; const selectionControls = { - selectable: () => true, + selectable: job => (job.deleting !== true), selectableMessage: (selectable) => (!selectable) ? intl.formatMessage({ id: 'xpack.ml.jobsList.cannotSelectJobTooltip', defaultMessage: 'Cannot select job' }) @@ -115,6 +115,7 @@ class JobsListUI extends Component { render: (item) => ( this.toggleRow(item)} + isDisabled={(item.deleting === true)} iconType={this.state.itemIdToExpandedRowMap[item.id] ? 'arrowDown' : 'arrowRight'} aria-label={this.state.itemIdToExpandedRowMap[item.id] ? intl.formatMessage({ diff --git a/x-pack/plugins/ml/public/jobs/jobs_list/components/jobs_list_view/jobs_list_view.js b/x-pack/plugins/ml/public/jobs/jobs_list/components/jobs_list_view/jobs_list_view.js index ef571c4783d8..d0683eef0531 100644 --- a/x-pack/plugins/ml/public/jobs/jobs_list/components/jobs_list_view/jobs_list_view.js +++ b/x-pack/plugins/ml/public/jobs/jobs_list/components/jobs_list_view/jobs_list_view.js @@ -22,6 +22,7 @@ import { JobStatsBar } from '../jobs_stats_bar'; import { NodeAvailableWarning } from '../node_available_warning'; import { UpgradeWarning } from '../../../../components/upgrade'; import { RefreshJobsListButton } from '../refresh_jobs_list_button'; +import { isEqual } from 'lodash'; import React, { Component @@ -35,7 +36,9 @@ import { const DEFAULT_REFRESH_INTERVAL_MS = 30000; const MINIMUM_REFRESH_INTERVAL_MS = 5000; +const DELETING_JOBS_REFRESH_INTERVAL_MS = 2000; let jobsRefreshInterval = null; +let deletingJobsRefreshTimeout = null; export class JobsListView extends Component { constructor(props) { @@ -50,6 +53,7 @@ export class JobsListView extends Component { selectedJobs: [], itemIdToExpandedRowMap: {}, filterClauses: [], + deletingJobIds: [], }; this.updateFunctions = {}; @@ -285,7 +289,19 @@ export class JobsListView extends Component { this.updateFunctions[j].setState({ job: fullJobsList[j] }); }); + jobs.forEach((job) => { + if (job.deleting && this.state.itemIdToExpandedRowMap[job.id]) { + this.toggleRow(job.id); + } + }); + this.isDoneRefreshing(); + if (jobsSummaryList.some(j => j.deleting === true)) { + // if there are some jobs in a deleting state, start polling for + // deleting jobs so we can update the jobs list once the + // deleting tasks are over + this.checkDeletingJobTasks(); + } } catch (error) { console.error(error); this.setState({ loading: false }); @@ -293,6 +309,24 @@ export class JobsListView extends Component { } } + async checkDeletingJobTasks() { + const { jobIds } = await ml.jobs.deletingJobTasks(); + + if (jobIds.length === 0 || isEqual(jobIds.sort(), this.state.deletingJobIds.sort())) { + this.setState({ + deletingJobIds: jobIds, + }); + this.refreshJobSummaryList(true); + } + + if (jobIds.length > 0 && deletingJobsRefreshTimeout === null) { + deletingJobsRefreshTimeout = setTimeout(() => { + deletingJobsRefreshTimeout = null; + this.checkDeletingJobTasks(); + }, DELETING_JOBS_REFRESH_INTERVAL_MS); + } + } + renderJobsListComponents() { const { loading, jobsSummaryList } = this.state; const jobIds = jobsSummaryList.map(j => j.id); diff --git a/x-pack/plugins/ml/public/services/ml_api_service/jobs.js b/x-pack/plugins/ml/public/services/ml_api_service/jobs.js index ff277eaa13d5..c38c30f06d24 100644 --- a/x-pack/plugins/ml/public/services/ml_api_service/jobs.js +++ b/x-pack/plugins/ml/public/services/ml_api_service/jobs.js @@ -100,4 +100,11 @@ export const jobs = { }); }, + deletingJobTasks() { + return http({ + url: `${basePath}/jobs/deleting_jobs_tasks`, + method: 'GET', + }); + }, + }; diff --git a/x-pack/plugins/ml/server/client/elasticsearch_ml.js b/x-pack/plugins/ml/server/client/elasticsearch_ml.js index 47574fb6576c..3f67dd1e7234 100644 --- a/x-pack/plugins/ml/server/client/elasticsearch_ml.js +++ b/x-pack/plugins/ml/server/client/elasticsearch_ml.js @@ -83,15 +83,7 @@ export const elasticsearchJsPlugin = (Client, config, components) => { ml.closeJob = ca({ urls: [ { - fmt: '/_ml/anomaly_detectors/<%=jobId%>/_close', - req: { - jobId: { - type: 'string' - } - } - }, - { - fmt: '/_ml/anomaly_detectors/<%=jobId%>/_close?force=true', + fmt: '/_ml/anomaly_detectors/<%=jobId%>/_close?force=<%=force%>', req: { jobId: { type: 'string' @@ -100,30 +92,41 @@ export const elasticsearchJsPlugin = (Client, config, components) => { type: 'boolean' } } + }, + { + fmt: '/_ml/anomaly_detectors/<%=jobId%>/_close', + req: { + jobId: { + type: 'string' + } + } } ], method: 'POST' }); ml.deleteJob = ca({ - urls: [{ - fmt: '/_ml/anomaly_detectors/<%=jobId%>', - req: { - jobId: { - type: 'string' + urls: [ + { + fmt: '/_ml/anomaly_detectors/<%=jobId%>?&force=<%=force%>&wait_for_completion=false', + req: { + jobId: { + type: 'string' + }, + force: { + type: 'boolean' + } + } + }, + { + fmt: '/_ml/anomaly_detectors/<%=jobId%>?&wait_for_completion=false', + req: { + jobId: { + type: 'string' + } } } - }, { - fmt: '/_ml/anomaly_detectors/<%=jobId%>?force=true', - req: { - jobId: { - type: 'string' - }, - force: { - type: 'boolean' - } - } - }], + ], method: 'DELETE' }); @@ -207,24 +210,27 @@ export const elasticsearchJsPlugin = (Client, config, components) => { }); ml.deleteDatafeed = ca({ - urls: [{ - fmt: '/_ml/datafeeds/<%=datafeedId%>', - req: { - datafeedId: { - type: 'string' + urls: [ + { + fmt: '/_ml/datafeeds/<%=datafeedId%>?force=<%=force%>', + req: { + datafeedId: { + type: 'string' + }, + force: { + type: 'boolean' + } + } + }, + { + fmt: '/_ml/datafeeds/<%=datafeedId%>', + req: { + datafeedId: { + type: 'string' + } } } - }, { - fmt: '/_ml/datafeeds/<%=datafeedId%>?force=true', - req: { - datafeedId: { - type: 'string' - }, - force: { - type: 'boolean' - } - } - }], + ], method: 'DELETE' }); diff --git a/x-pack/plugins/ml/server/models/job_service/jobs.js b/x-pack/plugins/ml/server/models/job_service/jobs.js index e121096e3e15..6faac1cdc4aa 100644 --- a/x-pack/plugins/ml/server/models/job_service/jobs.js +++ b/x-pack/plugins/ml/server/models/job_service/jobs.js @@ -5,6 +5,7 @@ */ +import { i18n } from '@kbn/i18n'; import { JOB_STATE, DATAFEED_STATE } from '../../../common/constants/states'; import { datafeedsProvider } from './datafeeds'; import { jobAuditMessagesProvider } from '../job_audit_messages'; @@ -95,8 +96,12 @@ export function jobsProvider(callWithRequest) { return p; }, {}); + const deletingStr = i18n.translate('xpack.ml.models.jobService.deletingJob', { + defaultMessage: 'deleting', + }); + const jobs = fullJobsList.map((job) => { - const hasDatafeed = (typeof job.datafeed_config === 'object' && Object.keys(job.datafeed_config).length); + const hasDatafeed = (typeof job.datafeed_config === 'object' && Object.keys(job.datafeed_config).length > 0); const { earliest: earliestTimestampMs, latest: latestTimestampMs } = earliestAndLatestTimestamps(job.data_counts); @@ -107,7 +112,7 @@ export function jobsProvider(callWithRequest) { groups: (Array.isArray(job.groups) ? job.groups.sort() : []), processed_record_count: job.data_counts.processed_record_count, memory_status: (job.model_size_stats) ? job.model_size_stats.memory_status : '', - jobState: job.state, + jobState: (job.deleting === true) ? deletingStr : job.state, hasDatafeed, datafeedId: (hasDatafeed && job.datafeed_config.datafeed_id) ? job.datafeed_config.datafeed_id : '', datafeedIndices: (hasDatafeed && job.datafeed_config.indices) ? job.datafeed_config.indices : [], @@ -116,6 +121,7 @@ export function jobsProvider(callWithRequest) { earliestTimestampMs, isSingleMetricViewerJob: isTimeSeriesViewJob(job), nodeName: (job.node) ? job.node.name : undefined, + deleting: (job.deleting || undefined), }; if (jobIds.find(j => (j === tempJob.id))) { tempJob.fullJob = job; @@ -259,11 +265,33 @@ export function jobsProvider(callWithRequest) { return obj; } + async function deletingJobTasks() { + const actions = ['cluster:admin/xpack/ml/job/delete']; + const detailed = true; + const jobIds = []; + try { + const tasksList = await callWithRequest('tasks.list', { actions, detailed }); + Object.keys(tasksList.nodes).forEach((nodeId) => { + const tasks = tasksList.nodes[nodeId].tasks; + Object.keys(tasks).forEach((taskId) => { + jobIds.push(tasks[taskId].description.replace(/^delete-job-/, '')); + }); + }); + } catch (e) { + // if the user doesn't have permission to load the task list, + // use the jobs list to get the ids of deleting jobs + const { jobs } = await callWithRequest('ml.jobs'); + jobIds.push(...jobs.filter(j => j.deleting === true).map(j => j.job_id)); + } + return { jobIds }; + } + return { forceDeleteJob, deleteJobs, closeJobs, jobsSummary, createFullJobsList, + deletingJobTasks, }; } diff --git a/x-pack/plugins/ml/server/routes/job_service.js b/x-pack/plugins/ml/server/routes/job_service.js index 024bd03999a1..cd4697ed28e9 100644 --- a/x-pack/plugins/ml/server/routes/job_service.js +++ b/x-pack/plugins/ml/server/routes/job_service.js @@ -134,4 +134,18 @@ export function jobServiceRoutes(server, commonRouteConfig) { } }); + server.route({ + method: 'GET', + path: '/api/ml/jobs/deleting_jobs_tasks', + handler(request) { + const callWithRequest = callWithRequestFactory(server, request); + const { deletingJobTasks } = jobServiceProvider(callWithRequest); + return deletingJobTasks() + .catch(resp => wrapError(resp)); + }, + config: { + ...commonRouteConfig + } + }); + }