Skip to content

Commit

Permalink
fix: moved long running creation steps to worker threads to prevent m…
Browse files Browse the repository at this point in the history
…ain process overload (microsoft#6323)

* Initial addition of worker thread

* commneted out long running code and moved to worker
(scaffolding and initial implementation)

* Removing unused reference

* Adding dialog merge work to merge_worker execution

* Moving yeomen template execution to worker thread

* Added status updarte for creation from within worker

* Fixing yeoman spelling mistake

* Handling PR changes
- file and funciton name changes

* PR requested name changes

* fixing unit test

* Adding autogenerated locale strings

* Fixing unit test to mock worker and remove process.env assertion

Co-authored-by: Patrick Volum <[email protected]>
Co-authored-by: Srinaath Ravichandran <[email protected]>
Co-authored-by: Soroush <[email protected]>
  • Loading branch information
4 people authored Mar 10, 2021
1 parent 823d4e0 commit 5137457
Show file tree
Hide file tree
Showing 8 changed files with 190 additions and 81 deletions.
2 changes: 1 addition & 1 deletion Composer/packages/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
},
"author": "",
"nodemonConfig": {
"exec": "cross-env TS_NODE_FILES=true node --max-http-header-size=16000 --inspect=9228 -r ts-node/register src/init.ts",
"exec": "yarn build && yarn start",
"watch": [
"src",
"../extension/lib"
Expand Down
9 changes: 9 additions & 0 deletions Composer/packages/server/src/locales/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -1568,6 +1568,9 @@
"getting_template_910a4116": {
"message": "Getting template"
},
"getting_yeoman_environment_4f50aae3": {
"message": "Getting Yeoman environment"
},
"go_to_qna_all_up_view_page_d475333d": {
"message": "Go to QnA all-up view page."
},
Expand Down Expand Up @@ -1718,6 +1721,12 @@
"install_the_update_and_restart_composer_fac30a61": {
"message": "Install the update and restart Composer."
},
"installing_yeoman_template_38cf1e48": {
"message": "Installing Yeoman template"
},
"instantiating_yeoman_template_5acf7e3f": {
"message": "Instantiating Yeoman template"
},
"integer_7f378275": {
"message": "integer"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import { Path } from '../../../utility/path';
import { AssetManager } from '../assetManager';
import StorageService from '../../../services/storage';

const mockchdir = jest.spyOn(process, 'chdir').mockImplementation(() => {});

jest.mock('azure-storage', () => {
return {};
});
Expand Down Expand Up @@ -42,6 +40,14 @@ jest.mock('../../../models/extension/extensionContext', () => {
};
});

jest.mock('../../../workers/templateInstallation.worker', () => {
return {
runYeomanTemplatePipeline: () => {
return;
},
};
});

const mockSampleBotPath = Path.join(__dirname, '../../../__mocks__/asset/projects/SampleBot');
const mockCopyToPath = Path.join(__dirname, '../../../__mocks__/new');
const locationRef = {
Expand Down Expand Up @@ -157,13 +163,13 @@ describe('assetManager', () => {
'generator-conversational-core',
'1.0.3',
'sampleConversationalCore',
mockLocRef
mockLocRef,
'0'
);
expect(newBotLocationRef).toStrictEqual({
path: '/path/to/npmbot/sampleConversationalCore',
storageId: 'default',
});
expect(mockchdir).toBeCalledWith('/path/to/npmbot');
});
});
});
67 changes: 4 additions & 63 deletions Composer/packages/server/src/models/asset/assetManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ import path from 'path';
import find from 'lodash/find';
import { UserIdentity, FileExtensions, FeedType } from '@bfc/extension';
import { mkdirSync, readFile } from 'fs-extra';
import yeoman from 'yeoman-environment';
import Environment from 'yeoman-environment';
import TerminalAdapter from 'yeoman-environment/lib/adapter';
import { BotTemplate, QnABotTemplateId } from '@bfc/shared';

import { ExtensionContext } from '../extension/extensionContext';
Expand All @@ -21,7 +18,7 @@ import { copyDir } from '../../utility/storage';
import StorageService from '../../services/storage';
import { IFileStorage } from '../storage/interface';
import { BotProject } from '../bot/botProject';
import { templateGeneratorPath } from '../../settings/env';
import { runYeomanTemplatePipeline } from '../../workers/templateInstallation.worker';

