Skip to content

Commit

Permalink
[Code] Do not persist git update error (#42629)
Browse files Browse the repository at this point in the history
* [Code] Update log level for scheduler

* [Code] Do not persist git update error

* remove console.log
  • Loading branch information
mw-ding authored Aug 6, 2019
1 parent f7568e6 commit fe1389c
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 9 deletions.
56 changes: 55 additions & 1 deletion x-pack/legacy/plugins/code/server/queue/update_worker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ import { UpdateWorker } from './update_worker';

const log: Logger = new ConsoleLoggerFactory().getLogger(['test']);

const esClient = {};
const esClient = {
update: emptyAsyncFunc,
get: emptyAsyncFunc,
};
const esQueue = {};

afterEach(() => {
Expand Down Expand Up @@ -213,3 +216,54 @@ test('Execute update job failed because of low disk watermark ', async () => {
expect(updateSpy.notCalled).toBeTruthy();
}
});

test('On update job error or timeout will not persis error', async () => {
// Setup EsClient
const esUpdateSpy = sinon.spy();
esClient.update = esUpdateSpy;

// Setup CancellationService
const cancelUpdateJobSpy = sinon.spy();
const registerCancelableUpdateJobSpy = sinon.spy();
const cancellationService: any = {
cancelUpdateJob: emptyAsyncFunc,
registerCancelableUpdateJob: emptyAsyncFunc,
};
cancellationService.cancelUpdateJob = cancelUpdateJobSpy;
cancellationService.registerCancelableUpdateJob = registerCancelableUpdateJobSpy;

const updateWorker = new UpdateWorker(
esQueue as Esqueue,
log,
esClient as EsClient,
{
security: {
enableGitCertCheck: true,
},
disk: {
thresholdEnabled: true,
watermarkLowMb: 100,
},
} as ServerOptions,
{} as GitOperations,
{} as RepositoryServiceFactory,
cancellationService as CancellationSerivce,
{} as DiskWatermarkService
);

await updateWorker.onJobExecutionError({
job: {
payload: {
uri: 'mockrepo',
},
options: {},
timestamp: 0,
},
error: 'mock error message',
});

// The elasticsearch update will be called and the progress should be 'Completed'
expect(esUpdateSpy.calledOnce).toBeTruthy();
const updateBody = JSON.parse(esUpdateSpy.getCall(0).args[0].body);
expect(updateBody.doc.repository_git_status.progress).toBe(100);
});
17 changes: 16 additions & 1 deletion x-pack/legacy/plugins/code/server/queue/update_worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { CloneWorkerResult, Repository } from '../../model';
import { CloneWorkerResult, Repository, WorkerReservedProgress } from '../../model';
import { EsClient, Esqueue } from '../lib/esqueue';
import { DiskWatermarkService } from '../disk_watermark';
import { GitOperations } from '../git_operations';
Expand Down Expand Up @@ -77,4 +77,19 @@ export class UpdateWorker extends AbstractGitWorker {
this.log.info(`Update job done for ${job.payload.uri}`);
return await super.onJobCompleted(job, res);
}

public async onJobExecutionError(res: any) {
return await this.overrideUpdateErrorProgress(res);
}

public async onJobTimeOut(res: any) {
return await this.overrideUpdateErrorProgress(res);
}

private async overrideUpdateErrorProgress(res: any) {
this.log.warn(`Update job error`);
this.log.warn(res.error);
// Do not persist update errors assuming the next update trial is scheduling soon.
return await this.updateProgress(res.job, WorkerReservedProgress.COMPLETED);
}
}
10 changes: 5 additions & 5 deletions x-pack/legacy/plugins/code/server/scheduler/index_scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class IndexScheduler extends AbstractScheduler {
}

protected async executeSchedulingJob(repo: Repository) {
this.log.info(`Schedule index repo request for ${repo.uri}`);
this.log.debug(`Schedule index repo request for ${repo.uri}`);
try {
// This repository is too soon to execute the next index job.
if (repo.nextIndexTimestamp && new Date() < new Date(repo.nextIndexTimestamp)) {
Expand All @@ -44,27 +44,27 @@ export class IndexScheduler extends AbstractScheduler {
!RepositoryUtils.hasFullyCloned(cloneStatus.cloneProgress) ||
cloneStatus.progress !== WorkerReservedProgress.COMPLETED
) {
this.log.info(`Repo ${repo.uri} has not been fully cloned yet or in update. Skip index.`);
this.log.debug(`Repo ${repo.uri} has not been fully cloned yet or in update. Skip index.`);
return;
}

const repoIndexStatus = await this.objectClient.getRepositoryLspIndexStatus(repo.uri);

// Schedule index job only when the indexed revision is different from the current repository
// revision.
this.log.info(
this.log.debug(
`Current repo revision: ${repo.revision}, indexed revision ${repoIndexStatus.revision}.`
);
if (
repoIndexStatus.progress >= 0 &&
repoIndexStatus.progress < WorkerReservedProgress.COMPLETED
) {
this.log.info(`Repo is still in indexing. Skip index for ${repo.uri}`);
this.log.debug(`Repo is still in indexing. Skip index for ${repo.uri}`);
} else if (
repoIndexStatus.progress === WorkerReservedProgress.COMPLETED &&
repoIndexStatus.revision === repo.revision
) {
this.log.info(`Repo does not change since last index. Skip index for ${repo.uri}.`);
this.log.debug(`Repo does not change since last index. Skip index for ${repo.uri}.`);
} else {
const payload = {
uri: repo.uri,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class UpdateScheduler extends AbstractScheduler {
this.log.debug(`Repo ${repo.uri} is too soon to execute the next update job.`);
return;
}
this.log.info(`Start to schedule update repo request for ${repo.uri}`);
this.log.debug(`Start to schedule update repo request for ${repo.uri}`);

let inDelete = false;
try {
Expand Down Expand Up @@ -69,7 +69,7 @@ export class UpdateScheduler extends AbstractScheduler {

await this.updateWorker.enqueueJob(payload, {});
} else {
this.log.info(
this.log.debug(
`Repo ${repo.uri} has not been fully cloned yet or in update/delete. Skip update.`
);
}
Expand Down

0 comments on commit fe1389c

Please sign in to comment.