Skip to content

Commit 1d20c55

Browse files
[ML] Improve performance of job exists check (#77156) (#77191)
* [ML] Improve performance of job exists check * adding tests * possible undefined error body
1 parent d2a02a6 commit 1d20c55

File tree

2 files changed

+157
-19
lines changed

2 files changed

+157
-19
lines changed

x-pack/plugins/ml/server/models/job_service/jobs.ts

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -407,28 +407,21 @@ export function jobsProvider(client: IScopedClusterClient) {
407407
// Job IDs in supplied array may contain wildcard '*' characters
408408
// e.g. *_low_request_rate_ecs
409409
async function jobsExist(jobIds: string[] = []) {
410-
// Get the list of job IDs.
411-
const { body } = await asInternalUser.ml.getJobs<MlJobsResponse>({
412-
job_id: jobIds.join(),
413-
});
414-
415410
const results: { [id: string]: boolean } = {};
416-
if (body.count > 0) {
417-
const allJobIds = body.jobs.map((job) => job.job_id);
418-
419-
// Check if each of the supplied IDs match existing jobs.
420-
jobIds.forEach((jobId) => {
421-
// Create a Regex for each supplied ID as wildcard * is allowed.
422-
const regexp = new RegExp(`^${jobId.replace(/\*+/g, '.*')}$`);
423-
const exists = allJobIds.some((existsJobId) => regexp.test(existsJobId));
424-
results[jobId] = exists;
425-
});
426-
} else {
427-
jobIds.forEach((jobId) => {
411+
for (const jobId of jobIds) {
412+
try {
413+
const { body } = await asInternalUser.ml.getJobs<MlJobsResponse>({
414+
job_id: jobId,
415+
});
416+
results[jobId] = body.count > 0;
417+
} catch (e) {
418+
// if a non-wildcarded job id is supplied, the get jobs endpoint will 404
419+
if (e.body?.status !== 404) {
420+
throw e;
421+
}
428422
results[jobId] = false;
429-
});
423+
}
430424
}
431-
432425
return results;
433426
}
434427

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import expect from '@kbn/expect';
8+
9+
import { FtrProviderContext } from '../../../ftr_provider_context';
10+
import { COMMON_REQUEST_HEADERS } from '../../../../functional/services/ml/common_api';
11+
import { USER } from '../../../../functional/services/ml/security_common';
12+
import { SINGLE_METRIC_JOB_CONFIG, DATAFEED_CONFIG } from './common_jobs';
13+
14+
export default ({ getService }: FtrProviderContext) => {
15+
const esArchiver = getService('esArchiver');
16+
const supertest = getService('supertestWithoutAuth');
17+
const ml = getService('ml');
18+
19+
const testSetupJobConfigs = [SINGLE_METRIC_JOB_CONFIG];
20+
21+
const responseBody = {
22+
[SINGLE_METRIC_JOB_CONFIG.job_id]: true,
23+
[`${SINGLE_METRIC_JOB_CONFIG.job_id.slice(0, 10)}*`]: true, // wildcard, use first 10 chars
24+
[`${SINGLE_METRIC_JOB_CONFIG.job_id}_fail`]: false,
25+
[`${SINGLE_METRIC_JOB_CONFIG.job_id.slice(0, 10)}_fail*`]: false, // wildcard, use first 10 chars
26+
};
27+
28+
const testDataList = [
29+
{
30+
testTitle: 'as ML Poweruser',
31+
user: USER.ML_POWERUSER,
32+
requestBody: {
33+
jobIds: Object.keys(responseBody),
34+
},
35+
expected: {
36+
responseCode: 200,
37+
responseBody,
38+
},
39+
},
40+
{
41+
testTitle: 'as ML Viewer',
42+
user: USER.ML_VIEWER,
43+
requestBody: {
44+
jobIds: Object.keys(responseBody),
45+
},
46+
expected: {
47+
responseCode: 200,
48+
responseBody,
49+
},
50+
},
51+
];
52+
53+
const testDataListUnauthorized = [
54+
{
55+
testTitle: 'as ML Unauthorized user',
56+
user: USER.ML_UNAUTHORIZED,
57+
requestBody: {
58+
jobIds: Object.keys(responseBody),
59+
},
60+
expected: {
61+
responseCode: 404,
62+
error: 'Not Found',
63+
},
64+
},
65+
];
66+
67+
async function runJobsExistRequest(
68+
user: USER,
69+
requestBody: object,
70+
expectedResponsecode: number
71+
): Promise<any> {
72+
const { body } = await supertest
73+
.post('/api/ml/jobs/jobs_exist')
74+
.auth(user, ml.securityCommon.getPasswordForUser(user))
75+
.set(COMMON_REQUEST_HEADERS)
76+
.send(requestBody)
77+
.expect(expectedResponsecode);
78+
79+
return body;
80+
}
81+
82+
describe('jobs_exist', function () {
83+
before(async () => {
84+
await esArchiver.loadIfNeeded('ml/farequote');
85+
await ml.testResources.createIndexPatternIfNeeded('ft_farequote', '@timestamp');
86+
await ml.testResources.setKibanaTimeZoneToUTC();
87+
});
88+
89+
after(async () => {
90+
await ml.api.cleanMlIndices();
91+
});
92+
93+
it('sets up jobs', async () => {
94+
for (const job of testSetupJobConfigs) {
95+
const datafeedId = `datafeed-${job.job_id}`;
96+
await ml.api.createAnomalyDetectionJob(job);
97+
await ml.api.openAnomalyDetectionJob(job.job_id);
98+
await ml.api.createDatafeed({
99+
...DATAFEED_CONFIG,
100+
datafeed_id: datafeedId,
101+
job_id: job.job_id,
102+
});
103+
}
104+
});
105+
106+
describe('jobs exist', function () {
107+
for (const testData of testDataList) {
108+
it(`${testData.testTitle}`, async () => {
109+
const body = await runJobsExistRequest(
110+
testData.user,
111+
testData.requestBody,
112+
testData.expected.responseCode
113+
);
114+
const expectedResponse = testData.expected.responseBody;
115+
const expectedRspJobIds = Object.keys(expectedResponse).sort((a, b) =>
116+
a.localeCompare(b)
117+
);
118+
const actualRspJobIds = Object.keys(body).sort((a, b) => a.localeCompare(b));
119+
120+
expect(actualRspJobIds).to.have.length(expectedRspJobIds.length);
121+
expect(actualRspJobIds).to.eql(expectedRspJobIds);
122+
expectedRspJobIds.forEach((id) => {
123+
expect(body[id]).to.eql(testData.expected.responseBody[id]);
124+
});
125+
});
126+
}
127+
});
128+
129+
describe('rejects request', function () {
130+
for (const testData of testDataListUnauthorized) {
131+
describe('fails to check jobs exist', function () {
132+
it(`${testData.testTitle}`, async () => {
133+
const body = await runJobsExistRequest(
134+
testData.user,
135+
testData.requestBody,
136+
testData.expected.responseCode
137+
);
138+
139+
expect(body).to.have.property('error').eql(testData.expected.error);
140+
});
141+
});
142+
}
143+
});
144+
});
145+
};

0 commit comments

Comments
 (0)