export class AssetManager {
public templateStorage: LocalDiskStorage;
Expand Down Expand Up @@ -96,6 +93,7 @@ export class AssetManager {
templateVersion: string,
projectName: string,
ref: LocationRef,
jobId: string,
user?: UserIdentity
): Promise<LocationRef> {
try {
Expand All @@ -110,30 +108,9 @@ export class AssetManager {
log('About to create folder', dstDir);
mkdirSync(dstDir, { recursive: true });

// find selected template
const npmPackageName = templateId === QnABotTemplateId ? 'generator-empty-bot' : templateId;
const generatorName = npmPackageName.toLowerCase().replace('generator-', '');

// create yeoman environment
const yeomanEnv = yeoman.createEnv(
'',
{ yeomanRepository: templateGeneratorPath },
new TerminalAdapter({ console: console })
);
yeomanEnv.lookupLocalPackages();

const remoteTemplateAvailable = await this.installRemoteTemplate(
yeomanEnv,
generatorName,
npmPackageName,
templateVersion
);

if (remoteTemplateAvailable) {
await this.instantiateRemoteTemplate(yeomanEnv, generatorName, dstDir, projectName);
} else {
throw new Error(`error hit when installing remote template`);
}

await runYeomanTemplatePipeline(npmPackageName, templateVersion, dstDir, projectName, jobId);

ref.path = `${ref.path}/${projectName}`;

Expand All @@ -149,42 +126,6 @@ export class AssetManager {
}
}

private async installRemoteTemplate(
yeomanEnv: Environment,
generatorName: string,
npmPackageName: string,
templateVersion: string
): Promise<boolean> {
yeomanEnv.cwd = templateGeneratorPath;
try {
log('Installing generator', npmPackageName);
templateVersion = templateVersion ? templateVersion : '*';
await yeomanEnv.installLocalGenerators({ [npmPackageName]: templateVersion });

log('Looking up local packages');
await yeomanEnv.lookupLocalPackages();
return true;
} catch {
return false;
}
}

private async instantiateRemoteTemplate(
yeomanEnv: Environment,
generatorName: string,
dstDir: string,
projectName: string
): Promise<boolean> {
log('About to instantiate a template!', dstDir, generatorName, projectName);
yeomanEnv.cwd = dstDir;
process.chdir(dstDir);

await yeomanEnv.run([generatorName, projectName], {}, () => {
log('Template successfully instantiated', dstDir, generatorName, projectName);
});
return true;
}

private async copyDataFilesTo(templateId: string, dstDir: string, dstStorage: IFileStorage, locale?: string) {
const template = find(ExtensionContext.extensions.botTemplates, { id: templateId });
if (template === undefined || (template.path === undefined && template.package === undefined)) {
Expand Down
1 change: 1 addition & 0 deletions Composer/packages/server/src/services/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@ export class BotProjectService {
templateVersion,
name,
locationRef,
jobId,
user
);

Expand Down
15 changes: 2 additions & 13 deletions Composer/packages/server/src/utility/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import * as fs from 'fs';

import { remove } from 'fs-extra';
import { SchemaMerger } from '@microsoft/bf-dialog/lib/library/schemaMerger';
import formatMessage from 'format-message';
import { UserIdentity } from '@botframework-composer/types';

Expand All @@ -15,6 +14,7 @@ import log from '../logger';
import AssetService from '../services/asset';
import { BotProject } from '../models/bot/botProject';
import { BackgroundProcessManager } from '../services/backgroundProcessManager';
import { runDialogMerge } from '../workers/dialogMerge.worker';

import { Path } from './path';

Expand Down Expand Up @@ -79,18 +79,7 @@ export async function ejectAndMerge(currentProject: BotProject, jobId: string) {

// run the merge command to merge all package dependencies from the template to the bot project
BackgroundProcessManager.updateProcess(jobId, 202, formatMessage('Merging Packages'));
const realMerge = new SchemaMerger(
[manifestFile, '!**/imported/**', '!**/generated/**'],
Path.join(currentProject.dataDir, 'schemas/sdk'),
Path.join(currentProject.dataDir, 'dialogs/imported'),
false,
false,
console.log,
console.warn,
console.error
);

await realMerge.merge();
await runDialogMerge(manifestFile, currentProject);
} else {
log('Schema merge step skipped for project without runtime path');
}
Expand Down
50 changes: 50 additions & 0 deletions Composer/packages/server/src/workers/dialogMerge.worker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { Worker, isMainThread, workerData, parentPort } from 'worker_threads';

import { SchemaMerger } from '@microsoft/bf-dialog/lib/library/schemaMerger';

import { Path } from '../utility/path';
import { BotProject } from '../models/bot/botProject';

if (!isMainThread) {
const realMerge = new SchemaMerger(
[workerData.manifestFile, '!**/imported/**', '!**/generated/**'],
Path.join(workerData.dataDir, 'schemas/sdk'),
Path.join(workerData.dataDir, 'dialogs/imported'),
false,
false,
console.log,
console.warn,
console.error
);

realMerge
.merge()
.then(() => {
process.exit(0);
})
.catch((err) => {
parentPort?.postMessage({ error: err });
process.exit(1);
});
}

export function runDialogMerge(manifestFile: string, currentProject: BotProject) {
return new Promise<void>((resolve, reject) => {
const w = new Worker(__filename, {
workerData: { manifestFile, dataDir: currentProject.dataDir },
});
w.on('exit', (returnCode) => {
if (returnCode === 0) {
resolve();
}
});
w.on('message', (message) => {
if (message?.error) {
reject(message.error);
}
});
});
}
113 changes: 113 additions & 0 deletions Composer/packages/server/src/workers/templateInstallation.worker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { Worker, isMainThread, workerData, parentPort } from 'worker_threads';

import Environment from 'yeoman-environment';
import yeoman from 'yeoman-environment';
import TerminalAdapter from 'yeoman-environment/lib/adapter';
import formatMessage from 'format-message';

import { templateGeneratorPath } from '../settings/env';
import log from '../logger';
import { BackgroundProcessManager } from '../services/backgroundProcessManager';

const installRemoteTemplate = async (
yeomanEnv: Environment,
generatorName: string,
npmPackageName: string,
templateVersion: string
): Promise<boolean> => {
yeomanEnv.cwd = templateGeneratorPath;
try {
log('Installing generator', npmPackageName);
templateVersion = templateVersion ? templateVersion : '*';
await yeomanEnv.installLocalGenerators({ [npmPackageName]: templateVersion });

log('Looking up local packages');
await yeomanEnv.lookupLocalPackages();
return true;
} catch {
return false;
}
};

const instantiateRemoteTemplate = async (
yeomanEnv: Environment,
generatorName: string,
dstDir: string,
projectName: string
) => {
log('About to instantiate a template!', dstDir, generatorName, projectName);
yeomanEnv.cwd = dstDir;

await yeomanEnv.run([generatorName, projectName], {}, () => {
log('Template successfully instantiated', dstDir, generatorName, projectName);
});
};

const yeomanWork = async (npmPackageName: string, templateVersion: string, dstDir: string, projectName: string) => {
const generatorName = npmPackageName.toLowerCase().replace('generator-', '');
// create yeoman environment
parentPort?.postMessage({ status: formatMessage('Getting Yeoman environment') });

const yeomanEnv = yeoman.createEnv(
'',
{ yeomanRepository: templateGeneratorPath },
new TerminalAdapter({ console: console })
);
await yeomanEnv.lookupLocalPackages();

parentPort?.postMessage({ status: formatMessage('Installing Yeoman template') });

const remoteTemplateAvailable = await installRemoteTemplate(
yeomanEnv,
generatorName,
npmPackageName,
templateVersion
);
if (remoteTemplateAvailable) {
parentPort?.postMessage({ status: formatMessage('Instantiating Yeoman template') });

await instantiateRemoteTemplate(yeomanEnv, generatorName, dstDir, projectName);
} else {
// handle error
throw new Error(`error hit when installing remote template`);
}
};

export function runYeomanTemplatePipeline(
npmPackageName: string,
templateVersion: string,
dstDir: string,
projectName: string,
jobId: string
) {
return new Promise<void>((resolve, reject) => {
const w = new Worker(__filename, {
workerData: { npmPackageName, templateVersion, dstDir, projectName },
});
w.on('exit', () => {
resolve();
});
w.on('message', (message) => {
if (message?.error) {
reject(message.error);
}
if (message?.status) {
BackgroundProcessManager.updateProcess(jobId, 202, message?.status);
}
});
});
}

if (!isMainThread) {
yeomanWork(workerData.npmPackageName, workerData.templateVersion, workerData.dstDir, workerData.projectName)
.then(() => {
process.exit(0);
})
.catch((err) => {
parentPort?.postMessage({ error: err });
process.exit(1);
});
}

0 comments on commit 5137457

Please sign in to comment.