Skip to content

Commit

Permalink
[ML] Improve performance of job exists check (elastic#77156) (elastic…
Browse files Browse the repository at this point in the history
…#77191)

* [ML] Improve performance of job exists check

* adding tests

* possible undefined error body
  • Loading branch information
jgowdyelastic authored Sep 10, 2020
1 parent d2a02a6 commit 1d20c55
Show file tree
Hide file tree
Showing 2 changed files with 157 additions and 19 deletions.
31 changes: 12 additions & 19 deletions x-pack/plugins/ml/server/models/job_service/jobs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -407,28 +407,21 @@ export function jobsProvider(client: IScopedClusterClient) {
// Job IDs in supplied array may contain wildcard '*' characters
// e.g. *_low_request_rate_ecs
async function jobsExist(jobIds: string[] = []) {
// Get the list of job IDs.
const { body } = await asInternalUser.ml.getJobs<MlJobsResponse>({
job_id: jobIds.join(),
});

const results: { [id: string]: boolean } = {};
if (body.count > 0) {
const allJobIds = body.jobs.map((job) => job.job_id);

// Check if each of the supplied IDs match existing jobs.
jobIds.forEach((jobId) => {
// Create a Regex for each supplied ID as wildcard * is allowed.
const regexp = new RegExp(`^${jobId.replace(/\*+/g, '.*')}$`);
const exists = allJobIds.some((existsJobId) => regexp.test(existsJobId));
results[jobId] = exists;
});
} else {
jobIds.forEach((jobId) => {
for (const jobId of jobIds) {
try {
const { body } = await asInternalUser.ml.getJobs<MlJobsResponse>({
job_id: jobId,
});
results[jobId] = body.count > 0;
} catch (e) {
// if a non-wildcarded job id is supplied, the get jobs endpoint will 404
if (e.body?.status !== 404) {
throw e;
}
results[jobId] = false;
});
}
}

return results;
}

Expand Down
145 changes: 145 additions & 0 deletions x-pack/test/api_integration/apis/ml/jobs/jobs_exist.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import expect from '@kbn/expect';

import { FtrProviderContext } from '../../../ftr_provider_context';
import { COMMON_REQUEST_HEADERS } from '../../../../functional/services/ml/common_api';
import { USER } from '../../../../functional/services/ml/security_common';
import { SINGLE_METRIC_JOB_CONFIG, DATAFEED_CONFIG } from './common_jobs';

export default ({ getService }: FtrProviderContext) => {
const esArchiver = getService('esArchiver');
const supertest = getService('supertestWithoutAuth');
const ml = getService('ml');

const testSetupJobConfigs = [SINGLE_METRIC_JOB_CONFIG];

const responseBody = {
[SINGLE_METRIC_JOB_CONFIG.job_id]: true,
[`${SINGLE_METRIC_JOB_CONFIG.job_id.slice(0, 10)}*`]: true, // wildcard, use first 10 chars
[`${SINGLE_METRIC_JOB_CONFIG.job_id}_fail`]: false,
[`${SINGLE_METRIC_JOB_CONFIG.job_id.slice(0, 10)}_fail*`]: false, // wildcard, use first 10 chars
};

const testDataList = [
{
testTitle: 'as ML Poweruser',
user: USER.ML_POWERUSER,
requestBody: {
jobIds: Object.keys(responseBody),
},
expected: {
responseCode: 200,
responseBody,
},
},
{
testTitle: 'as ML Viewer',
user: USER.ML_VIEWER,
requestBody: {
jobIds: Object.keys(responseBody),
},
expected: {
responseCode: 200,
responseBody,
},
},
];

const testDataListUnauthorized = [
{
testTitle: 'as ML Unauthorized user',
user: USER.ML_UNAUTHORIZED,
requestBody: {
jobIds: Object.keys(responseBody),
},
expected: {
responseCode: 404,
error: 'Not Found',
},
},
];

async function runJobsExistRequest(
user: USER,
requestBody: object,
expectedResponsecode: number
): Promise<any> {
const { body } = await supertest
.post('/api/ml/jobs/jobs_exist')
.auth(user, ml.securityCommon.getPasswordForUser(user))
.set(COMMON_REQUEST_HEADERS)
.send(requestBody)
.expect(expectedResponsecode);

return body;
}

describe('jobs_exist', function () {
before(async () => {
await esArchiver.loadIfNeeded('ml/farequote');
await ml.testResources.createIndexPatternIfNeeded('ft_farequote', '@timestamp');
await ml.testResources.setKibanaTimeZoneToUTC();
});

after(async () => {
await ml.api.cleanMlIndices();
});

it('sets up jobs', async () => {
for (const job of testSetupJobConfigs) {
const datafeedId = `datafeed-${job.job_id}`;
await ml.api.createAnomalyDetectionJob(job);
await ml.api.openAnomalyDetectionJob(job.job_id);
await ml.api.createDatafeed({
...DATAFEED_CONFIG,
datafeed_id: datafeedId,
job_id: job.job_id,
});
}
});

describe('jobs exist', function () {
for (const testData of testDataList) {
it(`${testData.testTitle}`, async () => {
const body = await runJobsExistRequest(
testData.user,
testData.requestBody,
testData.expected.responseCode
);
const expectedResponse = testData.expected.responseBody;
const expectedRspJobIds = Object.keys(expectedResponse).sort((a, b) =>
a.localeCompare(b)
);
const actualRspJobIds = Object.keys(body).sort((a, b) => a.localeCompare(b));

expect(actualRspJobIds).to.have.length(expectedRspJobIds.length);
expect(actualRspJobIds).to.eql(expectedRspJobIds);
expectedRspJobIds.forEach((id) => {
expect(body[id]).to.eql(testData.expected.responseBody[id]);
});
});
}
});

describe('rejects request', function () {
for (const testData of testDataListUnauthorized) {
describe('fails to check jobs exist', function () {
it(`${testData.testTitle}`, async () => {
const body = await runJobsExistRequest(
testData.user,
testData.requestBody,
testData.expected.responseCode
);

expect(body).to.have.property('error').eql(testData.expected.error);
});
});
}
});
});
};

0 comments on commit 1d20c55

Please sign in to comment.